feat(storage): retry SignBlob call for URL signing

[thiyaguk09]

Adds a retry mechanism to the SignBlob call's SignedURL() method to improve reliability in case of transient network or server issues. This change ensures that the SignedURL() operation can be retried under certain conditions, increasing the likelihood of successful URL generation.

[bshaffer]

generally speaking, calling private methods is a bad way to test. It would be better to mock the Connection class being passed to v2Sign, in a way that can trigger the retry.

I can work with you on how to write tests like that, if you'd like.

[thiyaguk09]

That's a great point. Mocking the Connection class to trigger retries is a much more robust and testable approach. I'd be happy to work with you on implementing this.

[frankyn]

I'm confused; shouldn't the retryDecider decide max number of attempts?

Hmm the documentation on this trait is confusing that it uses maxAttempts but doesn't have it in the function signature:
https://github.com/googleapis/google-cloud-php/blob/main/Storage/src/Connection/RetryTrait.php#L190

@bshaffer am I misunderstanding this? i'm not up-to-date on PHP

[thiyaguk09]

You're right, I couldn't find a maxRetries or maxAttempts option in the documentation either. That's why I added the maxAttempt directly.

[frankyn]

I'll approve from my side; just need @bshaffer to weigh in here when he has a chance.

[bshaffer]

If I'm reading this correctly, the current code does not have any check for maxAttempts, so it seems like this would result in an infinite loop. There is no test case for a retryable error meeting its max attempts. I feel like we definitely need a retry max attempts.

[bshaffer]

I know that the logic for "maxAttempts" was intentionally removed, but I think that we still need some kind of protection here. Otherwise the user could get in an infinite loop.

Here's an example of a test I've added that results in an infinite loop, when it should be passing:

    public function testRetrySignBlobThrowsExceptionAfterThreeAttempts()
    {
        $this->expectException(ServiceException::class);
        $this->expectExceptionMessage('Transient error (4 attempts)');

        $attempt = 0;
        $signBlobFn = function () use (&$attempt) {
            throw new ServiceException(
                sprintf('Transient error (%s attempts)', ++$attempt),
                503
            );
        };

        $this->helper->proxyPrivateMethodCall('retrySignBlob', [$signBlobFn]);
    }

I've hardcoded this to have a maxAttempts of 3, but it could be any number.