dream icon indicating copy to clipboard operation
dream copied to clipboard

How to get notified of websockets that close

Open tcoopman opened this issue 3 years ago • 10 comments

When a websocket connection closes I often see:

03.07.21 22:35:58.492      dream.http  WARN WebSocket (127.0.0.1:33826): Unix.Unix_error(Unix.ECONNRESE)
03.07.21 22:35:58.492      dream.http  WARN Raised by primitive operation at Lwt_unix.shutdown in file 8
03.07.21 22:35:58.492      dream.http  WARN Called from Gluten_lwt_unix.Io.close.(fun) in file "src/ven7
03.07.21 22:35:58.492      dream.http  WARN Called from Lwt.Sequential_composition.catch in file "src/c0

But I cannot react to that. I only know that a connection is not there anymore when I try to send something to it and that fails. I should explore this a bit more, but it seems that Dream.receive does not always get a None when the connection is closed.

tcoopman avatar Jul 03 '21 20:07 tcoopman

But I cannot react to that.

You should be able to catch the rejection of Dream.send with ECONNRESET.

I should explore this a bit more, but it seems that Dream.receive does not always get a None when the connection is closed.

This issue seems to be about what happens when you call Dream.send on a socket that has been closed by the client, rather than what happens when you call Dream.receive on it. Dream.send needs some way to tell you that the socket has been closed by the other side. ECONNRESET is the standard "systems programming" way of doing that.

I can agree, however, that it's not a very ergonomic interface, and perhaps we can think of something better. However

  • We do need to tell the sending code that the send failed.
  • We need then some other "centralized" way to communicate closing of the socket by the client.

aantron avatar Jul 04 '21 06:07 aantron

  • We do need to tell the sending code that the send failed.

I guess it's also possible to never resolve the promise that comes from Dream.send, relying on the client handling some kind of other "remote close" notification.

aantron avatar Jul 04 '21 06:07 aantron

I have this code:

let send client client_id payload =
  try%lwt Dream.send client payload with
  | e ->
      Stdlib.print_endline (Exn.to_string e) ;
      log.error (fun log -> log "failed to send to client: %i" client_id) ;
      Lwt.return_unit

And sometimes my log only prints:

04.07.21 14:00:59.083      dream.http  WARN WebSocket (127.0.0.1:33880): Unix.Unix_error(Unix.ECONNRESE)
04.07.21 14:00:59.083      dream.http  WARN Raised by primitive operation at Lwt_unix.shutdown in file 8
04.07.21 14:00:59.083      dream.http  WARN Called from Gluten_lwt_unix.Io.close.(fun) in file "src/ven7
04.07.21 14:00:59.083      dream.http  WARN Called from Lwt.Sequential_composition.catch in file "src/c0

and sometimes only:

(Failure "cannot write to closed writer")
04.07.21 14:01:38.647  dream.channels ERROR REQ 2 failed to send to client: 0

So I have the feeling that the try%lwt not always catches the exception, but I guess that would be very weird, so it's probably something in my code?

(the reason for the Stdlib.print_endline was because I didn't always see the log.error but it does work, so I can remove it again)

Anyway why not change Dream.send to return an Lwt_result.t and also offer Dream.send_exn?

tcoopman avatar Jul 04 '21 12:07 tcoopman

For now, I'll wrap Dream.send to return a result.

I have 2 remaining questions though:

  1. Can Dream.send fail for an other reason than that the client is disconnected?
  2. Can dream detect a broken websocket when there is nothing being send/received. So say a clients internet connection drops and nothing is send to that client?

tcoopman avatar Jul 04 '21 18:07 tcoopman

  1. Can Dream.send fail for an other reason than that the client is disconnected?

For example, if you do Dream.send before the previous Dream.send's promise has resolved.

2. Can dream detect a broken websocket when there is nothing being send/received. So say a clients internet connection drops and nothing is send to that client?

I don't believe so. In general, the assumption is that you'd always be either sending or receiving on a WebSocket. It's the same API as for systems sockets, but we probably shouldn't duplicate it verbatim since we don't have to, for the reasons your question is implying :)

aantron avatar Jul 04 '21 18:07 aantron

For example, if you do Dream.send before the previous Dream.send's promise has resolved.

Good to know! (This would be nice if this was added to the documentation / or a result with some self documenting type)

I don't believe so. In general, the assumption is that you'd always be either sending or receiving on a WebSocket. It's the same API as for systems sockets, but we probably shouldn't duplicate it verbatim since we don't have to, for the reasons your question is implying :)

So it's totally possible that broken connections would linger forever if you never send to them? That doesn't seem right? Shouldn't we need some mechanism to clean these up?

For example, using a slightly adapted w-chat example but connecting with a node client (https://github.com/websockets/ws/blob/master/doc/ws.md#websocketterminate.) When I create multiple ws clients and call ws.terminate() node will destroy the socket, but this is not always received by dream.

let send_exn client payload =
  try%lwt Dream.send client payload with
  | error ->
      let error = Printexc.to_string error in
      log.error (fun log -> log "failed to send to client - error: %s" error) ;
      Lwt.return_unit

let send message =
  Hashtbl.to_seq_values clients
  |> List.of_seq
  |> Lwt_list.iter_p (fun client -> send_exn client message)

let handle_client websocket =
  let client_id = connect websocket in
  let rec loop () =
    match%lwt Dream.receive websocket with
    | Some message ->
      let%lwt () = send message in
      loop ()
    | None ->
      log.info (fun log -> log "disconnection %i" client_id) ;
      disconnect client_id;
      Dream.close_websocket websocket
...
// starting ws1 to ws3 and sending 1 message
ws1.terminate();
ws2.terminate();
ws3.terminate();

I get this output:

04.07.21 21:24:06.679                       Running on 0.0.0.0:8080 (http://localhost:8080)
04.07.21 21:24:06.679                       Type Ctrl+C to stop
04.07.21 21:24:09.174       dream.log  INFO REQ 1 GET /ws 127.0.0.1:34640
04.07.21 21:24:09.174       dream.log  INFO REQ 1 101 in 50 μs
04.07.21 21:24:09.174       dream.log  INFO REQ 2 GET /ws 127.0.0.1:34646
04.07.21 21:24:09.174       dream.log  INFO REQ 2 101 in 21 μs
04.07.21 21:24:09.174       dream.log  INFO REQ 3 GET /ws 127.0.0.1:34642
04.07.21 21:24:09.174       dream.log  INFO REQ 3 101 in 18 μs
04.07.21 21:24:09.177      dream.http  WARN WebSocket (127.0.0.1:34640): Unix.Unix_error(Unix.ECONNRESET, "read", "")
04.07.21 21:24:09.177      dream.http  WARN Raised by primitive operation at Lwt_unix.shutdown in file "src/unix/lwt8
04.07.21 21:24:09.177      dream.http  WARN Called from Gluten_lwt_unix.Io.close.(fun) in file "src/vendor/gluten/lw7
04.07.21 21:24:09.177      dream.http  WARN Called from Lwt.Sequential_composition.catch in file "src/core/lwt.ml", 0
04.07.21 21:24:09.177      dream.http  WARN WebSocket (127.0.0.1:34646): Unix.Unix_error(Unix.ECONNRESET, "read", "")
04.07.21 21:24:09.177      dream.http  WARN Raised by primitive operation at Lwt_unix.shutdown in file "src/unix/lwt8
04.07.21 21:24:09.177      dream.http  WARN Called from Gluten_lwt_unix.Io.close.(fun) in file "src/vendor/gluten/lw7
04.07.21 21:24:09.177      dream.http  WARN Called from Lwt.Sequential_composition.catch in file "src/core/lwt.ml", 0
04.07.21 21:24:09.177      dream.http  WARN WebSocket (127.0.0.1:34642): Unix.Unix_error(Unix.ECONNRESET, "read", "")
04.07.21 21:24:09.177      dream.http  WARN Raised by primitive operation at Lwt_unix.shutdown in file "src/unix/lwt8
04.07.21 21:24:09.177      dream.http  WARN Called from Gluten_lwt_unix.Io.close.(fun) in file "src/vendor/gluten/lw7
04.07.21 21:24:09.177      dream.http  WARN Called from Lwt.Sequential_composition.catch in file "src/core/lwt.ml", 0

As you can see, I get no disconnection whatsoever here, sometimes I do get 1 or 2 disconnections, but I have never seen 3. I'm not sure why a broken connection is sometimes detected, and sometimes not, but I guess that has more to do with TCP than with dream.

tcoopman avatar Jul 04 '21 19:07 tcoopman

Phoenix has a ping/pong (https://hexdocs.pm/phoenix_gen_socket_client/Phoenix.Channels.GenSocketClient.Transport.WebSocketClient.html) but it requires client support of course.

I'm working (very WIP - not public yet) on dream-channels - Phoenix like channels for dream and one of my goals is to have a client library as well.

Maybe I could add it in there? Any thoughts

tcoopman avatar Jul 04 '21 19:07 tcoopman

I'll have to take a very close look at this as I'm doing the I/O internals rework, which I intend for the next alpha release.

Phoenix has a ping/pong (https://hexdocs.pm/phoenix_gen_socket_client/Phoenix.Channels.GenSocketClient.Transport.WebSocketClient.html) but it requires client support of course.

I'm working (very WIP - not public yet) on dream-channels - Phoenix like channels for dream and one of my goals is to have a client library as well.

Maybe I could add it in there? Any thoughts

WebSockets already have their own low-level ping-pong built into the protocol, and Dream already responds to the client's WebSocket pings, but doesn't send its own pings:

https://github.com/aantron/dream/blob/7e4dae59fd17922c29c41df3c323b81454a2eed0/src/http/http.ml#L88-L89

I suspect (but don't know, could be way off) Phoenix has higher-level ping-pong (if it does) because it can use non-WebSocket transports for channels (is that still/was ever true?). IIRC Phoenix could use server-sent events and maybe other methods to get channel behavior. Given that, it makes sense that Phoenix would implement a Phoenix-level ping-pong, since SSE doesn't have anything like that built into the "protocol."

Perhaps one thing we need is for Dream to send low-level pings to WebSocket clients, probably with some optionals for configuration.

aantron avatar Jul 05 '21 12:07 aantron

Here's the Dream server-sent events example in action by the way, but I don't think we should rely on this. I suspect (again, could be way off) that Phoenix supports SSE as a transport for channels because Phoenix channels were written at a time when WebSockets were still very new, and not that well supported yet. We can probably just use only WebSockets at this point.

aantron avatar Jul 05 '21 12:07 aantron

Phoenix supports SSE as a transport for channels because Phoenix channels were written at a time when WebSockets were still very new, and not that well supported yet

I believe that's true, Phoenix has a fallback for longpolling for example.

We can probably just use only WebSockets at this point.

Agree!

WebSockets already have their own low-level ping-pong built into the protocol, and Dream already responds to the client's WebSocket pings, but doesn't send its own pongs:

awesome, didn't know that. I need to start looking into the websocket protocol more :-)

tcoopman avatar Jul 05 '21 19:07 tcoopman