Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync robustness fixes #302

Closed
wants to merge 5 commits into from
Closed

Sync robustness fixes #302

wants to merge 5 commits into from

Conversation

SimonWoolf
Copy link
Member

Fixes #300 . Will expand that ticket and do prs for lib version qs and trans pref tomorrow.

As it's a failure before the sync has actually started, can just do
abandon() and continue on the old transport, which we can't do for
actual sync failures.

Also regularise the connection state check, was missing an edge-case
(connecting)
…ransports

If there's been some problem with syncing, we have a problem -- we
can't just fall back on the old transport, as we don't know whether
realtime got the sync -- if it did, the old transport is no longer
valid. To be safe, we disconnect both and start again from scratch.
@paddybyers
Copy link
Member

Re: 9c5295a

in realtime we have an EventEmitter.timedOnce() for things like this. It would be nice for it to be a bit more generic.

Otherwise LGTM.

@mattheworiordan
Copy link
Member

Awesome 👍

@SimonWoolf
Copy link
Member Author

in realtime we have an EventEmitter.timedOnce() for things like this

good idea, but should probably convert all one-off timers at once rather than just this one -- added a refactoring issue #306

SimonWoolf added a commit that referenced this pull request Jul 6, 2016
@SimonWoolf
Copy link
Member Author

Merged as 8a3ff82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants