Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: include exception in on_retry_callback #38

Closed
ViktorCollin opened this issue Nov 21, 2023 · 0 comments
Closed

Suggestion: include exception in on_retry_callback #38

ViktorCollin opened this issue Nov 21, 2023 · 0 comments
Assignees
Milestone

Comments

@ViktorCollin
Copy link

Detailed description

It would be useful for logging and reporting reasons to know get access to the exception that caused the retry in the on_retry_callback. I have not found a way to log what type of error that caused a retry due to a ConnectException.

Context

my setup looks like this:

GuzzleRetryMiddleware::factory([
    'max_retry_attempts' => 5,
    'give_up_after_secs' => 20,
    'retry_on_status' => [429, 500, 502, 503, 504],
    // This is not only timeouts, it is all ConnectException (https://github.com/caseyamcl/guzzle_retry_middleware/blob/v2.9.0/src/GuzzleRetryMiddleware.php#L239C55)
    'retry_on_timeout' => true,
    'on_retry_callback' => function (int $attemptNumber, float $delay, RequestInterface &$request, array &$options, ?ResponseInterface $response) {
        $maxAttempt = $options['max_retry_attempts'];
        $cause = $response ? "response status: {$response->getStatusCode()}" : 'Unknown cause';
        $this->logger->warning("Retrying request ($attemptNumber/$maxAttempt) in {$delay}ms. cause: $cause");
    }
])

As you can see i'm forces to log Unknown cause if a ConnectException occurred as there is no response in that case.

Possible implementation

One possible solution could be to change the signature of the doRetry function to something like this:

/**
 * Retry the request
 *
 * Increments the retry count, determines the delay (timeout), executes callbacks, sleeps, and re-sends the request
 *
 * @param RequestInterface $request
 * @param array<string,mixed> $options
 * @param ResponseInterface|null $response
 * @param RequestException|null $exception
 * @return Promise
 */
protected function doRetry(RequestInterface $request, array $options, ResponseInterface $response = null, RequestException $exception = null): Promise

using GuzzleHttp\Exception\RequestException as that includes both BadResponseException and ConnectException

I can help with the implementation if you think that it is a good idea

@caseyamcl caseyamcl added this to the 2.10 milestone Nov 30, 2023
@caseyamcl caseyamcl self-assigned this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants