search

akka / akka-http

The Streaming-first HTTP server/module of Akka
https://doc.akka.io/docs/akka-http
Other
1.34k stars 594 forks source link

WebSocket server never send back close connection #1358

Open buster84 opened 7 years ago

buster84 commented 7 years ago

Hi, I found a problem that WebSocket server never send back WebSocket Close Connection when server output is infinite. Some WS libraries can't close connection because of this problem(At least SocketRocket and jetfire can't close connection).

I checked InfinitOutputServer.scala never send back Close connection and InfinitOutputServer.js do send back close connection using wireshark.

InfinitOutputServer.scala result is: screen shot 2017-08-15 at 18 45 23

And InfinitOutputServer.js result is: screen shot 2017-08-15 at 18 46 17

rfc6455 says:

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.) It SHOULD do so as soon as practical. An endpoint MAY delay sending a Close frame until its current message is sent (for instance, if the majority of a fragmented message is already sent, an endpoint MAY send the remaining fragments before sending a Close frame). However, there is no guarantee that the endpoint that has already sent a Close frame will continue to process data.

I guess current behavior breaks RFC because of the endpoint MUST send a Close frame in response..

Problematic code is here

raboof commented 6 years ago

@buster84 I wonder if using Flow.fromSinkAndSourceCoupled instead of Flow.fromSinkAndSource would have produced the desired behavior for your example? If so, perhaps we should call that out in the documentation in a couple of places and perhaps include an example? If not, then I agree with you and @jrudolph that it would probably make sense to make it possible to switch this behavior from configuration, and starting by introducing the switch and then considering swapping the default.

buster84 commented 6 years ago

screen shot 2018-02-20 at 11 24 07

@raboof Thanks!! I didn't know that flow. The flow works as we expect. I agree with mentioning about this in the documentation. Also we should provide public api which use Flow.fromSinkAndSourceCoupled . Currently we only provide handleMessagesWithSinkSource which use Flow.fromSinkAndSource. https://github.com/akka/akka-http/blob/v10.1.0-RC2/akka-http-core/src/main/scala/akka/http/scaladsl/model/ws/UpgradeToWebSocket.scala#L41-L55

raboof commented 6 years ago

@buster84 that sounds reasonable, would you be interested in PR'ing that?

buster84 commented 6 years ago

@raboof Thank you for the response. I will send PR for adding new api which use Flow.fromSinkAndSourceCoupled but not for documentation because I am not native English speaker so I think someone not me is suitable to write the document.

ktoso commented 6 years ago

None of us are native english speakers ;-) Please give it a try and we'll help out! :-) Thanks in advance

buster84 commented 6 years ago

Okay, thanks. I will try to write the document in this weekend.