fix(app): reconnect the event stream after the server restarts - #227
Closed
kateebonner wants to merge 1 commit into
Closed
fix(app): reconnect the event stream after the server restarts#227kateebonner wants to merge 1 commit into
kateebonner wants to merge 1 commit into
Conversation
The v1 event stream reports failures through its `onSseError` callback and
then simply stops yielding — the async iterator neither throws nor completes.
The reconnect loop only comes round when that iterator ends, so it parked
inside `for await` forever: the catch, the loop tail and the 250ms retry were
never reached, and the client stayed disconnected from a server that was
already back. Only a page reload recovered it.
Traced in the browser against a real restart. Before:
+1.9s loop iteration start
+1.9s stream obtained -> CONNECTED
+18.2s onSseError: TypeError: network error
+53.2s (end — no catch, no loop tail, no retry, still disconnected 35s
after the server was listening again)
After:
+17.6s onSseError -> abort
+17.6s loop iteration start
+17.6s stream obtained -> CONNECTED
This is not a rare edge. #221 made the solver toggle restart the opencode
server by design, so every solver switch left the webview dead until reload —
and it is the same shape as opencode#132's stuck reconnecting banner, which is
very likely why ConnectionBanner was unmounted in f696388 rather than fixed.
The abort is extracted into applySseError() and covered by tests: on its own it
reads as redundant next to the disconnect, which is exactly how it would get
tidied away again. The early return for an already-closed stream is what stops
our own abort from recursing.
Refs #132.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesSSE error handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The reconnect fix is localized and tested, and no actionable merge-blocking risk remains at the current head. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
Folded into #228 — same two commits, one PR. Closing this to keep review in one place. |
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
1 participant
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.
The bug
After the opencode server restarts, the app never reconnects. It stays disconnected from a server that is already back, indefinitely — only a page reload recovers it.
Traced in a real browser against a real restart:
No
catch. No loop tail. No secondloop iteration start. The retry never runs.Why
The v1 event stream reports failures through its
onSseErrorcallback and then simply stops yielding — the async iterator neither throws nor completes. The reconnect loop only comes round when that iterator ends, so it parks insidefor awaitforever.RECONNECT_DELAY_MSis 250ms and it never gets to use it.onSseErrorsetstreamStatus = "disconnected"and returned, which is why the symptom is a permanently disconnected client rather than a crash.The fix
Abort the attempt from
onSseError. That ends the iterator, control reaches the catch and the loop tail, and the next iteration reconnects.The abort is extracted into
applySseError()and covered by tests. On its own it reads as redundant next to thedisconnectright above it — which is exactly how it would get tidied away again. The early return for an already-closed stream is what keeps our own abort from recursing.Why this matters more than it looks
This is not a rare edge. #221 made the solver toggle restart the opencode server by design, so every solver switch has been leaving the webview dead until reload.
It is also the same shape as #132's stuck reconnecting banner. I suspect it is the real reason
ConnectionBannerwas unmounted in f696388/a03aa04 rather than fixed — the banner was reporting this bug accurately and looked broken for it.Found while smoke-testing #225 (the solver-switch banner), which could reach
restartingbut neverready. With this fix the full sequence lands:Verification
bun turbo typecheck— 30/30oxlinton the changed files — 0 errorspackages/appunit — 917 pass / 1 fail; the failure isi18n parity, pre-existing and unrelatedRefs #132. Unblocks #225.
Summary by CodeRabbit