fix(host-service): classify a deliberate worker abort as a non-500

Merged
SA
2mo ago
+182-13

Problem

The host worker pool rejects an in-flight task with WorkerTaskAbortedError in three ordinary lifecycle situations, and all three arrived at the tRPC boundary as INTERNAL_SERVER_ERROR — the only thing sentryMiddleware looks at:

  1. The client hung up. hono's node-server aborts the request signal ("Client connection prematurely closed") when the renderer navigates away or closes a pane, cancelling a batched query whose slowest member is still running. This is 68 of the 69 events: usage.leaderboardPayload in a 30-procedure batch (HOST-SERVICE-3N, 66 events) and standalone (HOST-SERVICE-59, 2 events).
  2. Shutdown. dispose() rejects whatever each slot was holding (HOST-SERVICE-5K, 1 event, git.getStatus).
  3. Superseded by a newer request for the same dedupe key.

User-visible harm. For (1) and (3), none — nobody is waiting for the answer (all three issues report Users Impacted: 0 and HTTP 200). The cost is ours: together this family is the largest recurring non-bug reporter in host-service, still accruing at ~51 events/14d, and archiving the biggest of the three did not stop it.

For (2) the harm is real. hostServiceQueryRetry in the renderer retries only what isHostServiceConnectionError accepts, which is SERVICE_UNAVAILABLE / BAD_GATEWAY. Its own doc comment says it exists "so a query in flight during a host-service restart self-heals instead of settling into a permanent 'Failed to fetch' that only a manual 'Try again' click clears". Because the shutdown rejection was coded INTERNAL_SERVER_ERROR, that retry never fired for exactly the case it was built for: in HOST-SERVICE-5K the host-service went down 8s after start on a freshly booted machine and the open pane's git.getStatus failed permanently instead of retrying.

Root cause

WorkerTaskAbortedError carried no discriminator, so the boundary had no way to tell a deliberate abort from a failure without matching on our own message text. It fell through to the default 500.

The desktop copy of this class already solved this and is ratified: apps/desktop/src/lib/trpc/workers/WorkerTaskRunner.ts declares kind: "disposed" | "superseded" | "cancelled", and git-task-runner.ts name-checks and maps it. This finishes the same job on the host-service side.

Fix

  • WorkerTaskAbortedError takes kind as its first constructor argument, matching the desktop declaration exactly. All 8 throw sites (6 in WorkerTaskRunner.ts, 2 in host-worker-pool.ts) pass one. Every message is unchanged.
  • New src/trpc/worker-abort.ts translates it: disposed → SERVICE_UNAVAILABLE, superseded/cancelled → CLIENT_CLOSED_REQUEST, with const exhaustive: never = kind in the default arm.
  • Wired in at the two places the reported procedures reach the pool: offLoop() (covers usage.leaderboardPayload, usage.history, settings.branchPrefix) and the existing git.getStatus catch.

No typed cause. The consumer that reads this — isHostServiceConnectionError — branches on the tRPC code, so a kind on the cause would be maintained for nothing.

Naming vs #7210. That PR adds WORKER_TASK_TIMEOUT_CODE / isWorkerTaskTimeout on WorkerTaskError.code. This is a sibling on a different class (WorkerTaskAbortedError.kind) with a different field name; the two don't collide and neither depends on the other.

What still reports on this path

Everything that is not a deliberate abort:

  • Task timeouts — WorkerTaskError, and with #7210 WorkerTaskTimeoutError + WORKER_TASK_TIMEOUT_CODE.
  • Worker crashes / non-zero exits — WorkerCrashedError.
  • Errors thrown by the task handler itself, which cross the boundary as WorkerTaskError carrying their original name (GitError, NotGitRepoError, …).
  • Queue rejections at circuit-open (rejectQueuedTasks).
  • An abort with an unrecognised kind — the default arm throws a plain Error, so it stays a 500.

Over-match, and what prevents it

Name-checking error.name === "WorkerTaskAbortedError" would on its own match an error a task handler threw, because the boundary rebuilds errors as WorkerTaskError carrying the original name. rethrowWorkerTaskAbort returns early on error instanceof WorkerTaskError before the name check, so anything that crossed the worker boundary can never be reclassified — only a locally-constructed abort can. There is no message matching anywhere in this change.

