Pass along CONNECTED errors #291
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
SimonWoolf
force-pushed
the
connected-errors
branch
from
402bb0c to
966bb80
Compare
This was referenced Jun 22, 2016
Merged
| test.equal(stateChange.reason.code, 80008, "verify unrecoverable-connection error set in stateChange.reason"); | ||
| test.equal(realtime.connection.errorReason.code, 80008, "verify unrecoverable-connection error set in connection.errorReason"); | ||
| test.equal(realtime.connection.serial, -1, "verify serial is -1 (new connection), not 5"); | ||
| test.equal(realtime.connection.key.indexOf('aaaaaaaaaaaaaaaa'), -1, "verify connection using a new connectionkey"); |
There was a problem hiding this comment.
I don't understand the aaaaaaaaaaaaaa
There was a problem hiding this comment.
argh oops. Right before I put this up I decided to change the fake recovery key from aaaaa!aaaaaaaaaaaaaaaa-aaaaaaa:5 to _____!ablyjs_test_fake-key____:5 (to make it obvious to anyone looking in the realtime sandbox logs and seeing failed resumes what was going on). Forgot to change that line :(
|
LGTM except for one comment |
…erial Resetting the message serial is a bit of a hack - fixes the problem for now but may change in the future depending on the conclusion of ably/realtime#587
SimonWoolf
force-pushed
the
connected-errors
branch
from
ea266cb to
c42255d
Compare
SimonWoolf
added a commit
that referenced
this pull request
Jun 22, 2016
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.
Was putting this off until the untilAttach, but pulled forward due to https://github.com/ably/realtime/issues/589
For connected errors in upgrades, I did the workaround described in https://github.com/ably/realtime/issues/587#issuecomment-227158648 - ie syncing with the new (-1) connectionSerial after a failed upgrade. This is a bit hacky, but fixes the problem from that issue for now. If we decide to change realtime behaviour (eg to not require a sync in the case of a failed upgrade), then we can change it then.