Corrupted state when JSON write fails

[ferdinand-beyer]

I've encountered a tricky situation when a request result is not JSON serializable. While this is definitely a programmer's mistake, this unfortunately leads into a situation where the LSP server stops working.

I'm working with VS-Code as a Language Client. After a request, the client reported:

Connection to server got closed. Server will restart.

Then the client spun up a new server process. The old process was still alive, the JVM up, and my REPL connected. It was somewhat tricky to diagnose what happened.

I tracked this down to the lsp4clj.io-chan/output-stream->output-chan, which connects an async channel with a OutputStream like this:

(let [output (io/output-stream output)
        messages (async/chan 1)]
    (async/thread
      (with-open [writer output] ;; close output when channel closes
        (loop []
          (when-let [msg (async/<!! messages)]
            (write-message writer msg)
            (recur)))))
    messages)

When write-message fails, because the message is not serializable, an exception is thrown. This code then stops consuming the channel and closes the OutputStream through with-open. The client then assumes the server died, and will spin up a new one.

However, the messages channel will not be closed and will no longer be consumed. That means that the server process will continue to run, and probably hang very soon because of backpressure from the blocked output channel.

[ferdinand-beyer]

With the current design, there is not much we can do here. We could:

• ignore the error and thus the outgoing message
• transform it to an error message
• walk the message and find out which value(s) are not serialisable
• catch exceptions and close the output channel

To me, only the last option sounds feasible. Note that there could be other exceptions as well, most likely IOException.

We could also change the way the server puts messages on the channel. As far as I know we use >!! and ignore the result, so we would not notice when the output channel gets closed and continue to run. We could check the return value to see if the peer is still there, but we would only notice on the next send attempt, which might not happen.

I do expect though that when the client hangs up, we will either get an exit message, or at least the input channel would close, so the server should notice that something's going on. I have not debugged that yet, but I expect the server does not shut down because it will be blocked on the output channel. So closing it on error might unblock the server and allows it to shut down properly.

[ericdallo]

thanks for the detailed info!
Catch the exception, log it somehow as an critical error and close the channel seems the best indeed