Handle Close Codes

[jeregrine]

Currently Bandit and this Cowboy adapter, only allow for sending of "WebSocket Close" codes 1000 and 1002. Or in the case of cowboy adapter they drop/ignore close codes entirely.

The WebSocket specification allows any code from with registered values here. As well as a short UTF8 Binary message along with close codes.

I previously did some work to handle the case where the user is force disconnected by sending a status code 3000, which is registered unauthorized. This allowed the socket.js to not retry connections if they are booted by the server purposely. While maintaining some level of backwards of compatibility.

I don't see any reason why would should limit to 1000 and 1002. With cowboy we could handle the reply {:stop, {:close, CODE, REASON}, state} correctly. In Bandit it looks like this is a known thing but it is simply not being used by the websocket close function.

I Post all this because maybe I'm missing something?

Why limit the close codes? Why drop/ignore close messages? I can do the work to enable it on your end and phoenix's end but I wanted to gut check myself.

[mtrudel]

I was just looking at phoenixframework/phoenix#5024!

I deliberately didn't accommodate sending close frames in WebSock because I didn't see a clear way to expose it, but now that the dust has settled I think I see a way forward that's really easy to land:

• Add a clauses to t:WebSock.handle_result that looks like {:stop, reason, code | {code, message}, state}
• Support this in WebSockAdapter.CowboyAdapter.handle_reply (I'm unclear whether Cowboy will automatically close the connection if you return a {:reply, {:close...}, state} tuple; otherwise we'd need to get fancy here with messaging ourselves or the like).
• Support this in Bandit by adding a clause to Bandit.WebSocket.Connection.handle_continuation very much like the existing {:stop,....} tuple, making sure that the semantics are consistent with those of Cowboy
• Document whatever those semantics are in WebSock (so long as these semantics are reasonable and consistent between implementations I'm sure they're fine).

Happy to accept PRs for this! I'd suspect it to be three PRs, one each for websock, websock_adapter, and bandit.

[Gazler]

Just to confirm on the note about Cowboy, it does automatically close the websocket if a :close reply is sent.

https://github.com/ninenines/cowboy/blob/30ee75cea14c1fd63a46093cfb2bb9ad3f844f75/src/cowboy_websocket.erl#L506

[mtrudel]

This functionality is ready for review on Cowboy at the PRs linked above.

Next step is to get the websock PR reviewed & landed.

Once that's done, I'll cut a websock release and update the websock_adapter PR and Bandit PR to point at it.

[jeregrine]

Awesome!

[mtrudel]

All work for this is done & merged. This will now work for websock_adapter >= 0.5.0. You can update this dependency locally within any recent phoenix app & have access as needed.

[mtrudel]

Just to confirm on the note about Cowboy, it does automatically close the websocket if a :close reply is sent.

https://github.com/ninenines/cowboy/blob/30ee75cea14c1fd63a46093cfb2bb9ad3f844f75/src/cowboy_websocket.erl#L506

I just found explicit validation of this in Cowboy's documentation at the very bottom of https://ninenines.eu/docs/en/cowboy/2.6/guide/ws_handlers/