fix(model-auto-router): treat stalled stream as transient failure, enabling failover #12

Merged

Copilot merged 4 commits from copilot/fix-stalled-stream-error-handling into main

2026-09-22 17:44:42 +08:00

Copilot commented

2026-09-22 17:34:58 +08:00

(Migrated from github.com)

When a provider accepts a connection but sends no SSE events for stallTimeoutMs, the watchdog was hardcoded to fatal — calling pushError/finishRunSummary directly and bypassing the failover machinery entirely. A stall is operationally identical to a transient upstream timeout, so all configured backup targets were wasted.

Changes

src/index.ts + dist/index.js

Watchdog callback: removed logEvent({ event: "fatal" }), pushError(), and finishRunSummary(). Replaced with two per-iteration flags (stalledByWatchdog, stallError). Still sets ended.done = true and calls iterator.return() to unblock the frozen await iterator.next().
- Side-fix: original code double-decremented selectedState.active (watchdog + finally). The new path relies solely on the finally block, which always runs.
Post-try/finally guard: changed if (ended.done || committed) → if (committed || (ended.done && !stalledByWatchdog)) so a watchdog stall no longer short-circuits the request.
New stall failover block: when stalledByWatchdog, adds the target to tried, records it in transientFailures, increments failovers, and emits a "failover" log event — feeding the existing retry/backoff/all-failed machinery.

// before: fatal, no failover
logEvent({ event: "fatal", route: routeId, target: key, error: message });
pushError(outer, model, message);
finishRunSummary(routeId, "failed", failovers);

// after: transient failover
stallError = `[model-auto-router] ${key} sent no events for …`;
stalledByWatchdog = true;
// → handled post-try/finally: transientFailures.set(key, …); rankTargets → next target

test/e2e/model-auto-router.test.ts

Updated stall test: the basic route (2 targets) now exhausts both stalled targets and resolves with "All targets failed" instead of a direct stall error message.

Fixes #10

When a provider accepts a connection but sends no SSE events for `stallTimeoutMs`, the watchdog was hardcoded to `fatal` — calling `pushError`/`finishRunSummary` directly and bypassing the failover machinery entirely. A stall is operationally identical to a transient upstream timeout, so all configured backup targets were wasted. ## Changes **`src/index.ts` + `dist/index.js`** - **Watchdog callback**: removed `logEvent({ event: "fatal" })`, `pushError()`, and `finishRunSummary()`. Replaced with two per-iteration flags (`stalledByWatchdog`, `stallError`). Still sets `ended.done = true` and calls `iterator.return()` to unblock the frozen `await iterator.next()`. - Side-fix: original code double-decremented `selectedState.active` (watchdog + `finally`). The new path relies solely on the `finally` block, which always runs. - **Post-try/finally guard**: changed `if (ended.done || committed)` → `if (committed || (ended.done && !stalledByWatchdog))` so a watchdog stall no longer short-circuits the request. - **New stall failover block**: when `stalledByWatchdog`, adds the target to `tried`, records it in `transientFailures`, increments `failovers`, and emits a `"failover"` log event — feeding the existing retry/backoff/all-failed machinery. ```js // before: fatal, no failover logEvent({ event: "fatal", route: routeId, target: key, error: message }); pushError(outer, model, message); finishRunSummary(routeId, "failed", failovers); // after: transient failover stallError = `[model-auto-router] ${key} sent no events for …`; stalledByWatchdog = true; // → handled post-try/finally: transientFailures.set(key, …); rankTargets → next target ``` **`test/e2e/model-auto-router.test.ts`** - Updated stall test: the `basic` route (2 targets) now exhausts both stalled targets and resolves with `"All targets failed"` instead of a direct stall error message.  - Fixes #10

weisanju (Migrated from github.com) reviewed

2026-09-22 17:34:58 +08:00