Don't emit ERROR until a keepalive poke fails #223
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
This accomplishes the same as https://github.com/matrix-org/matrix-js-sdk/pull/216/files, but without the client waiting 110 seconds for a sync request to time out. That is, don't display an error message a soon as a sync request fails, since we should accept that sometimes long lived HTTP connections will go away and that's fine. Also: * Use request rather than deprecated requestWithPrefix * http-api: The empty string may be falsy, but it's a valid prefix
* Go back to previous behaviour of continuing to emit ERROR states if it continues to fail * Don't set a timer if the timeout is zero * Change test to assert the continued-error behaviour, not exactly multiple syncs failing * Update other tests to fail the keepalive requests where appropriate
richvdh
reviewed
Oct 7, 2016
| // fails, since long lived HTTP connections will | ||
| // go away sometimes and we shouldn't treat this as | ||
| // erroneous. | ||
| // We sent the first keep alive immediately because |
richvdh
approved these changes
Oct 7, 2016
Changed my mind - it's a good idea to wait a short time before sending a keepalive request: this will make sure we never tightloop. This also adds a 'RECONNECTING' state for when a sync request has failed but there is no reason to suspect there is actually a connectivity problem. This is necessary for the tests to be able to advance the clock at the appropriate time, but could be nice for clients. Add a breaking change changelog entry since technically this is an API change that might break clients if they were relying on 'SYNCING' always coming before 'ERROR' for some reason.
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.
This accomplishes the same as
https://github.com/matrix-org/matrix-js-sdk/pull/216/files, but
without the client waiting 110 seconds for a sync request to time
out. That is, don't display an error message a soon as a sync
request fails, since we should accept that sometimes long lived
HTTP connections will go away and that's fine.
Also: