Ensure outbox messages are sent in the correct order #3272
Merged
Conversation
This file contains 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
borisyankov
force-pushed
the
send-in-order
branch
2 times, most recently
from
fd0e73b
to
041a8bd
Compare
Fixes zulip#3259. When sending more than one message with poor connectivity there is a likely and unwanted scenario that can happen: mesages sent early might fail to reach the server but later ones might succeed thus resulting in the incorrect order. Extracting the try-catch block outside the 'forEach' loop ensures that a single failed message will also fail the sending of later messages. Better retry strategy is coming in a separate commit soon.
Extract the message sending code out of `trySendMessages` as a separate function. Rename the action to `sendOutbox`. This is to facilitate retrying on failed send without reissuing the `toggleOutboxSending` actions. This also follows a well established convention for `try*` functions: * they handle an exception and do not throw * they return true/false depending on success/exception
Fixes zulip#3120. Try sending messages and keep retrying if sending fails. Using `progressiveTimeout` we make sure we are not trying too often. Thanks to `state.session.outboxSending` we are sure there will be no two loops at the same time.
Now that we are retrying on failed message send we are handling all possible cases where we might end up with unsent messages: * if message send fails immediately (we keep retrying) * if the app was closed with unsent messages (we try again on next launch, inside `doInitialFetch` -- and keep retrying) The connectivity change was a good heuristic with the imperfect retry strategy but is not needed any more.
gnprice
force-pushed
the
send-in-order
branch
from
041a8bd
to
400f4ac
Compare
|
Thanks @borisyankov ! Merged. |
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.
Fixes #3259
When sending more than one messages with poor connectivity there is
a likely an unwanted scenario that can happen: messages sent early might
fail to reach the server but later ones might succeed thus resulting in
the incorrect order.
Extracting the try-catch block outside the 'forEach' loop ensures that
a single failed message will also fail the sending of older messages.o
Fixes #3120
Try sending messages and keep retrying if sending fails.
Using
progressiveTimeoutwe make sure we are not trying too often.Thanks to
state.session.outboxSendingwe are sure there will beno two loops at the same time.