onRpcBroken should return a Disposable so registrations can be undone

[dimitropoulos]

Version: capnweb 0.10.0. Applies to RpcStub.onRpcBroken / RpcPromise.onRpcBroken.

Summary

onRpcBroken() returns void, so a registration can never be undone. Registering the same callback twice fires it twice, and there is no offRpcBroken. I would like it to return a Disposable handle that unregisters the callback.

Why this matters

Every other subscription-shaped API in the ecosystem hands back something you can use to stop listening: addEventListener/removeEventListener, AbortSignal, or a returned unsubscribe function. onRpcBroken is the odd one out, and the places you naturally want to call it are exactly the places where things get re-run.

The clearest case is a UI framework. A React effect that registers a handler and re-runs (a dependency changes, StrictMode double-invokes, a component remounts against a session that outlives it) has no way to clean up after itself, so handlers accumulate on a long-lived stub and each break fires all of them. The workaround is to hoist registration to the point where the session is created, which works but constrains where the code can live, and pushes people toward a mutable "current handler" variable that the permanent callback dispatches through.

Our own documentation currently has to warn readers about this rather than show them the fix:

onRpcBroken cannot be unregistered. It returns nothing, and registering twice on the same stub fires twice. Register it where the session is created rather than in an effect that might re-run.

Reproduction

const ret = main.onRpcBroken(() => {});
console.log(ret);                    // undefined

let fires = 0;
const cb = () => { fires++; };
main.onRpcBroken(cb);
main.onRpcBroken(cb);                // same function, registered twice
transport.breakNow(new Error('connection lost'));
// fires === 2

Deduplicating by function identity would not be a fix on its own, since two independent subscribers legitimately registering the same function should each be honoured. What is missing is a handle.

Proposed solution

Return a Disposable:

interface StubBase<T = unknown> extends Disposable {
  onRpcBroken(callback: (error: any) => void): Disposable;
}

const subscription = stub.onRpcBroken(handleBroken);
// ...later
subscription[Symbol.dispose]();

which makes the ergonomic form work directly, and matches how the rest of the library already asks you to think about lifetimes:

{
  using subscription = stub.onRpcBroken(handleBroken);
  await doWorkThatMightBreak();
}   // unregistered here

Notes on the shape:

• Non-breaking. The method currently returns undefined, so existing call sites that ignore the result keep working unchanged.
• Idempotent. Disposing twice, or disposing after the callback has already fired, should be a no-op rather than an error.
• Disposing the stub should continue to release registrations as it does today; the handle is for the narrower case of ending one subscription early.
• Returning a bare () => void unsubscribe function is the other obvious option, but Disposable composes with using and is the convention the library has already committed to for stubs and for the revoker objects people build on top of Symbol.dispose.

Implementation sketch

The bookkeeping this needs is largely present. RpcImportHook.onBroken() (src/rpc.ts) already pushes onto the session-level array and records the slot index:

let index = this.session.onBrokenCallbacks.length;
this.session.onBrokenCallbacks.push(callback);
if (!this.onBrokenRegistrations) this.onBrokenRegistrations = [];
this.onBrokenRegistrations.push(index);

and resolve() already deletes individual slots with delete this.session.onBrokenCallbacks[i], preserving the ordering that existing tests pin. A returned handle would close over the hook and that index and perform the same deletion.

The abstract StubHook.onBroken (src/core.ts:315) and its implementations would change return type. Two are trivial (ErrorStubHook fires immediately, so it can return an already-disposed no-op handle; the RpcTarget hook at src/core.ts:1967 is currently a no-op). PromiseStubHook.onBroken forwards to a resolution that may not exist yet, so its handle needs to unregister from whichever hook it eventually forwarded to.

Relationship to #210

#210 reports that registrations on a disposed import are retained for the session's lifetime and fire at teardown. That is a bug in the same bookkeeping this feature would build on, and its fix sketch (mirroring resolve()'s cleanup on the disposal path) is roughly the operation a disposable handle needs to perform on demand. It probably makes sense to do #210 first, or to do both together.

Alternative worth considering alongside

An AbortSignal option would compose well with code that already has one for teardown:

stub.onRpcBroken(handleBroken, { signal: controller.signal });

Worth noting for anyone who finds this issue by searching: this would be a purely local API, so it is unaffected by the fact that AbortSignal cannot currently be serialized and sent over the wire.

One reason to settle this soon

onRpcBroken does not exist in the Workers Runtime's native RPC yet, and the two systems are converging deliberately. Fixing the signature while Cap'n Web is the only implementation avoids having to change it later in two places, one of which would need a compatibility flag.

[ndisidore]

Good writeup here: my take is that signal handling is relatively easy to layer on top of the onRpcBroken handle
i.e.

const handle = stub.onRpcBroken(cb);
signal.addEventListener('abort', () => handle[Symbol.dispose](), { once: true });

and can be added later if necessary, so that seems a good starting point/minimum approach. the abort signal is mostly sugar

What do we think should happen to onRpcBroken callbacks when their stub is fully disposed while the session is healthy? My vote is we silently unregister