fix(sse): flush response headers before user handler runs#1038
Merged
Conversation
The SSE Body callback set Content-Type but never committed the status or flushed the response writer before invoking the user handler. For handlers that wait on a channel before sending the first event, the browser's EventSource.onopen did not fire until the first event arrived because headers stayed buffered. Call SetStatus(200) and Flush() right after the flusher is discovered so headers are sent to the client immediately. Adds a regression test that asserts WriteHeader and Flush happen before the handler blocks. Fixes danielgtaylor#1037
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1038 +/- ##
=======================================
Coverage 93.14% 93.14%
=======================================
Files 23 23
Lines 4898 4901 +3
=======================================
+ Hits 4562 4565 +3
Misses 272 272
Partials 64 64 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Collaborator
|
Awesome fix, thanks so much! This will be included in our next release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sse.RegistersetContent-Type: text/event-streambut never committed the response status or flushed before invoking the user handler.EventSource.onopendid not fire until the first event arrived — headers stayed buffered.ctx.SetStatus(http.StatusOK)andflusher.Flush()right after the flusher is discovered, so headers are pushed to the client immediately.Fixes #1037
Root cause
huma.Registerskipsctx.SetStatus(status)on theoutBodyFuncpath (huma.go:1166-1168), soWriteHeader(200)is never called for streaming responses.sse.Register'sBodycallback only flushed insidesend()after writing event data, so until the first event the client saw no response headers.Why this matters — spec references
WHATWG HTML Living Standard §9.2 Server-sent events binds
onopento the arrival of the response headers, not to the first event. Once the response is verified as200withContent-Type: text/event-stream, the user agent must announce the connection:MDN — EventSource: open event restates the same contract:
In Go's
net/http, response headers are not put on the wire untilWriteHeaderis called and anhttp.Flusher.Flush()pushes the buffered bytes. Without both, a handler that blocks waiting for the first event leaves the client stuck inCONNECTING— violating the spec's expected timing.Test plan
go test ./sse/... -count=1— all existing tests passsse flushes headers before first eventuses anorderedWriterto assertWriteHeader(200)andFlushboth happen before the user handler blocks on a channelmainwithout the fix and passes with it