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

Retry on errors 408 and 504 in RetryDecider #2633

Open
venix1 opened this issue Jan 26, 2020 · 1 comment
Open

Retry on errors 408 and 504 in RetryDecider #2633

venix1 opened this issue Jan 26, 2020 · 1 comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@venix1
Copy link

venix1 commented Jan 26, 2020

Add retry support for 408 and 504 codes per best practices.

API documentation suggests retry logic to handle errors, and indicates the following status codes as ones that should be retried.

  • 408 Request Timeout
  • 500 Internal Server Error
  • 502 Bad Gateway
  • 503 Service Unavailable
  • 504 Gateway Timeout
    [1]

The library currently includes support for this but only with 500, 502, and 503 errors. This leaves out 408 and 504 which requires a custom handler always be implemented to correctly retry all possible error conditions.

    private $httpRetryCodes = [
        500,
        502,
        503
    ];
...
            if (in_array($statusCode, $httpRetryCodes)) {
                return true;
            }

[2]

Adding 408 and 504 to $httpRetryCodes should enable support for this, due to CLA requirements I am unable to open a PR.

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Jan 27, 2020
@dwsupplee dwsupplee added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed triage me I really want to be triaged. labels Jan 28, 2020
@ava12
Copy link
Contributor

ava12 commented Aug 3, 2020

Truncated exponential backoff section for Cloud Storage docs states that client should retry requests for any 5xx code for any Storage request and 408 code for resumable uploads. Looks like we need two custom handlers.

@dwsupplee, what is the status of this feature request? Do we need this feature? If we do, should it be enabled by default or there should be some flag to enable it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

4 participants