Fix provider timeout failover

[mcowger]

Summary

• enforce upstream timeouts per dispatcher attempt so timed-out providers can fail over
• make per-provider timeout override the global timeout, including longer overrides
• return real 504 responses for final upstream timeouts while preserving timeout usage logging

Verification

• bun run test:force
• pre-commit hooks: backend tests, biome format/lint, no migrations, typecheck

Fixes #522
Fixes #531

[greptile-apps]

Greptile Summary

This PR rearchitects upstream timeout enforcement: instead of wiring a single global AbortSignal.timeout() into the route's AbortController (which would abort the whole route on first timeout), per-attempt timeouts are now created inside the dispatcher for each provider attempt. A timed-out provider fails over to the next candidate, and the route only surfaces a 504 once all providers are exhausted.

• wireUpstreamTimeout is simplified to just a resolveTimeoutMs closure; the new createAttemptTimeout method builds an isolated timeoutController per attempt with AbortSignal.any([routeSignal, timeoutController.signal]), ensures cleanup at every exit path, and exposes isTimedOut() so catch blocks can distinguish a per-attempt timeout from a generic error.
• All four route handlers now record responseStatus = 'timeout' in the non-early-return error path (returning a real 504) rather than the early-return path that previously silently dropped the connection.
• The logger.info inside the client_disconnected-only branch in each route file still contains a now-dead upstream_timeout ternary, and a comment in response-handler.ts still references the old wireUpstreamTimeout timeout-wiring path that no longer exists.

Confidence Score: 4/5

Safe to merge — the core failover logic is correctly implemented and well-tested; the three findings are all cosmetic or low-impact cleanup items.

The architectural change is sound: per-attempt timeouts are properly isolated from the route signal, cleanup is called at every real exit path, and isTimedOut() correctly distinguishes provider timeouts from route-level client disconnects. The only substantive gap is a timer leak when enforceContextLimit throws, which is harmless in practice because unref() is called on the timer.

dispatcher.ts around the enforceContextLimit call and the four route handler files for the stale log ternary

Important Files Changed

Filename	Overview
packages/backend/src/utils/timeout.ts	Significantly simplified: drops the global route-level timeout signal and AbortController wiring; now just exposes a resolveTimeoutMs closure that the dispatcher uses per attempt
packages/backend/src/services/dispatcher.ts	Core of the fix: adds createAttemptTimeout and buildTimeoutError, wires per-attempt AbortSignal and cleanup; a missing cleanup before enforceContextLimit throw leaves a benign timer leak
packages/backend/src/routes/inference/chat.ts	Timeout status is now recorded in the error path (504 branch) rather than the early-return path; logger.info ternary inside the client_disconnected branch is now dead code (always 'cancelled')
packages/backend/src/routes/inference/gemini.ts	Same pattern as chat.ts — stale upstream_timeout ternary in the logger inside the client_disconnected-only branch
packages/backend/src/routes/inference/messages.ts	Same pattern as chat.ts — stale upstream_timeout ternary in the logger inside the client_disconnected-only branch
packages/backend/src/routes/inference/responses.ts	Same pattern as chat.ts — stale upstream_timeout ternary in the logger inside the client_disconnected-only branch
packages/backend/src/services/response-handler.ts	Minor comment update only; the abort-listener comment at line 359 still references old wireUpstreamTimeout() timeout wiring that no longer exists
packages/backend/src/services/tests/dispatcher-abort.test.ts	Adds per-attempt timeout tests: verifies route signal isolation, buildTimeoutError shape, and fixes missing afterEach timer reset
packages/backend/src/utils/tests/timeout.test.ts	New test file covering resolveTimeoutMs with per-provider and null (global fallback) timeout values

Comments Outside Diff (3)

1. packages/backend/src/routes/inference/chat.ts, line 148-150 (link)

   The ternary inside this client_disconnected-only branch can never evaluate to 'timeout' — e?.routingContext?.code is always 'client_disconnected' here. The same pattern exists in gemini.ts, messages.ts, and responses.ts.

2. packages/backend/src/services/response-handler.ts, line 355-363 (link)

   This comment still describes the old behavior where wireUpstreamTimeout() called abortController.abort() when a timeout fired. With the new design, per-attempt timeouts fire an isolated timeoutController and are cleared before the streaming response is returned; this listener is now exclusively for client-disconnect detection.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

3. packages/backend/src/services/dispatcher.ts, line 378-380 (link)

   Timer leak when enforceContextLimit throws

   createAttemptTimeout (which starts the per-attempt setTimeout) is called at line 297, before the main try/catch block at line 410. If enforceContextLimit throws a ContextLengthExceededError here, control escapes the loop iteration without hitting attemptTimeout.cleanup(), leaking the timer. The practical impact is low because timeoutId.unref?.() is called so the leaked timer won't block process shutdown, but wrapping this call in a small try/finally (or moving it inside the main try block) would eliminate the leak entirely.

_{Reviews (1): Last reviewed commit: "fix: handle provider timeouts per attemp..." | Re-trigger Greptile}