Ensure streaming ResponseWriters implement Flush #406
Conversation
The `http.Flusher` interface isn't part of the `http.ResponseWriter` interface, but it's implemented by the HTTP/1 and HTTP/2 writers provided by the standard library. It's also critical to Connect: streaming handlers must flush messages to ensure that the client receives them. This PR adds a check for unflushable response writers; when the response writer for a streaming procedure can't be flushed, we now return an explicit error instead of hanging. Fixes #393.
|
I don't quite understand why this needs to be checked within the handler itself; wouldn't it be safer if it failed at creation of the Edit: I guess that the |
|
whoops, apologies |
|
I'm not sure why the build is failing on Windows :/ I won't be able to dig into it until late tonight (at the earliest) - if anyone else has time and is interested, please take a look! (Just ping this thread so nobody else picks it up at the same time.) |
@akshayjshah - Pushed a fix which should resolve the test failure. The |
In #406, I was too cavalier about `net/http.ResponseWriter`'s API to send HTTP trailers. Because we're not calling `WriteHeader` before between predeclaring our trailers and actually setting the trailer values, we end up sending the same data as both HTTP headers and trailers.
In #406, I was too cavalier about `net/http.ResponseWriter`'s API to send HTTP trailers. Because we're not calling `WriteHeader` before between predeclaring our trailers and actually setting the trailer values, we end up sending the same data as both HTTP headers and trailers.
In #406, I was too cavalier about `net/http.ResponseWriter`'s API to send HTTP trailers. Because we're not calling `WriteHeader` before between predeclaring our trailers and actually setting the trailer values, we end up sending the same data as both HTTP headers and trailers.
This PR bumps the go version to 1.22 and the min version to 1.20. This was prompted by a CI failure which I believe is related to this issue: golang/go#55846. Also included is a removal of a workaround for go1.19 header handling introduced here in #406.
The
http.Flusherinterface isn't part of thehttp.ResponseWriterinterface, but it's implemented by the HTTP/1 and HTTP/2 writers
provided by the standard library. It's also critical to Connect:
streaming handlers must flush messages to ensure that the client
receives them. This PR adds a check for unflushable response writers;
when the response writer for a streaming procedure can't be flushed, we
now return an explicit error instead of hanging.
Fixes #393.