Surface and bound live view connect failures - #403
robertjamesprior wants to merge 2 commits into
Conversation
onMessage is assigned directly to ws.onmessage, so a throw from createPeer or setRemoteOffer became a discarded rejection: no error, no overlay, no parent-frame message. The only thing left running was the 15s connect watchdog, which closed the socket and reconnected into the identical failure indefinitely. Route a throw to onDisconnected, cap consecutive attempts that never reach a connected peer, and post the terminal reason to the parent frame so embedders can react. An unsupported browser is now terminal too rather than retried. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bound check incremented before comparing, so giving up after three attempts reported four, contradicting its own message. Count only attempts that opened a socket. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Out of draft. Verified before flipping it: No dependencies. Single commit on One accuracy fix pushed ( if (++this._connectAttempts > MAX_CONNECT_ATTEMPTS) {so on the call that gave up, Worth catching now rather than later: All 5 tests pass locally ( Note on scope: this stops the infinite loop and reports the terminal reason. The designed terminal state with one-click resume, the shared recovery heuristic, and the |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f00dbc7. Configure here.
| } catch (err: unknown) { | ||
| this.onDisconnected(err instanceof Error ? err : new Error(String(err))) | ||
| } | ||
| } |
There was a problem hiding this comment.
Failed connects skip parent report
High Severity
A throw from createPeer now reaches onDisconnected, which tears down the socket and clears the 15s watchdog, but never calls giveUp. connect() is only invoked from the connect overlay mounted hook, and that overlay stays mounted while connected remains false, so a later connect() never runs. KERNEL_CONNECTION_FAILED is never posted, and the parent also loses the former KERNEL_CONNECTION_TIMEOUT signal.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit f00dbc7. Configure here.


Summary
Draft for review — this is the minimum needed to stop the live view failing silently, split out from the larger recovery work so it can land on its own.
onMessageis assigned directly tows.onmessage, so a throw fromcreatePeerorsetRemoteOfferhad nowhere to go. The rejection was discarded: noemit('error'), noonDisconnected, no overlay, no parent-frame message. The signaling socket stayed open and healthy, so the client could not distinguish "peer construction failed" from "still connecting". The only thing still running was the 15s connect watchdog, which closed the socket and reconnected into the identical failure — and becausecleanup()re-renders the connect component, which auto-logs-in on mount, that cycle repeated indefinitely.Three changes:
onMessagenow wraps the handler so a rejection reachesonDisconnected. This covers the whole handler rather than just thesignal/providebranch, sincesetRemoteAnswerand the rest can throw the same way.connect()and reset inonConnected()once frames can actually flow. A retry that reproduces the same failure is not recovery.KERNEL_CONNECTION_FAILEDto the parent frame with the reason, attempt count, and the ICE/signaling/socket state at that moment, reusing the existingpostParentMessagehelper.An unsupported browser (
RTCPeerConnectionmissing) is now terminal as well. It previously calledonDisconnectedand then looped through the same auto-login cycle, which could never succeed.Follow-ups this does not do
KERNEL_CONNECTION_FAILEDis a new parent-frame event and is not yet documented. The live view events table was published in Document live view parent frame events docs#611, so that page needs a follow-up entry. The event name is worth a second opinion before it becomes a public contract.addIceCandidateis still not awaited in the candidate branch, so it remains a separate un-surfaced rejection path. Left alone here to keep the diff to the reported failure.Testing
bun test testsinimages/chromium-headful/client: 27 pass, 0 fail (22 pre-existing plus 5 new intests/connect-bound.test.ts, covering the swallowed throw, the attempt bound, the parent-frame payload, no further sockets after giving up, unsupported-browser termination, and the counter reset on success).vue-cli-service lintreports no new warnings on the changed files.Not yet exercised against a real browser image — the failure needs
RTCPeerConnectionto throw, which the test reproduces by construction.🤖 Generated with Claude Code
Note
Medium Risk
Changes embedded live-view connection and recovery behavior and introduces a new parent-frame
KERNEL_CONNECTION_FAILEDcontract; mis-handling could affect dashboards that embed the client.Overview
Stops the chromium-headful live view from failing silently when WebRTC setup throws or never reaches a connected peer.
WebSocket handler errors are no longer swallowed:
onMessagedelegates tohandleMessageinside try/catch so async failures fromcreatePeer,setRemoteOffer, and related signaling paths flow intoonDisconnectedinstead of leaving a healthy signaling socket with no peer.Retries are capped at three connect attempts (
_connectAttempts, reset inonConnectedonce media can flow). After that—or immediately whenRTCPeerConnectionis unsupported—the client sets_gaveUp, stops opening new sockets, and callsgiveUp, which postsKERNEL_CONNECTION_FAILEDto the parent frame (reason, attempt count, ICE/connection/signaling state, socket open) via the existingpostParentMessagehelper. Users still see the normal disconnected notification; there is no new terminal UI in this PR.Adds
tests/connect-bound.test.ts(five cases) for the swallowed-throw path, attempt bound, parent payload, no further connects after give-up, unsupported-browser termination, and counter reset on success.Reviewed by Cursor Bugbot for commit f00dbc7. Bugbot is set up for automated code reviews on this repo. Configure here.