write-body-to-stream does not behave like in ring.adapter.jetty

[chpill]

Hello, first of all, thank you for this library! I am considering using it
instead of the original adapter for a web project, but I ran into an issue with
an SSE handler. Here is a barebone handler that demonstrates the issue:

(Tested with Clojure 1.12.0-alpha5 and Java 21)

{:deps {org.clojure/clojure {:mvn/version "1.12.0-alpha5"}
        ;; info.sunng/ring-jetty9-adapter {:mvn/version "0.31.0"}
        ring/ring-jetty-adapter {:mvn/version "1.11.0"}
        }}

(ns sse-with-ring
  (:require [clojure.java.io :as io]
            ;; [ring.adapter.jetty9 :refer [run-jetty]]
            [ring.adapter.jetty :refer [run-jetty]]
            [ring.core.protocols]))

(defn handler [_]
  {:status 200
   :body (reify
           ring.core.protocols/StreamableResponseBody
           (write-body-to-stream [_ _ output-stream]
             (let [writer (io/writer output-stream)]
               (future (loop [i 10]
                         (doto writer (.write (str "data: " i "\n")) .flush)
                         (if (zero? i)
                           (.close writer)
                           (do (Thread/sleep 100)
                               (recur (dec i)))))))))})

(def server (run-jetty (fn [req respond raise]
                         (try (respond (handler req))
                              (catch Exception e (raise e))))
                       {:port 12345
                        :async? true
                        :join? false}))

(comment (.stop server))

With ring/ring-jetty-adapter {:mvn/version "1.11.0"}, the endpoint behaves as
expected, streaming each line as soon as they are "produced":

curl -Nv localhost:12345
*   Trying [::1]:12345...
* Connected to localhost (::1) port 12345
> GET / HTTP/1.1
> Host: localhost:12345
> User-Agent: curl/8.4.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Date: Wed, 10 Jan 2024 16:14:34 GMT
< Transfer-Encoding: chunked
< Server: Jetty(11.0.18)
<
data: 10
data: 9
data: 8
data: 7
data: 6
data: 5
data: 4
data: 3
data: 2
data: 1
data: 0
* Connection #0 to host localhost left intact

But with info.sunng/ring-jetty9-adapter {:mvn/version "0.31.0"}, curl returns
immediately without displaying any of our "data: x" messages:

curl -Nv localhost:12345
*   Trying [::1]:12345...
* Connected to lol (::1) port 12345
> GET / HTTP/1.1
> Host: lol:12345
> User-Agent: curl/8.4.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Date: Wed, 10 Jan 2024 15:58:38 GMT
< Content-Length: 0
< Server: Jetty(11.0.18)
<
* Connection #0 to host lol left intact

I found out an issue explaining that it is perfectly valid for a ring
handler to return while the output-stream remains open, and it may be closed
later in another thread.

Note that this streaming handler only works in the original adapter with the
:async? true setting. Reading the spec, it is not evident to me why this is
the case, and it's a bit unfortunate as we have to wrap our otherwise completely
synchronous ring application with:

(fn [req respond raise]
  (try (respond (handler req))
       (catch Exception e (raise e))))

Maybe @weavejester could clarify why this is the case?

[weavejester]

This isn't intentional behavior in Ring; the StreamableResponseBody should be responsible for closing the response stream, as otherwise a response with a long InputStream as a body would not transmit. I'll see about clarifying that in the SPEC.

I'm unsure why this doesn't work in synchronous mode in Ring. My guess is that Jetty is trying to be helpful by closing the stream after the handler completes when in synchronous mode, perhaps using some timeout. Can you raise an issue on the Ring repository so we can investigate this?

[chpill]

@weavejester thank you for your input! Yes, I'll open an issue in the Ring repository shortly.

[sunng87]

Note sure if it's related to issue we investigated in #113. I will find time this weekend to see

[weavejester]

I investigated this, and at least in the case of Ring, I believe I now understand why this occurs and why it probably isn't a bug: ring-clojure/ring#491 (comment)

[sunng87]

After some investigation I found it has something to do with https://github.com/sunng87/ring-jetty9-adapter/blob/master/src/ring/adapter/jetty9/handlers/sync.clj#L36

When I remove this line, it prints data x as expected. By notifying the callback, the handler tells Jetty that the request has been handled successfully. So the async thread won't be able to write more content into the response.

I'm not 100% if async thread is allowed as in this scenario. At least we will need a callback mechanism that once we close the output stream we need to notify Jetty that the request has been successfully handled.