fix(webapp): an unstreamable connection says so instead of going quiet - #218
Merged
Merged
Conversation
Both SSE handlers set their headers, wrote a 200, and only then asked
whether the writer could flush — answering "no" by returning. What reaches
the browser then is a valid, empty, closed 200, which EventSource retries
forever. No error, no log, nothing to search for.
That is exactly how it failed on the managed hub: an analytics middleware
there wrapped http.ResponseWriter and implemented neither Flush nor
Unwrap, so http.ResponseController could not reach the real writer.
content-length: 0 on every /events and /collab stream. Live change
notification delivered no frames and collaborative editing delivered no
keystrokes, while the POST leg kept answering {"ok":true} — for as long as
it took someone to open two tabs and notice.
The hub cannot repair a broken wrapper, but it can refuse to pretend.
refuseUnstreamable walks the Unwrap chain BEFORE a byte is written, so the
answer can be a 500 and a log line naming the cause. The next middleware
that does this costs one log line, not a silent feature outage.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
TL;DR
200, so nothing anywhere said so.500plus a log line naming the cause.What went wrong
handleEventsandhandleCollabStreamboth did this:By the time the question is asked, the 200 and the
text/event-streamheaders are already on the wire. Returning hangs up, and what the browser receives is a valid, empty, closed stream — soEventSourceretries it, forever, without ever firing an error it can distinguish from a flaky network.On the managed hub that is exactly what happened: an analytics middleware wrapped
http.ResponseWriterand implemented neitherFlushnorUnwrap, sohttp.ResponseControllercould not reach the real writer. Every stream came back:Live updates delivered no frames. Two browser tabs on one document each edited in their own world, while the POST leg kept answering
{"ok":true}. Nothing failed to compile and nothing was logged — it was found by opening two tabs.The change
refuseUnstreamablewalks theUnwrapchain before anything is written, so the answer can be a real 500 and a log line naming the cause:The hub can't repair somebody's broken wrapper. It can refuse to pretend the stream is fine.
The post-header
rc.Flush()check stays — a later flush can still fail on a dead connection, which is the case it now describes.Tests
stream_guard_test.gocovers all three states through the real handler:Flush/Unwrap200, 0 bytes500+ logUnwrap()200, 0 bytesnet/httpwriterEach fails without the fix — checked by reverting in a throwaway tree.
Verification
go vet ./...clean.internal/webapp,internal/syncer,cmd/bdrivepass. Playwright e2e 243/243, and the guard never fires there — the harness's writer is flushable, which is the point.Two notes worth carrying forward:
/eventsstream and every editing tab a second/collabone. On HTTP/1.1 the browser allows six sockets per origin across all tabs, so six browsing tabs — or two editing plus two idle — wedge the whole app, POSTs included. Measured, reproducible, separate change.cmd/bdrive/desktop.go'scopyFlushinghas the same fail-quiet shape, left alone: nothing wraps the sidecar's writer, and it is not on the path this broke.🤖 Generated with Claude Code