Repository navigation
fix: retry transient GraphQL errors beyond HTTP 502 - #2915
Draft
CodeByPeace wants to merge 1 commit into
Draft
CodeByPeace wants to merge 1 commit into
CodeByPeace wants to merge 1 commit into
Conversation
Both GraphQL retry loops only retried on HTTP 502. A GraphQL error returned with HTTP 200 has no status, so it was thrown at once. Add one shared check that also treats HTTP 500, 503 and 504 and the two transient error messages from the issue as retryable. Other errors are handled as before. Fixes googleapis#2905
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2905
This is a draft until a maintainer answers my comment on the issue.
What was wrong: Both GraphQL retry loops, in github.ts and github-api.ts, only retried when the status was 502. A GraphQL error returned with HTTP 200 has no status, so it was thrown after one request. HTTP 500, 503 and 504 also stopped after one request.
What changed: I added one shared check, isTransientGraphqlError, in github-api.ts. Both loops now use it. It retries HTTP 500, 502, 503 and 504, and the two error messages named in the issue. Everything else is thrown at once, as before. The log line now says transient GraphQL error instead of 502 error.
How I tested: I wrote a failing test for the HTTP 200 case first and watched it fail. I added tests for 500, 503 and 504, a test that a normal query error is not retried, and direct tests of the check. I ran npm test, 1226 passing, and npm run fix with no errors. I ran these myself.
Limits: The batch size halving that already ran on 502 now runs for the other transient errors too. I did not add a loop test for the copy in github-api.ts, only tests of the check it uses.
Left alone: rate limit, auth and query errors, the retry count and the sleep times. There are no mutations in src, so nothing that creates a release or PR is retried.
AI use: written with Claude Sonnet 5.5. I ran and checked every command and test myself.