Sender methods are not retrying on all scenarios

GoogleCodeExporter commented 9 years ago

What steps will reproduce the problem?
1. Face a JSON InternalServerError when calling send(Message, List<String>, int)

What is the expected output? What do you see instead?

It's expected that the Sender retries to send the messages in the same way when 
a JSON UnavailableError is received.

What version of the product are you using? On what operating system?

1.0.2 on Ubuntu 64bits

Please provide any additional information below.

Attached a patch to fix the issue, I'll add a git clone soon.

Original issue reported on code.google.com by gper...@ubikod.com on 4 Sep 2012 at 1:35

Attachments:

json_internal_server_error_retry.patch

GoogleCodeExporter commented 9 years ago

Fix available at 
https://code.google.com/r/gperrot-gcm/source/detail?r=1219eed29bad46a5817da15858
c8c15088459efb

Original comment by gper...@ubikod.com on 4 Sep 2012 at 1:55

GoogleCodeExporter commented 9 years ago

Sender should retry in the following scenarios (where it currently doesn't):
- error 500 on plain-text post
- IOException on plain-text post  
- InternalServerError error code on JSON post
- IOException on JSON post

We'll fix it for next release.

Original comment by felip...@android.com on 4 Sep 2012 at 4:44

Changed title: Sender methods are not retrying on all scenarios

GoogleCodeExporter commented 9 years ago

Hi,
Thanks for the review.
Please also consider the "Google Proxy" 502 error which tells us to retry in 
30s:

{{{
<!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}}
  </style>
  <a href=//www.google.com/><img src=//www.google.com/images/errors/logo_sm.gif alt=Google></a>
  <p>502. <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>
}}}

All kind of 50x errors should be processed the same way I guess (check for 
Retry-After header or use exponential back off if missing).

Original comment by gper...@ubikod.com on 7 Sep 2012 at 9:54

GoogleCodeExporter commented 9 years ago

Fixed on 
https://code.google.com/p/gcm/source/detail?r=86d3988d4cedbd5833c211043eb4616dbc
a2c768

Original comment by felip...@android.com on 14 Sep 2012 at 9:06

Changed state: Fixed

GoogleCodeExporter commented 9 years ago

4XX errors are also retried, like invalid TTL:

IOException on attempt 1 : HTTP Status Code: 400(Invalid value (2419201) for 
"time_to_live": must be between 0 and "2419200")
IOException on attempt 2 : HTTP Status Code: 400(Invalid value (2419201) for 
"time_to_live": must be between 0 and "2419200")

4XX should be fatal.

Original comment by gper...@ubikod.com on 4 Dec 2012 at 2:33

astropop / gcm

Sender methods are not retrying on all scenarios #9