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

Don't close socket if preserveConnection is set in HocuspocusProvider #844

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

douira
Copy link

@douira douira commented Jul 8, 2024

Sending the CloseMessage also has the effect of closing the socket according to its description.

I noticed that before patching this in the package, the socket would be closed when I called destroy on the provider even if it was supposed to preserve the connection. When using multiple providers on the same socket at once, it's important that no provider getting destroyed closes the whole socket. disconnecting the provider is not enough because it still has listeners on the document, socket, and elsewhere which is not acceptable.

douira and others added 2 commits July 8, 2024 02:14
Sending the CloseMessage also has the effect of closing the socket according to its description.
@douira
Copy link
Author

douira commented Jul 11, 2024

The test could be fixed by either

  • setting preserveConnection to false in the test (it's true by default),
  • or changing the default to false,
  • or changing how many open connections the test expects,
  • or adding a socket.destroy() to the test

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.

2 participants