Skip to content

Commit 4ce67bb

Browse files
committed
refactor(gateway): extract #attempt's error classification into its own method
#attempt's catch block had grown a cyclomatic/cognitive complexity flag from five stacked review-round fixes (C1-C4, P1, P2, H1) sharing one branch. Pulled the classification logic (NO_CAPACITY's stale-view exception aside) into #classifyLeaseRequestError, next to #classifyRelayedError it composes with -- no behavior change, same tests all still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MA98m7ua7qvDFZjxFaww6Z
1 parent ca188e8 commit 4ce67bb

1 file changed

Lines changed: 31 additions & 30 deletions

File tree

src/gateway/fleet-coordinator.ts

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -594,43 +594,44 @@ export class FleetLeaseCoordinator {
594594
this.#staleView(waiter, workerId);
595595
return;
596596
}
597-
// A terminal failure past this point is the worker's own fact (it already emitted its own
598-
// `lease.rejected`, relayed onto this bus with `workerId` added by `WorkerLink`) -- this
599-
// class does not emit a second one for it (see the module doc, "two different fields" and
600-
// events.md's post-commit rule apply equally to not inventing a duplicate fact). Only the
601-
// error code is preserved so the caller sees what the worker actually said, unless this
602-
// was a transport failure (the uplink itself, not the worker's own answer) -- ADR §28/§29
603-
// name `WORKER_UNREACHABLE` for that, not whatever the client's own connection loss happens
604-
// to be called (`daemon/dispatch.js`'s `SimlockError.kind` is what tells the two apart).
605-
// P2 (round 2 review): `#withLeaseRequestTimeout`'s own `DispatchError` (never a
606-
// `SimlockError` -- it never reached the worker at all) is forwarded as-is rather than
607-
// falling into the generic "not a SimlockError" branch below.
608-
// H1 (round 2 review): a value that is neither a `DispatchError` this class raised nor a
609-
// `SimlockError` the wire produced is not a fact about the worker at all -- every real
610-
// transport failure is already a `kind: "transport"` `SimlockError` by the time it gets
611-
// here (see the comment above), so anything else reaching this branch is a bug in this
612-
// coordinator's own request-building code. `WORKER_UNREACHABLE` previously answered that
613-
// too, misreporting a gateway-side crash as "the machine is unreachable"; `INTERNAL` is
614-
// what `#forwardToWorker`'s matching branch now answers for the same shape of failure.
615-
this.#queue.reject(
616-
waiter,
617-
error instanceof DispatchError
618-
? error
619-
: isSimlockError(error)
620-
? this.#classifyRelayedError(error, workerId)
621-
: new DispatchError(
622-
"INTERNAL",
623-
`Unexpected error forwarding lease.request to worker ${workerId}`,
624-
{ workerId },
625-
),
626-
);
597+
this.#queue.reject(waiter, this.#classifyLeaseRequestError(error, workerId));
627598
return;
628599
}
629600

630601
announceDispatched();
631602
this.#settleGrant(waiter, workerId, grant);
632603
}
633604

605+
/**
606+
* A terminal failure past `#attempt`'s `NO_CAPACITY`/stale-view check is the worker's own fact
607+
* (it already emitted its own `lease.rejected`, relayed onto this bus with `workerId` added by
608+
* `WorkerLink`) -- this class does not emit a second one for it (see the module doc, "two
609+
* different fields", and events.md's post-commit rule apply equally to not inventing a
610+
* duplicate fact). Only the error code is preserved so the caller sees what the worker actually
611+
* said, with three exceptions:
612+
*
613+
* - a transport failure (the uplink itself, not the worker's own answer) -- ADR §28/§29 name
614+
* `WORKER_UNREACHABLE` for that, not whatever the client's own connection loss happens to be
615+
* called (`daemon/dispatch.js`'s `SimlockError.kind` is what tells the two apart);
616+
* - `#withLeaseRequestTimeout`'s own `DispatchError` (P2, round 2 review) -- never a
617+
* `SimlockError`, since it never reached the worker at all -- forwarded as-is;
618+
* - anything else that is neither of those (H1, round 2 review): not a fact about the worker,
619+
* since every real transport failure already arrives as a `kind: "transport"` `SimlockError`,
620+
* so this is a bug in this coordinator's own request-building code. `WORKER_UNREACHABLE`
621+
* used to answer this case too, misreporting a gateway-side crash as "the machine is
622+
* unreachable"; `INTERNAL` is what `#forwardToWorker`'s matching branch answers for the same
623+
* shape of failure.
624+
*/
625+
#classifyLeaseRequestError(error: unknown, workerId: string): DispatchError {
626+
if (error instanceof DispatchError) return error;
627+
if (isSimlockError(error)) return this.#classifyRelayedError(error, workerId);
628+
return new DispatchError(
629+
"INTERNAL",
630+
`Unexpected error forwarding lease.request to worker ${workerId}`,
631+
{ workerId },
632+
);
633+
}
634+
634635
/** ADR §11: "an immediate `NO_CAPACITY` is the only answer that leaves it queued ... the
635636
* request waits" -- unconditionally, even for a caller that asked `noWait: true` (the one
636637
* explicit exception the ADR calls out), because this is a stale view, not a real refusal. */

0 commit comments

Comments
 (0)