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(WebSocketSubject): handle slow WebSocket close #6708

Merged
merged 1 commit into from Dec 6, 2021

Conversation

maknapp
Copy link
Contributor

@maknapp maknapp commented Dec 6, 2021

Description:
The close event on WebSocket does not always occur immediately after running the close function. If the close event is occurs after a new WebSocket is opened, then the reset needs to be skipped. This checks to make sure the socket being reset is the one that matches the event.

The MockWebSocket class skipped the state between CLOSING and CLOSED. I removed the automatic closing so that the CLOSED state must be trigger manually. This caused many of the existing tests to expect CLOSING instead - I changed the codes instead of triggering close because it did not affect the tests.

I also changed the readyState codes to use an enum for clarity. If that is an unwanted change I can revert back to comments.

Related issues:
Closes #4650, #3935

The close event on WebSocket does not always occur immediately after
running the close function. If the close event is occurs after a new
WebSocket is opened, then the reset needs to be skipped. This checks to
make sure the socket being reset is the one that matches the event.

Closes ReactiveX#4650, ReactiveX#3935
benlesh
benlesh approved these changes Dec 6, 2021
@benlesh
Copy link
Member

@benlesh benlesh commented Dec 6, 2021

This seems like a reasonable fix. Thank you for submitting the PR @maknapp. It seems this has been an issue for a while. The WebSocketSubject can definitely use some love. This is appreciated.

@benlesh benlesh added 6.x 7.x 8.x labels Dec 6, 2021
@benlesh benlesh merged commit 8cb201c into ReactiveX:master Dec 6, 2021
5 checks passed
@benlesh benlesh removed the 6.x label Dec 6, 2021
benlesh pushed a commit that referenced this issue Dec 6, 2021
The close event on WebSocket does not always occur immediately after
running the close function. If the close event is occurs after a new
WebSocket is opened, then the reset needs to be skipped. This checks to
make sure the socket being reset is the one that matches the event.

Closes #4650, #3935

Co-authored-by: Mark Knapp <mknapp@leightronix.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.x 8.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants