Skip to content

Resource subscribe/unsubscribe failures are silent — a bare void discards the promise #2174

Description

@cliffhall

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

  1. Connect to subscriptions-legacy-http.json from test-servers/configs.
  2. Subscribe to a resource, then kill the server (or let the session lapse).
  3. 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

  • Neither handler discards its promise
  • A mid-session 401 on subscribe/unsubscribe reaches the same recovery the refreshes do
  • Any other failure is surfaced rather than swallowed
  • Tests cover both paths; npm run local:gate green

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingv2Issues and PRs for v2

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions