Skip to content

Conversation

@bever1337
Copy link

Description:

When the output observer is closed because of de-serialization errors, ensure the WebSocketSubject is reset and the WebSocket is closed.

Related issue (if exists): (#5312)

@bever1337
Copy link
Author

bever1337 commented Sep 30, 2022

I don't believe there's any marble testing to be done since this work is making native features observable. In my experience, there is no equivalent framework like msw for mocking and testing websockets. This is my first contribution to RxJS, and I would appreciate anyone's time ensuring my above statement is accurate. Additionally, are there other changes that need to be made in RxJS to support this? Other tests? Docs? Thank you

@bever1337 bever1337 changed the title fix(WebSocketSubject): Closes socket when closing socket subject obse… fix(WebSocketSubject): Closes socket when closing socket observer Sep 30, 2022
@bever1337
Copy link
Author

bever1337 commented Sep 30, 2022

I need to research further, but I think the multiplexer can also create the same state where the websocket subject has errored but the underlying socket remains open. I don't want to conflate issues and the multiplexer code is trickier to change because it has more error paths.

@maknapp
Copy link
Contributor

maknapp commented Oct 10, 2022

There is a MockWebSocket in webSocket-spec.ts to write tests with. There seems to be only one test for deserializer errors, and it does not check for ws.close. There are certainly more tests to add in this area.

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