Skip to content

Fix: Disconnect before reconnecting to prevent duplication #90

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

Merged
merged 2 commits into from
Oct 24, 2019

Conversation

dmsnell
Copy link
Contributor

@dmsnell dmsnell commented Oct 24, 2019

In Automattic/simplenote-electron we noticed that after signing out of
the application and signing in again we were getting infinite
duplication of changes. This occurred until we quit the app and
restarted it.

We discovered that when signing back in the Simperium client library was
opening a new WebSocket connection without closing the old one. Whenever
the server sent back changes the client got them twice and tried to
update the content twice, which then cascaded duplication without end.

In this patch we're closing the existing connection if it's already open
when we call connect(). This probably fixes defects we haven't
associated with this change.

Testing

  1. Checkout this branch
  2. Run npm run prepare && npm link
  3. Clone Automattic/simplenote-electron
  4. In that directory run npm link simperium && npm run dev

With this branch npm linked when you logout/login and type something in a note the app should enter your changes and things will work as expected.

Without this branch when you logout/login and type something then you should see after a short delay that you change replays infinitely and duplicates itself.

In Automattic/simplenote-electron we noticed that after signing out of
the application and signing in again we were getting infinite
duplication of changes. This occurred until we quit the app and
restarted it.

We discovered that when signing back in the Simperium client library was
opening a new WebSocket connection without closing the old one. Whenever
the server sent back changes the client got them twice and tried to
update the content twice, which then cascaded duplication without end.

In this patch we're closing the existing connection if it's already open
when we call `connect()`. This probably fixes defects we haven't
associated with this change.
Copy link
Contributor

@belcherj belcherj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works! Code looks good!

Copy link

@codebykat codebykat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the issue for me!!!

@dmsnell dmsnell merged commit e3822e2 into master Oct 24, 2019
@dmsnell dmsnell deleted the fix/disconnect-before-connect branch October 24, 2019 17:48
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.

3 participants