Skip to content

Commit 7396fdb

Browse files
committed
fix(proxy-retry): gate streamIdleTimeoutMs watchdog to retry modes (not off)
The watchdog (added in 02e295f) wraps response.body in a TransformStream to bound idle on a hung upstream stream. But the interceptor (server/interceptor.js) already reconstructs response.body via getReader() + a new ReadableStream for logging/live-streaming. Under CI's concurrent load the watchdog's pipeThrough races that reconstruction and surfaces as a spurious `fetch failed` → status 0 → 502 on a healthy streaming 200. Gate the watchdog to retry modes only (serial/race/stagger), mirroring the existing connectTimeoutMs off-exclusion. off mode is the legacy pass-through path (no retry): the watchdog's value (bound idle on a hung stream) is marginal there since the interceptor already observes the stream, and gating it off restores byte-identical pre-watchdog streaming behavior. Verified: proxy-server.test.js streaming-200 test passes 3/3 under full test:cli flags (--test-force-exit, --test-timeout=120000); proxy-retry-stream-idle.test.js still passes (hung-stream aborts in serial mode, healthy stream does not).
1 parent 31aa63e commit 7396fdb

1 file changed

Lines changed: 10 additions & 3 deletions

File tree

server/lib/proxy-retry.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -496,9 +496,16 @@ export async function executeRequest({ url, fetchOptions, retryConfig, ctx }) {
496496
// headers well past 10s — so with retry disabled we must not introduce a new
497497
// failure mode. The timeout applies only when a retry mode is active.
498498
const effectiveConnectTimeoutMs = cfg.mode === 'off' ? 0 : cfg.connectTimeoutMs;
499-
// streamIdleTimeoutMs applies to streaming responses in ALL modes (including off):
500-
// even with retry disabled, a hung streaming body must not pin sockets forever.
501-
const effectiveStreamIdleMs = cfg.streamIdleTimeoutMs > 0 ? cfg.streamIdleTimeoutMs : 0;
499+
// streamIdleTimeoutMs is gated to retry modes only (NOT off), mirroring the
500+
// connectTimeoutMs off-exclusion above. The watchdog wraps response.body in a
501+
// TransformStream, but the interceptor (server/interceptor.js) already
502+
// reconstructs response.body via getReader() + a new ReadableStream for
503+
// logging/live-streaming; under concurrent load the watchdog's pipeThrough
504+
// races that reconstruction and surfaces as a spurious `fetch failed` →
505+
// status 0 → 502. off mode is the legacy pass-through path (no retry), so the
506+
// watchdog's value (bound idle on a hung stream) is marginal here and the
507+
// interceptor already observes the stream — serial/race/stagger keep the guard.
508+
const effectiveStreamIdleMs = cfg.mode === 'off' ? 0 : (cfg.streamIdleTimeoutMs > 0 ? cfg.streamIdleTimeoutMs : 0);
502509
const commonCtx = { dispatcher, connectTimeoutMs: effectiveConnectTimeoutMs, streamIdleTimeoutMs: effectiveStreamIdleMs };
503510

504511
if (cfg.mode === 'off' || cfg.mode === 'serial') {

0 commit comments

Comments
 (0)