Verification

Negative tests were written and run before the change. The four negatives passed pre-change (so they guard rather than ride a general failure) and only the three abort cases were red:

$ bun test src/trpc/off-loop.test.ts        # BEFORE the change
(fail) offLoop > pool rejections > a client that hung up is not a server error
  Expected constructor: [class TRPCError extends Error]
  Received value: WorkerTaskAbortedError: cancelled
(fail) offLoop > pool rejections > shutdown is retryable rather than a server error
  Expected: "SERVICE_UNAVAILABLE"   Received: undefined
(fail) offLoop > pool rejections > a superseded task is not a server error
  Expected: "CLIENT_CLOSED_REQUEST" Received: undefined
 8 pass
 3 fail
Ran 11 tests across 1 file. [32.00ms]

After:

$ bun test src/trpc/off-loop.test.ts
 11 pass
 0 fail
 17 expect() calls
Ran 11 tests across 1 file. [21.00ms]

The exhaustiveness guard was proved by temporarily adding a fourth variant:

$ bun run typecheck   # with "evicted" added to WorkerTaskAbortKind
src/trpc/worker-abort.ts(47,10): error TS2322: Type '"evicted"' is not assignable to type 'never'.

Biome (formatting fixes applied with --write, then re-checked clean):

$ bunx biome check --write <6 changed files>
Checked 6 files in 24ms. Fixed 2 files.
$ bunx biome check <6 changed files>
Checked 6 files in 14ms. No fixes applied.

Typecheck and the full package suite:

$ cd packages/host-service && bun run typecheck
$ tsc --noEmit --emitDeclarationOnly false
   (no output)

$ bun test src/
 1370 pass
 0 fail
 3034 expect() calls
Ran 1370 tests across 116 files. [88.26s]

$ bun test test/
 327 pass
 8 todo
 0 fail
 924 expect() calls
Ran 335 tests across 48 files. [136.45s]

No desktop git code was touched, so no-main-process-blocking.test.ts is not in scope.

Left alone

Adjacent pool call sites that can also abort but are not in the reported family, and so keep their current classification: git.commit / git.push / getDiffStatsByWorkspaces and the other getHostWorkerPool().run sites in git.ts, workspaces.ts:282, pull-requests/procedures/create-for-workspace.ts:42, workspace-cleanup/git-ops.ts (which has its own isIndeterminateGitTaskFailure handling and is unchanged), and the PR-refs sync in app.ts:150, which is not a tRPC procedure.

Refs HOST-SERVICE-3N
Refs HOST-SERVICE-59
Refs HOST-SERVICE-5K

https://claude.ai/code/session_01FH9RULrVnhp78uaKRShmCq


Summary by cubic

Reclassifies deliberate worker task aborts so they no longer surface as INTERNAL_SERVER_ERROR, the only code Sentry reports. Client hang-ups, host shutdown, and superseded tasks now map to CLIENT_CLOSED_REQUEST or SERVICE_UNAVAILABLE, making shutdown rejections retryable and stopping the largest recurring non-bug Sentry reporter in host-service.

Details

  • WorkerTaskAbortedError now carries a kind discriminator (disposed | superseded | cancelled) matching the desktop declaration.
  • A new worker-abort.ts maps kinds to tRPC codes with an exhaustive switch; unrecognized kinds stay 500s via a never guard.
  • Wired in at offLoop() and the git.getStatus catch; other pool call sites keep their current classification.
  • Timeouts, worker crashes, and task-handler errors keep their 500s — only locally-constructed aborts are reclassified, with negative tests guarding against over-matching.
  • Refs HOST-SERVICE-3N, HOST-SERVICE-59, HOST-SERVICE-5K.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of interrupted background operations when a request is cancelled, superseded, or the host restarts.
    • Requests cancelled by a closed client now return the appropriate cancellation response instead of appearing as generic failures.
    • Host restart conditions are reported as temporarily unavailable, allowing supported clients to retry.
    • Genuine task failures continue to surface unchanged, preserving accurate error reporting.