Skip to content

Commit fd473da

Browse files
mcowgermcowger
authored
Fix provider timeout failover (#536)
## 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
1 parent 0624317 commit fd473da

9 files changed

Lines changed: 197 additions & 147 deletions

File tree

‎packages/backend/src/routes/inference/chat.ts‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,13 @@ export async function registerChatRoute(
9191
}
9292

9393
const abortController = new AbortController();
94-
const { signal: dispatchSignal, addTimeoutSource } = wireUpstreamTimeout(abortController);
94+
const { signal: dispatchSignal, resolveTimeoutMs } = wireUpstreamTimeout(abortController);
9595
earlyDisconnect = wireEarlyDisconnectDetection(request, abortController);
9696
const stallDetectionResult = wireStallDetection(abortController, getGlobalStallConfig());
9797
const unifiedResponse = await dispatcher.dispatch(
9898
unifiedRequest,
9999
dispatchSignal,
100-
addTimeoutSource,
100+
resolveTimeoutMs,
101101
stallDetectionResult?.addStallConfig
102102
);
103103

@@ -138,12 +138,8 @@ export async function registerChatRoute(
138138
return result;
139139
} catch (e: any) {
140140
earlyDisconnect?.cleanup();
141-
if (
142-
e?.routingContext?.code === 'client_disconnected' ||
143-
e?.routingContext?.code === 'upstream_timeout'
144-
) {
145-
usageRecord.responseStatus =
146-
e?.routingContext?.code === 'upstream_timeout' ? 'timeout' : 'cancelled';
141+
if (e?.routingContext?.code === 'client_disconnected') {
142+
usageRecord.responseStatus = 'cancelled';
147143
usageRecord.durationMs = Date.now() - startTime;
148144
usageRecord.attemptCount = e.routingContext?.attemptCount || usageRecord.attemptCount || 1;
149145
usageRecord.retryHistory =
@@ -154,7 +150,8 @@ export async function registerChatRoute(
154150
);
155151
return;
156152
}
157-
usageRecord.responseStatus = 'error';
153+
usageRecord.responseStatus =
154+
e?.routingContext?.code === 'upstream_timeout' ? 'timeout' : 'error';
158155
usageRecord.durationMs = Date.now() - startTime;
159156
usageRecord.attemptCount = e.routingContext?.attemptCount || usageRecord.attemptCount || 1;
160157
usageRecord.retryHistory = e.routingContext?.retryHistory || usageRecord.retryHistory || null;

‎packages/backend/src/routes/inference/gemini.ts‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,13 @@ export async function registerGeminiRoute(
9999
}
100100

101101
const abortController = new AbortController();
102-
const { signal: dispatchSignal, addTimeoutSource } = wireUpstreamTimeout(abortController);
102+
const { signal: dispatchSignal, resolveTimeoutMs } = wireUpstreamTimeout(abortController);
103103
earlyDisconnect = wireEarlyDisconnectDetection(request, abortController);
104104
const stallDetectionResult = wireStallDetection(abortController, getGlobalStallConfig());
105105
const unifiedResponse = await dispatcher.dispatch(
106106
unifiedRequest,
107107
dispatchSignal,
108-
addTimeoutSource,
108+
resolveTimeoutMs,
109109
stallDetectionResult?.addStallConfig
110110
);
111111

@@ -147,12 +147,8 @@ export async function registerGeminiRoute(
147147
return result;
148148
} catch (e: any) {
149149
earlyDisconnect?.cleanup();
150-
if (
151-
e?.routingContext?.code === 'client_disconnected' ||
152-
e?.routingContext?.code === 'upstream_timeout'
153-
) {
154-
usageRecord.responseStatus =
155-
e?.routingContext?.code === 'upstream_timeout' ? 'timeout' : 'cancelled';
150+
if (e?.routingContext?.code === 'client_disconnected') {
151+
usageRecord.responseStatus = 'cancelled';
156152
usageRecord.durationMs = Date.now() - startTime;
157153
usageRecord.attemptCount = e.routingContext?.attemptCount || usageRecord.attemptCount || 1;
158154
usageRecord.retryHistory =
@@ -163,7 +159,8 @@ export async function registerGeminiRoute(
163159
);
164160
return;
165161
}
166-
usageRecord.responseStatus = 'error';
162+
usageRecord.responseStatus =
163+
e?.routingContext?.code === 'upstream_timeout' ? 'timeout' : 'error';
167164
usageRecord.durationMs = Date.now() - startTime;
168165
usageRecord.attemptCount = e.routingContext?.attemptCount || usageRecord.attemptCount || 1;
169166
usageRecord.retryHistory = e.routingContext?.retryHistory || usageRecord.retryHistory || null;

‎packages/backend/src/routes/inference/messages.ts‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,13 @@ export async function registerMessagesRoute(
8989
}
9090

9191
const abortController = new AbortController();
92-
const { signal: dispatchSignal, addTimeoutSource } = wireUpstreamTimeout(abortController);
92+
const { signal: dispatchSignal, resolveTimeoutMs } = wireUpstreamTimeout(abortController);
9393
earlyDisconnect = wireEarlyDisconnectDetection(request, abortController);
9494
const stallDetectionResult = wireStallDetection(abortController, getGlobalStallConfig());
9595
const unifiedResponse = await dispatcher.dispatch(
9696
unifiedRequest,
9797
dispatchSignal,
98-
addTimeoutSource,
98+
resolveTimeoutMs,
9999
stallDetectionResult?.addStallConfig
100100
);
101101

@@ -137,12 +137,8 @@ export async function registerMessagesRoute(
137137
return result;
138138
} catch (e: any) {
139139
earlyDisconnect?.cleanup();
140-
if (
141-
e?.routingContext?.code === 'client_disconnected' ||
142-
e?.routingContext?.code === 'upstream_timeout'
143-
) {
144-
usageRecord.responseStatus =
145-
e?.routingContext?.code === 'upstream_timeout' ? 'timeout' : 'cancelled';
140+
if (e?.routingContext?.code === 'client_disconnected') {
141+
usageRecord.responseStatus = 'cancelled';
146142
usageRecord.durationMs = Date.now() - startTime;
147143
usageRecord.attemptCount = e.routingContext?.attemptCount || usageRecord.attemptCount || 1;
148144
usageRecord.retryHistory =
@@ -153,7 +149,8 @@ export async function registerMessagesRoute(
153149
);
154150
return;
155151
}
156-
usageRecord.responseStatus = 'error';
152+
usageRecord.responseStatus =
153+
e?.routingContext?.code === 'upstream_timeout' ? 'timeout' : 'error';
157154
usageRecord.durationMs = Date.now() - startTime;
158155
usageRecord.attemptCount = e.routingContext?.attemptCount || usageRecord.attemptCount || 1;
159156
usageRecord.retryHistory = e.routingContext?.retryHistory || usageRecord.retryHistory || null;

‎packages/backend/src/routes/inference/responses.ts‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -172,13 +172,13 @@ export async function registerResponsesRoute(
172172
}
173173

174174
const abortController = new AbortController();
175-
const { signal: dispatchSignal, addTimeoutSource } = wireUpstreamTimeout(abortController);
175+
const { signal: dispatchSignal, resolveTimeoutMs } = wireUpstreamTimeout(abortController);
176176
earlyDisconnect = wireEarlyDisconnectDetection(request, abortController);
177177
const stallDetectionResult = wireStallDetection(abortController, getGlobalStallConfig());
178178
const unifiedResponse = await dispatcher.dispatch(
179179
unifiedRequest,
180180
dispatchSignal,
181-
addTimeoutSource,
181+
resolveTimeoutMs,
182182
stallDetectionResult.addStallConfig
183183
);
184184

@@ -239,12 +239,8 @@ export async function registerResponsesRoute(
239239
return result;
240240
} catch (e: any) {
241241
earlyDisconnect?.cleanup();
242-
if (
243-
e?.routingContext?.code === 'client_disconnected' ||
244-
e?.routingContext?.code === 'upstream_timeout'
245-
) {
246-
usageRecord.responseStatus =
247-
e?.routingContext?.code === 'upstream_timeout' ? 'timeout' : 'cancelled';
242+
if (e?.routingContext?.code === 'client_disconnected') {
243+
usageRecord.responseStatus = 'cancelled';
248244
usageRecord.durationMs = Date.now() - startTime;
249245
usageRecord.attemptCount = e.routingContext?.attemptCount || usageRecord.attemptCount || 1;
250246
usageRecord.retryHistory =
@@ -255,7 +251,8 @@ export async function registerResponsesRoute(
255251
);
256252
return;
257253
}
258-
usageRecord.responseStatus = 'error';
254+
usageRecord.responseStatus =
255+
e?.routingContext?.code === 'upstream_timeout' ? 'timeout' : 'error';
259256
usageRecord.durationMs = Date.now() - startTime;
260257
usageRecord.attemptCount = e.routingContext?.attemptCount || usageRecord.attemptCount || 1;
261258
usageRecord.retryHistory = e.routingContext?.retryHistory || usageRecord.retryHistory || null;

‎packages/backend/src/services/__tests__/dispatcher-abort.test.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, test, vi, beforeEach } from 'vitest';
1+
import { describe, expect, test, vi, beforeEach, afterEach } from 'vitest';
22
import { Dispatcher } from '../dispatcher';
33

44
// Prevent real network calls
@@ -11,6 +11,10 @@ describe('Dispatcher — AbortSignal / cancellation', () => {
1111
dispatcher = new Dispatcher();
1212
});
1313

14+
afterEach(() => {
15+
vi.useRealTimers();
16+
});
17+
1418
test('buildCancelledError returns 499 and client_disconnected for a plain abort', () => {
1519
const controller = new AbortController();
1620
controller.abort();
@@ -35,6 +39,31 @@ describe('Dispatcher — AbortSignal / cancellation', () => {
3539
expect(err.routingContext.code).toBe('upstream_timeout');
3640
});
3741

42+
test('per-attempt timeout does not abort the route signal', async () => {
43+
vi.useFakeTimers();
44+
const routeController = new AbortController();
45+
46+
const attemptTimeout = (dispatcher as any).createAttemptTimeout(
47+
routeController.signal,
48+
35_000,
49+
() => 4_000
50+
);
51+
52+
await vi.advanceTimersByTimeAsync(4_000);
53+
54+
expect(attemptTimeout.isTimedOut()).toBe(true);
55+
expect(attemptTimeout.signal.aborted).toBe(true);
56+
expect(routeController.signal.aborted).toBe(false);
57+
});
58+
59+
test('buildTimeoutError returns retryable upstream timeout metadata', () => {
60+
const err = (dispatcher as any).buildTimeoutError();
61+
62+
expect(err.message).toBe('Upstream timeout');
63+
expect(err.routingContext.statusCode).toBe(504);
64+
expect(err.routingContext.code).toBe('upstream_timeout');
65+
});
66+
3867
test('executeProviderRequest forwards AbortSignal to fetch', async () => {
3968
const controller = new AbortController();
4069
const mockFetch = vi.fn(async () => new Response('{}', { status: 200 }));

0 commit comments

Comments
 (0)