apns - handling error from previous batch#774
Open
denisborovnev wants to merge 3 commits intoRedth:masterfrom
Open
apns - handling error from previous batch#774denisborovnev wants to merge 3 commits intoRedth:masterfrom
denisborovnev wants to merge 3 commits intoRedth:masterfrom
Conversation
when response wasn't read for previous batch (due to not enough timeout) and for current batch we read error from previous batch. As a result we can't find failed notification in current batch
removed TODO
Owner
|
@denisborovnev Is this working well for you in production? I do think we should use the existing MillisecondsToWaitBeforeDeclaredSuccess as the value. If you can update your PR I'm happy to merge this. Thanks :) |
ResponseWaitTimeout see Redth#774 (comment)
Author
|
Hi @Redth !
We haven't released yet, so can't give answer right now :) But we have already tested and haven't faced problem when server stops send any notification for some period of time. |
|
Hi @Redth. We have that fix in production, working well, no issues yet. Could you please merge it? |
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.
Sometimes 750ms is not enough to get response from apns when there is an error in batch
In this case all messages in batch marked as sent and connection is reused for new batches.
During sending next batch error is read, but there is no failed notification in current batch and reader does nothing
As a result notifications from this batch are not marked as sent or failed. Also next batches won't be sent because "error" connection is not closed and ApnsConnection instance continues to use it.
To fix this problem:
PS: there is already setting called MillisecondsToWaitBeforeMessageDeclaredSuccess, not sure if it might be used instead of new one ResponseWaitTimeout