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

core: add 502 errors to retry logic #769

Merged

Conversation

stephenplusplus
Copy link
Contributor

Our system tests have been flaking out again. We previously implemented retry logic with exponential backoff to try to resolve the problem. I'm wondering if we need to loosen what triggers the retry logic.

Currently, we check if the status code returned is: 429, 500, or 503. This PR adds 502 to the list, "Bad Gateway", as an attempt to resolve this consistently failing test when run on Travis (but not locally):

1) datastore querying the datastore should select projections:
   Uncaught Error: Bad Gateway
    at Object.parseApiResp (lib/common/util.js:9:5290)
    at Object.handleResp (lib/common/util.js:9:4578)
    at IncomingMessage.<anonymous> (lib/datastore/request.js:9:15948
    at _stream_readable.js:908:16

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 31, 2015
@callmehiphop
Copy link
Contributor

Do we actually want to retry requests for bad gateways? If we do great, but I'd rather not add this code if it is only to work around Travis.

@stephenplusplus
Copy link
Contributor Author

I don't actually know. We currently support:

  • 500 The server encountered an unexpected condition which prevented it from fulfilling the request.
  • 503 The server is currently unable to handle the request due to a temporary overloading or maintenance of the server. The implication is that this is a temporary condition which will be alleviated after some delay.

And 502:

  • 502 The server, while acting as a gateway or proxy, received an invalid response from the upstream server it accessed in attempting to fulfill the request.

(http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html)

@stephenplusplus
Copy link
Contributor Author

@jgeewax any idea if we should we run our retry logic when the server responds with a 502?

@jgeewax
Copy link
Contributor

jgeewax commented Aug 3, 2015

I think this is reasonable (to retry when we get a 502) as the request (as far as we're concerned) is perfectly fine, but the server behind the proxy had an issue...

@stephenplusplus
Copy link
Contributor Author

Sounds good. Thanks!

stephenplusplus added a commit that referenced this pull request Aug 3, 2015
@stephenplusplus stephenplusplus merged commit f680f63 into googleapis:master Aug 3, 2015
sofisl pushed a commit that referenced this pull request Nov 10, 2022
- [ ] Regenerate this pull request now.

Committer: @summer-ji-eng
PiperOrigin-RevId: 434859890

Source-Link: googleapis/googleapis@bc2432d

Source-Link: googleapis/googleapis-gen@930b673
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiOTMwYjY3MzEwM2U5MjUyM2Y4Y2ZlZDM4ZGVjZDdkM2FmYWU4ZWJlNyJ9
gcf-owl-bot bot added a commit that referenced this pull request Jan 20, 2023
Source-Link: googleapis/googleapis@9fac84a

Source-Link: googleapis/googleapis-gen@12ae1da
Copy-Tag: eyJwIjoicGFja2FnZXMvZ29vZ2xlLWNsb3VkLWNvbXB1dGUvLk93bEJvdC55YW1sIiwiaCI6IjEyYWUxZGE3YzEzNDcyYTExZTFhNGE2MTg5YWFiMTBmNzgzYzU4NTMifQ==
sofisl added a commit that referenced this pull request Jan 24, 2023
* feat: Update Compute Engine API to revision 20221224 (#760)

Source-Link: googleapis/googleapis@17d5613

Source-Link: googleapis/googleapis-gen@42cd552
Copy-Tag: eyJwIjoicGFja2FnZXMvZ29vZ2xlLWNsb3VkLWNvbXB1dGUvLk93bEJvdC55YW1sIiwiaCI6IjQyY2Q1NTJmYTViZTk1YmU1Y2U1MTM2YmE5M2U4MzM4OGI3NTkxZDgifQ==

* feat: Update Compute Engine API to revision 20221224 (#760)

Source-Link: googleapis/googleapis@17d5613

Source-Link: googleapis/googleapis-gen@42cd552
Copy-Tag: eyJwIjoicGFja2FnZXMvZ29vZ2xlLWNsb3VkLWNvbXB1dGUvLk93bEJvdC55YW1sIiwiaCI6IjQyY2Q1NTJmYTViZTk1YmU1Y2U1MTM2YmE5M2U4MzM4OGI3NTkxZDgifQ==

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* feat: Update Compute Engine API to revision 20230103 (#769)

Source-Link: googleapis/googleapis@9fac84a

Source-Link: googleapis/googleapis-gen@12ae1da
Copy-Tag: eyJwIjoicGFja2FnZXMvZ29vZ2xlLWNsb3VkLWNvbXB1dGUvLk93bEJvdC55YW1sIiwiaCI6IjEyYWUxZGE3YzEzNDcyYTExZTFhNGE2MTg5YWFiMTBmNzgzYzU4NTMifQ==

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
@release-please release-please bot mentioned this pull request Jan 24, 2023
@release-please release-please bot mentioned this pull request Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants