V1 no send no retry #248
V1 no send no retry #248
Conversation
This required some major changes to how the tests are structured, as I didnt want to be listening on what used to be "sendNoRetry". requestStub is now a sinon spy. it is reset before EVERY test (woops, before we only cleared state before each test CATEGORY) and how we expect some things has changed as a result of these structural changes
They used the registration_ids key, which we will no longer support. They passed because state was NOT reset before every test (fixed this in earlier commit)
|
@hypesystem excellent work! Tested it and works fine. One minor thing, the package doesn't detect an invalid registration token currently if GCM returned Line 155 in 7e55699 Is this intentional? All tests pass, by the way. Merge when ready. |
|
This is not something I have made a deliberate choice about now. It is how it was before... I think the reasoning is that only unavailable registration tokens should be retried --- if the registration token is invalid, there is no need to retry. In the returned result, the invalid registration ids should still be present 😄 we don't have a test for it, but I definitely think we could add one. I might do that later ! |
|
@hypesystem I'm thinking the package shouldn't be retrying an |
|
Absolutely! That's basically under the point of #238 that is about better responses 😄 gonna merge this now and will look at the responses later |
|
@hypesystem and @eladnava have doubt, what is the use of what we are going to achieve with |
|
@govardhanraoganji the GCM endpoint is bound to fail sometimes (due to an internal server error, for example) and so the GCM request should be retried in this scenario as it will likely succeed next time. We do not retry a request that has failed due to user error, e.g. bad authentication, bad registration token, etc. |
|
@govardhanraoganji we also retry the message for any registration tokens for which it was not delivered. Just in case it was a momentary outage. |
|
@eladnava and @hypesystem thank you guys, your support was un countable. |
Note: this merges into v1 #238
This refactors the sender code a whole bunch, and removes the previously exposed
sendNoRetrymethod. The same effect can now be achieved withsend(msg, recipient, { retries: 0 }, callback).