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.

Sign up for GitHub

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

Jump to bottom

Should the internal.HTTPClient retry when Google's servers return 502 default for the Messaging Endpoint? #640

Open
huyi1985 opened this issue Aug 26, 2024 · 2 comments
Open

Should the internal.HTTPClient retry when Google's servers return 502 default for the Messaging Endpoint? #640

huyi1985 opened this issue Aug 26, 2024 · 2 comments
Assignees
@chong-shao
Labels
api: messaging

Comments

@huyi1985
Copy link

When Google's servers reponse 502 for the Messaging Endpoint(https://fcm.googleapis.com/v1) requests, the response body has been read in advance and the RetryConfig can catch underlying network errors, as the following comment says,

// https://github.com/firebase/firebase-admin-go/blob/master/internal/http_client.go#L194
// ...
	// If a RetryConfig is available, always consult it to determine if the request should be retried
	// or not. Even if there was a network error, we may not want to retry the request based on the
	// RetryConfig that is in effect.
	if c.RetryConfig != nil {
		delay, retry := c.RetryConfig.retryDelay(retries, resp, result.Err)
		result.RetryAfter = delay
		result.Retry = retry
	}

But for the choice of most users, the default retry config WithDefaultRetryConfig only retries when http.StatusServiceUnavailable 503 occurred.

// https://github.com/firebase/firebase-admin-go/blob/master/internal/http_client.go#L82
func WithDefaultRetryConfig(hc *http.Client) *HTTPClient {
	twoMinutes := time.Duration(2) * time.Minute
	return &HTTPClient{
		Client: hc,
		RetryConfig: &RetryConfig{
			MaxRetries: 4,
			CheckForRetry: retryNetworkAndHTTPErrors(
				http.StatusServiceUnavailable,
			),
// ...

And it is true that Google often returns 502 for the Messaging Endpoint requests as shown below,

<!DOCTYPE html>
<html lang=en>
  <meta charset=utf-8>
  <meta name=viewport content="initial-scale=1, minimum-scale=1, width=device-width">
  <title>Error 502 (Server Error)!!1</title>
  <style>
    *{margin:0;padding:0}html,code{font:15px/22px arial,sans-serif}html{background:#fff;color:#222;padding:15px}body{margin:7% auto 0;max-width:390px;min-height:180px;padding:30px 0 15px}* > body{background:url(//www.google.com/images/errors/robot.png) 100% 5px no-repeat;padding-right:205px}p{margin:11px 0 22px;overflow:hidden}ins{color:#777;text-decoration:none}a img{border:0}@media screen and (max-width:772px){body{background:none;margin-top:0;max-width:none;padding-right:0}}#logo{background:url(//www.google.com/images/branding/googlelogo/1x/googlelogo_color_150x54dp.png) no-repeat;margin-left:-5px}@media only screen and (min-resolution:192dpi){#logo{background:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) no-repeat 0% 0%/100% 100%;-moz-border-image:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) 0}}@media only screen and (-webkit-min-device-pixel-ratio:2){#logo{background:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) no-repeat;-webkit-background-size:100% 100%}}#logo{display:inline-block;height:54px;width:150px}
  </style>
  <a href=//www.google.com/><span id=logo aria-label=Google></span></a>
  <p><b>502.</b> <ins>That’s an error.</ins>
  <p>The server encountered a temporary error and could not complete your request.<p>Please try again in 30 seconds.  <ins>That’s all we know.</ins>
@google-oss-bot
Copy link
Collaborator

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

@lahirumaramba
Copy link
Member

Hey @chong-shao do you think we should retry 502 errors?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chong-shao chong-shao

Labels
api: messaging
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@lahirumaramba @huyi1985 @google-oss-bot @chong-shao and others