-
Notifications
You must be signed in to change notification settings - Fork 38.5k
Set reason for WebSocket CloseStatus.SESSION_NOT_RELIABLE
#29220
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
Set reason for WebSocket CloseStatus.SESSION_NOT_RELIABLE
#29220
Conversation
Hi @rstoyanchev , could you pls review it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, providing a reason would be useful. My one concern would be to expose information to external clients that reveals the server being used. In that sense the response status isn't meant to be for debugging but more of a hint for what happened. Perhaps we could choose a a fixed reason for this case that is a bit more helpful than just the status code?
Hi @rstoyanchev , do you mean the same reason for both send time limit exceeded AND the buffer size limit exceeded cases? Current impl contains the following possible reasons: Would reason like this be acceptable? It hides session ids, current config values but still hints possible issues. |
Something like that, perhaps we can say |
51c27da
to
af43d98
Compare
@rstoyanchev updated according to your suggestion |
Providing a reason in
CloseStatus
gives a better understanding of the root cause on the WebSocket client side.