Frontend reconnect hardening: tab wake & reconnect probe

[ndisidore]

Handles 2 cases the server cannot automagically help with recovery on:

• Reconnecting flicker: probe each candidate with getServerConfig() and only publish a stub that has round-tripped. This stops false reporting a reconnected state just to go back to disconnected
• Probe the connection on tab wake. visibilitychange/online triggers a probe of the current stub

_note reconnecting chip_

Testing

Types, unit tests, lint. Manually against the local dev stack:

• Outage (backend down ~3 min with a workspace open): chip appears once, no flicker; exactly one RPC connection lost / one RPC connection restored; measured attempts at +0.2s, +1.3s, +3.2s, +6.8s, +14s, then ~10s apart (correct jittered doubling backoff); chat works after recovery.
• Zombie (kill -STOP on workerd, then a wake event): probe times out at ~10s, disposes the stub, and the app recovers on its own once the process thaws (no user action needed).
• Tab-switch noise (counted probe frames by hooking WebSocket.send): a burst of 8 wake/online events yields exactly 1 probe past the 15s idle window, 0 within it.

[ask-bonk]

useEffect is still used at line 135. No unused import.

Now let me assess one final real concern: the getServerConfig() probe refetch interaction. AppWithConnection's effect (line 194-208) also calls rpcState.stub.getServerConfig() on every stub change. When probeOnWake calls getServerConfig() on the live stub, that's an extra RPC but harmless. When handleBroken swaps currentStub and notifies, the effect re-runs getServerConfig() — a second call after the probe's own call. Minor redundancy (two getServerConfig per reconnect), not a bug.

Let me double-check the jitter/backoff produces the PR's claimed measurements to validate correctness of the loop, and confirm skipSleep doesn't cause a tight infinite loop when the server is down.

Trace: server down, connection was up > 1s → skipSleep=true. First iteration: no sleep, try connect, probe fails (times out at 20s or connects-then-fails fast). If the WS fails fast (connection refused), the probe rejects quickly → continue. Second iteration: skipSleep=false, sleep ~1s, backoff→2s. This matches "+0.2s, +1.3s, +3.2s...".

But there's a subtle issue: if getServerConfig() on a refused socket rejects immediately every iteration, the loop still sleeps between attempts (backoff grows), so no tight loop. Good. However, if a candidate's WS connects but the backend hangs, the probe waits the full 20s timeout each attempt — acceptable per the comment.

I've completed a thorough review. My findings are minor/low-confidence; none are clear defects introduced by the change. Let me summarize.

Overall this is a well-constructed change. Let me note the observations I have.

Review Summary

This is a solid, well-reasoned change. The design correctly separates the two concerns (socket-level RPC state via useConnectionLost() vs. workspace-level reopen failures) and the comments are excellent at explaining the why. The handleBroken refactor to prove-before-publish is a genuine improvement over the old connect-then-hope flow, and moving isConnectionLost clearing out of the auth path (markConnectionRestored) into the probe is cleaner.

I found no clear defects introduced by this change. A few low-confidence observations (none blocking):

1. onRpcBroken registration moved after the probe (main.tsx:133). In the old code the broken handler was attached immediately after startConnection(); now it's attached only after getServerConfig() round-trips. This introduces a narrow window (probe success → onRpcBroken registration) where a break could be dropped. This is only safe if capnweb invokes an onRpcBroken callback immediately for an already-broken stub. That's the conventional capnweb/Cap'n-Proto semantic, so this is almost certainly fine — but it's now load-bearing where it wasn't before. Worth a one-line comment noting the reliance, if the semantic is confirmed.

2. withTimeout leaves its sleep timer running (main.tsx:79-80) after the wrapped promise wins the race. Not an unhandled rejection (Promise.race still observes the loser), but on a successful probe a stray setTimeout fires up to 20s later and throws into an already-settled race. Harmless, just slightly untidy; a clearTimeout in a finally would tighten it.

3. Double getServerConfig() per reconnect. handleBroken probes with getServerConfig(), then swaps currentStub and notifies, which re-runs the AppWithConnection effect (main.tsx:194-208) that calls getServerConfig() again on the same stub. Redundant but cheap and correct.

4. lastProvenAt initialized to Date.now() at module load (main.tsx:75) treats the not-yet-proven initial socket as freshly proven, so an immediate tab-switch inside the 15s window won't probe a zombie initial connection. Edge case, acceptable.

The removed markConnectionRestored/notifyCurrentStubUpdated exports have no remaining references, and the useEffect import in __root.tsx is still used elsewhere — no dangling code.

LGTM!

github run

[kentonv]

• I think there's a subtle change in behavior here. Previously, when the stub was detected broken, it was immediately replaced by a stub representing the eventual new connection, so new RPCs made while the reconnection handshake was in flight would be queued and delivered once the connection was up. After this change, I believe we leave the old, broken stub in place until after we've reconnected and probed the new connection to prove that it is good. This means in the meantime a bunch of RPCs might fail, but it would probably be better to have those RPCs wait for the new connection. I believe you should be able to accomplish this by setting the top-level stub to an RpcStub constructed from a Promise<RpcStub>, where you later resolve the promise to the final stub once the probe succeeds.

• getServerConfig() is not exactly free on the server side. It does a KV read and some other stuff, which is all wasted in the case that it's being used as a probe. I think we should instead add a ping() method that just returns nothing.

[kentonv]

Hmm, it looks like Cap'n Web actually might not currently expose a way to construct an RpcPromise/RpcStub from a Promise (or generally a Promise). We should probably add that. In raw Cap'n Proto we use it all the time.

Either RpcPromise should have a constructor from Promise or we should have some sort of makePipelined(p: Promise<T>): RpcPromise<T> function...

[ndisidore]

Hmm, it looks like Cap'n Web actually might not currently expose a way to construct an RpcPromise/RpcStub from a Promise (or generally a Promise). We should probably add that. In raw Cap'n Proto we use it all the time.

Either RpcPromise should have a constructor from Promise or we should have some sort of makePipelined(p: Promise<T>): RpcPromise<T> function...

I actually have a branch that does exactly this: let me see if I can get it over the line