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

fix(#100): Ensure correct connection closure #112

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

seanjb
Copy link

@seanjb seanjb commented Jun 25, 2024

The connection closure method sets the readyState to CLOSING and emits a close event to the socket, then sets the ready state to CLOSED.

However, due to a race condition, the readyState is already at the state of CLOSED when the socket close method is triggered. This causes an exception and a reconnection attempt. As reported in #100

This fix ensures the connectionTimer is cleared before emitting the close event, and updates the socket close handler to only throw an exception if the closure was unexpected (e.g. readyState is not CLOSED and the connectionTimer was not previously destroyed).

Tests added to ensure the new logic is closing the connection correctly. Removing the changes to the connection.ts method the tests correctly catch the closure exception and fail.

@seanjb seanjb requested a review from Bugs5382 as a code owner June 25, 2024 04:49
@seanjb seanjb force-pushed the fix-connection-closure branch from c58e20e to 8409326 Compare June 25, 2024 04:50
Copy link
Owner

@Bugs5382 Bugs5382 left a comment

Choose a reason for hiding this comment

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

lgtm

@Bugs5382 Bugs5382 self-assigned this Jun 25, 2024
@Bugs5382 Bugs5382 added bug Something isn't working enhancement New feature or request labels Jun 25, 2024
@Bugs5382 Bugs5382 merged commit 7be02c3 into Bugs5382:develop Jun 25, 2024
4 checks passed
Bugs5382 pushed a commit that referenced this pull request Jun 25, 2024
## [2.3.1-beta.1](v2.3.0...v2.3.1-beta.1) (2024-06-25)

### Bug Fixes

* **#100:** Ensure correct connection closure ([#112](#112)) ([7be02c3](7be02c3)), closes [#100](#100)
* **#100:** update closure check logic to avoid error when closed correctly ([8409326](8409326))
@Bugs5382
Copy link
Owner

🎉 This PR is included in version 2.3.1-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Bugs5382 pushed a commit that referenced this pull request Jun 25, 2024
## [2.3.1](v2.3.0...v2.3.1) (2024-06-25)

### Bug Fixes

* **#100:** Ensure correct connection closure ([#112](#112)) ([7be02c3](7be02c3)), closes [#100](#100)
* **#100:** update closure check logic to avoid error when closed correctly ([8409326](8409326))
@Bugs5382
Copy link
Owner

🎉 This PR is included in version 2.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Bugs5382 Bugs5382 mentioned this pull request Nov 2, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request released on @develop released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants