Incorrect handling of `Connection_close` can result in an endless loop

[tcoopman]

I'm not saying this is a bug, maybe just something to put a warning in the docs.

When you receive a Connection_close on a continuously-running handler but don't return Lwt.return_unit then the handler will be stuck in an infinite loop with no way to recover from.

Depending on how you structured your code, this will also result in a cpu spike because pull will always immediately return with Empty.

Example of a wrong code with high cpu usage:

  let rec handler = (pull, push) => {
    let incoming =
      pull(timeout)
      |> Lwt.map(x => {
           switch (x) {
           | Ok(incoming) =>
             Stdlib.print_endline("incoming");
             Some(incoming);
           | Error(`Connection_close) =>
             Stdlib.print_endline("connection closed but continue");
             Some("connection close");
           | Error(`Empty) =>
             Stdlib.print_endline("empty");
             Some("empty");
           | Error(`Timeout) =>
             Stdlib.print_endline("timeout");
             Some("timeout");
           }
         });

    let%lwt incoming = incoming;

    let%lwt () =
      Option.fold(
        ~none=Lwt.return_unit,
        ~some=msg => Topic.publish(topic, ~msg),
        incoming,
      );
    handler(pull, push);
  };

So I'm not sure it's fixable by re-web but I do think the documentation should be clear about this.

[yawaramin]

Good point! I'll make a note of this in the docs. I guess an unintended consequence of allowing the WS handler to shut itself down after the client closes the connection, instead of killing it immediately like I used to...

[tcoopman]

Well the other option would be to change the API to take an on close handler, but I'm not sure I would like that more. For me it's good as it is.