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

Choose close code and reason on response #245

Closed
ErwanDL opened this issue Oct 19, 2021 · 3 comments · Fixed by #246
Closed

Choose close code and reason on response #245

ErwanDL opened this issue Oct 19, 2021 · 3 comments · Fixed by #246

Comments

@ErwanDL
Copy link

ErwanDL commented Oct 19, 2021

Currently, when the closing handshake is initiated by the other side of the WebSocket connection, we can't customize the CloseFrame of the response: it is automatically set to CloseFrame { code: CloseCode::Normal, reason: "".into() }, cf https://github.com/snapview/tungstenite-rs/blob/master/src/protocol/mod.rs#L562-L565.

This is problematic for my use case, where I would like to customize the CloseCode in the response (typically, I want the response to match the CloseCode of the closing request, so if the client closes with CloseCode::Library(4000) I want the server to respond with the same code).

Is there any way or workaround to make this work ?

@daniel-abramov
Copy link
Member

According to the RFC:

If an endpoint receives a Close frame and did not previously send a
Close frame, the endpoint MUST send a Close frame in response. (When
sending a Close frame in response, the endpoint typically echos the
status code it received.)

So perhaps we could echo the frame that we received when sending the response. I think that it would be indeed a better behavior and also should solve the problem you have.

@ErwanDL
Copy link
Author

ErwanDL commented Oct 20, 2021

That would be great, thank you! Do you know in what release this change will come out?

@daniel-abramov
Copy link
Member

Ok, so the changes have just been merged, so I think with the next release we'll include this change!

If you need it right now, you could for sure rely on the version from master for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants