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
Websocket shouldUpgrade() fail causes empty reply from server #3155
Comments
Without the |
Adding the -f and -v flags:
|
Due to vapor/vapor#3155, the server-mediated path won't return the correct error message when attempting to call someone who hasn't favorited you (although the call does fail correctly and doesn't ring the callee's phone). Works correctly in the direct-call path. Also, freaked out when I saw 5 "Sending incoming phonecall to callee." messages in a row. Turns out it's because I had 5 devices running logged in as that user. Still, this logging is better.
Due to vapor/vapor#3155, the server-mediated path won't return the correct error message when attempting to call someone who hasn't favorited you (although the call does fail correctly and doesn't ring the callee's phone). Works correctly in the direct-call path. Also, freaked out when I saw 5 "Sending incoming phonecall to callee." messages in a row. Turns out it's because I had 5 devices running logged in as that user. Still, this logging is better. Co-authored-by: Chall Fry <chall@challfry.com>
Ah ha, literally no response at all, not even an HTTP status line. My apologies for my partial misread of your original post! That is definitely incorrect behavior 😕. |
More information that's likely related. When calling a ws route, the middleware futures (or async completions; the part that runs on the way out of the middleware stack) all fire before the "shouldUpgrade" closure is called. Oddly, as the middlewares unwind they all receive a Response object with a 101 status. This means that e.g. ErrorMiddleware never sees an error response when shouldUpgrade() returns an error, because ErrorMiddleware exits before shouldUpgrade gets to run. Later, after shouldUpgrade throws an error or returns nil the future chain enters the .failure state, the Response object with the 101 status disappears, and eventually NIO closes the channel. I wish I could understand NIO better to figure out which part was where things go wrong, but alas. |
Describe the bug
When the shouldUpgrade() closure in a webSocket route returns nil or throws an error, Vapor returns a zero length response.
Expected: When an attempt to open a WebSocket fails the server should send a 400 or 500 level HTTP response.
To Reproduce
vapor new hello -n
.Expected behavior
I believe HTTP servers are always supposed to return at least a status line in responses. A zero-length response leaves clients no way to communicate why the WebSocket request failed.
Environment
Additional context
These curl commands use http:// instead of ws:// to make it easy to show the issue. Note that the success case opens the socket and gives the proper 101 response.
The text was updated successfully, but these errors were encountered: