najamelan / ws_stream_tungstenite

Provide AsyncRead/AsyncWrite over tungstenite websockets
32 stars 16 forks source link

websocket close code #6

Open databasedav opened 3 years ago

databasedav commented 3 years ago

calling sink.close() sends the client a 1005 close code even though it is correctly initiating the close handshake (waiting for the client to send back a close frame), so shouldn't this be 1000? is there anyway to set this or is it buried deep within tungstenite? thanks for this library :)

najamelan commented 3 years ago

So 1005 means "no close status received", which is as I can tell correct if you don't give any close status and reason. It should not be considered an error and is often equivalent to 1000. The Sink API doesn't allow you to give any extra information, however if you want to close normal, you can create a tungstenite::Message::Close with your code and reason, which can be 1000 and send it into the sink instead of calling close.

Was this situation causing specific issues for you?

databasedav commented 3 years ago

not a huge issue or anything. according to this page https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent it Indicates that no status code was provided even though one was expected., which seems to allude to some sort of error/mishap, contrasted with the "normalness" of 1000 (e.g. a server initiated close handshake), which the client i'm using conforms to with different error handling for each.

how can we send a tungstenite::Message::Close if we've set the sink to take Bytes?

najamelan commented 3 years ago

I took some time to look into it. So the default one comes from async-tungstenite. When you call Sink::close we just forward it to inner layers. So if you want the default close code to change, you should ask async-tungstenite to change that.

As to manually sending it, I had a look and indeed TungWebsocket is a Sink<Item=Vec<u8>>, not a Sink<Item=tungstenite::Message>. Thus, it's not currently possible. In principle you'd do this by calling into_inner until you have the correct layer, but this method isn't implemented in ws_stream_tungstenite right now. I think it would be a good thing to implement that.

Depending on whether you actually end up needing it, I could speed that up, if not I'll put it on the todo list but it won't be hight priority.

databasedav commented 3 years ago

no hurry at all, thanks for looking into it