onSubscribeResource and onUnsubscribeResource discard their promise with a bare void, so a failed subscribe or unsubscribe is invisible to the user and surfaces as an unhandled browser rejection.
// clients/web/src/hooks/useServerCommands.tsx
const onSubscribeResource = useCallback(
(uri: string) => {
if (!inspectorClient) return;
void inspectorClient.subscribeToResource(uri);
},
[inspectorClient],
);
The callee does not own its failures. InspectorClient.subscribeToResource throws on "Client is not connected", on "Server does not support resource subscriptions", and it rethrows a wrapped transport/request error after rolling its subscription state back. unsubscribeFromResource is the same shape. AGENTS.md states the rule this breaks directly:
Reach for [void] only when the callee already owns its failures … Where the callee does not own its failures, give it a catch rather than voiding the call.
Every neighbouring command in that hook already routes through the shared recovery — onCancelTask, onSetLogLevel, the refreshes and the load-mores all go through runWithCommandAuthRecovery / runCommandInBackground. These two are the only ones that do not, so a mid-session 401 on a subscribe does not trigger re-auth the way a 401 on a refresh does, and any other failure is silent.
Why it isn't fixed in #2173
Both lines were moved verbatim by #2155 (PR #2173), which is a decomposition step carrying an explicit "no behavior change" constraint — routing them through runCommandInBackground adds auth recovery and an error toast, which is a real behavior change and does not belong buried in a relocation. Raised by Copilot on that PR and deliberately deferred here.
Repro
- Connect to
subscriptions-legacy-http.json from test-servers/configs.
- Subscribe to a resource, then kill the server (or let the session lapse).
- Click Subscribe again — nothing happens in the UI; the rejection appears only in the browser console.
Proposed fix
Route both through runCommandInBackground(..., "resource", "Failed to subscribe to resource") so a 401 recovers like every other command and any other failure toasts. Cover both in useServerCommands.test.tsx — the harness already reproduces the wrapper's shape.
Done when
onSubscribeResourceandonUnsubscribeResourcediscard their promise with a barevoid, so a failed subscribe or unsubscribe is invisible to the user and surfaces as an unhandled browser rejection.The callee does not own its failures.
InspectorClient.subscribeToResourcethrows on "Client is not connected", on "Server does not support resource subscriptions", and it rethrows a wrapped transport/request error after rolling its subscription state back.unsubscribeFromResourceis the same shape.AGENTS.mdstates the rule this breaks directly:Every neighbouring command in that hook already routes through the shared recovery —
onCancelTask,onSetLogLevel, the refreshes and the load-mores all go throughrunWithCommandAuthRecovery/runCommandInBackground. These two are the only ones that do not, so a mid-session 401 on a subscribe does not trigger re-auth the way a 401 on a refresh does, and any other failure is silent.Why it isn't fixed in #2173
Both lines were moved verbatim by #2155 (PR #2173), which is a decomposition step carrying an explicit "no behavior change" constraint — routing them through
runCommandInBackgroundadds auth recovery and an error toast, which is a real behavior change and does not belong buried in a relocation. Raised by Copilot on that PR and deliberately deferred here.Repro
subscriptions-legacy-http.jsonfromtest-servers/configs.Proposed fix
Route both through
runCommandInBackground(..., "resource", "Failed to subscribe to resource")so a 401 recovers like every other command and any other failure toasts. Cover both inuseServerCommands.test.tsx— the harness already reproduces the wrapper's shape.Done when
npm run local:gategreen