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

Rxjs and Error on unsubscribe -> CloseEvent has to be wasClean = true #287

Closed
meriturva opened this issue Nov 26, 2019 · 3 comments · Fixed by #338
Closed

Rxjs and Error on unsubscribe -> CloseEvent has to be wasClean = true #287

meriturva opened this issue Nov 26, 2019 · 3 comments · Fixed by #338

Comments

@meriturva
Copy link

I have a really simple stackblitz to reproduce issue:

https://stackblitz.com/edit/rxjs-qxerxt

You could see that we have two different rxjs tests, The first one we unsubscribe from WebSocket but we just receive a CloseEvent error with code = 0 and wasClean = false.

On the second example, we just don't unsubscribe from rxjs WebSocket subject so jasmine just goes in timeout error.

What i have to say that the test just works correctly on real WebSocket server, so we just receive errors using your library.
Secondly using a classic approach based on MockWebSocket everything is correct. See: https://github.com/ReactiveX/rxjs/blob/master/spec/observables/dom/webSocket-spec.ts#L711

Do I miss something? Why there is no clean close when client just closes connection?

@meriturva meriturva changed the title Error on close -> i guess it has to be clean = true Rxjs and Error on unsubscribe -> CloseEvent has to be wasClean = true Nov 26, 2019
@DanHarman
Copy link

I've looked into this issue and it's because the createCloseEvent function breaks convention and treats the absence of a code on close() as wasClean= false. RxJs WebSocketSubject does not set the code on close, and when it gets a wasClean=false closeEvent it causes it to invoke error.

This is tough to fix as this project doesn't seem to be actively maintained, although it's really a 1 liner...

To be fair to everyone the webstandard is extremely vague as to the definition of wasClean.

Atrue added a commit that referenced this issue Oct 18, 2021
@Atrue
Copy link
Collaborator

Atrue commented Oct 18, 2021

Fixed in 9.0.6

@DanHarman
Copy link

Thanks for fixing. I would have happily made a PR after doing the analysis but wasn't convinced PRs were getting merged!

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 a pull request may close this issue.

3 participants