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

Emit error on failed resubscribe #174

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gkubisa
Copy link
Contributor

@gkubisa gkubisa commented Oct 17, 2017

Normally when a subscribe callback is missing, an error event is generated on error, except for the following scenario addressed by this PR:

  1. state === 'connecting'
  2. subscribe() - can't subscribe in this state
  3. state === 'connected'
  4. _resubscribe()
  5. Backend returns an error

Expected: doc.emit('error', error)
Actual: no error event

@coveralls
Copy link

coveralls commented Oct 17, 2017

Coverage Status

Coverage increased (+0.4%) to 96.497% when pulling e97489c on Teamwork:emit-error-on-failed-resubscribe into c0e2fee on share:master.

@gkubisa
Copy link
Contributor Author

gkubisa commented Jun 11, 2018

@nateps @ericyhwang , could you provide some feedback on this PR, or merge it, if it's all good, please?

@ericyhwang
Copy link
Contributor

ericyhwang commented Jul 11, 2018

Nate's comments, from the PR review meeting:

  • The state machine on reconnecting is interesting.
  • I'll have to look at the rest of the code to check for consistency.

@curran
Copy link
Contributor

curran commented Dec 21, 2019

Suggest to close without merging due to lack of activity.

@curran curran mentioned this pull request Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants