From b6b6c8f05a638b1dedf84524c2c5f11855fa723d Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Mon, 13 Apr 2026 11:32:08 -0400 Subject: [PATCH 1/5] Fix race condition between fetch AbortSignal and Express response timeout FETCH_TIMEOUT and RESPONSE_TIMEOUT were both 10s, causing them to fire simultaneously when an upstream host was unreachable. The AbortSignal would throw (triggering sendError 404) at the same moment res.setTimeout sent a 504, resulting in ERR_HTTP_HEADERS_SENT errors in production. Reduce FETCH_TIMEOUT to 5s so the 404 path always completes well before the 10s response timeout fires. Also replace new Date().getTime() with Date.now() in getCacheHeaders. Co-Authored-By: Claude Sonnet 4.6 --- src/ResponseHelper.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ResponseHelper.ts b/src/ResponseHelper.ts index 6b2f574..87cbcf7 100644 --- a/src/ResponseHelper.ts +++ b/src/ResponseHelper.ts @@ -6,7 +6,9 @@ import { getLogger } from "./logger"; const logger = getLogger(); export class ResponseHelper { - FETCH_TIMEOUT = 10 * 1000; // 10 seconds; + // Must be well under RESPONSE_TIMEOUT (10s) in ExpressSetup so that AbortSignal fires + // and sendError(404) completes before res.setTimeout sends a 504, preventing ERR_HTTP_HEADERS_SENT. + FETCH_TIMEOUT = 5 * 1000; // 5 seconds async pipe(body: ReadableStream, expressResponse: express.Response): Promise { try { @@ -99,7 +101,7 @@ export class ResponseHelper { // whereas s3 responses should live there for a long time // see LONG_CACHE_TIME and SHORT_CACHE_TIME, above getCacheHeaders(seconds: number): Map { - const now = new Date().getTime(); + const now = Date.now(); const expirationDateString = new Date(now + 1000 * seconds).toUTCString(); const cacheControl = `public, max-age=${String(seconds)}`; return new Map([ From 593b55366d73bcc1e489dc3c5bbff2638ef921db Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Mon, 13 Apr 2026 13:15:07 -0400 Subject: [PATCH 2/5] Extract timeout constants to shared timeoutConfig module Addresses CodeRabbit nitpick: FETCH_TIMEOUT and RESPONSE_TIMEOUT were only loosely coupled via comments, so a future change to one could silently reintroduce the race condition. Both values now live in timeoutConfig.ts as the single source of truth, with the constraint documented there. Co-Authored-By: Claude Sonnet 4.6 --- src/ExpressSetup.ts | 11 +++-------- src/ResponseHelper.ts | 5 ++--- src/timeoutConfig.ts | 11 +++++++++++ 3 files changed, 16 insertions(+), 11 deletions(-) create mode 100644 src/timeoutConfig.ts diff --git a/src/ExpressSetup.ts b/src/ExpressSetup.ts index de08f39..b088e4e 100644 --- a/src/ExpressSetup.ts +++ b/src/ExpressSetup.ts @@ -5,12 +5,7 @@ import express, { Express } from "express"; import * as Http from "node:http"; import { ThumbnailApi } from "./ThumbnailApi"; import { Server } from "node:http"; - -// How long we wait for a request from a socket -const REQUEST_TIMEOUT = 3000; // 3 secs - -// How long we wait on piping a response before we give up -const RESPONSE_TIMEOUT = 10000; // 10 seconds +import { REQUEST_TIMEOUT_MS, RESPONSE_TIMEOUT_MS } from "./timeoutConfig"; export class ExpressSetup { app: Express; @@ -77,7 +72,7 @@ export class ExpressSetup { } setRequestTimeout(server: Http.Server) { - server.requestTimeout = REQUEST_TIMEOUT; + server.requestTimeout = REQUEST_TIMEOUT_MS; } server(port: number): Http.Server { @@ -90,7 +85,7 @@ export class ExpressSetup { // next two methods are like this to make // eslint happy about the async get handler const handler = async (req: express.Request, res: express.Response) => { - res.setTimeout(RESPONSE_TIMEOUT, () => { + res.setTimeout(RESPONSE_TIMEOUT_MS, () => { res.status(504); res.send("Gateway Timeout"); }); diff --git a/src/ResponseHelper.ts b/src/ResponseHelper.ts index 87cbcf7..e34c763 100644 --- a/src/ResponseHelper.ts +++ b/src/ResponseHelper.ts @@ -2,13 +2,12 @@ import { Readable } from "stream"; import { pipeline } from "stream/promises"; import express from "express"; import { getLogger } from "./logger"; +import { FETCH_TIMEOUT_MS } from "./timeoutConfig"; const logger = getLogger(); export class ResponseHelper { - // Must be well under RESPONSE_TIMEOUT (10s) in ExpressSetup so that AbortSignal fires - // and sendError(404) completes before res.setTimeout sends a 504, preventing ERR_HTTP_HEADERS_SENT. - FETCH_TIMEOUT = 5 * 1000; // 5 seconds + FETCH_TIMEOUT = FETCH_TIMEOUT_MS; async pipe(body: ReadableStream, expressResponse: express.Response): Promise { try { diff --git a/src/timeoutConfig.ts b/src/timeoutConfig.ts new file mode 100644 index 0000000..de0b8bd --- /dev/null +++ b/src/timeoutConfig.ts @@ -0,0 +1,11 @@ +// REQUEST_TIMEOUT: how long we wait for a request from a socket +export const REQUEST_TIMEOUT_MS = 3_000; + +// RESPONSE_TIMEOUT: how long before Express gives up on the full response +export const RESPONSE_TIMEOUT_MS = 10_000; + +// FETCH_TIMEOUT: how long to wait on an upstream image fetch. +// Must be well under RESPONSE_TIMEOUT_MS so that AbortSignal fires and +// sendError(404) completes before res.setTimeout sends a 504, +// preventing ERR_HTTP_HEADERS_SENT. +export const FETCH_TIMEOUT_MS = 5_000; From dcd8baa47a4e57885929c5a0de1eb96085ffd54e Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Tue, 14 Apr 2026 00:55:10 -0400 Subject: [PATCH 3/5] fix: guard timeout handler against ERR_HTTP_HEADERS_SENT Check res.headersSent and res.writableEnded before sending 504 in the res.setTimeout callback to prevent double-write errors when the main handler completes before the timeout fires. Co-Authored-By: Claude Sonnet 4.6 --- src/ExpressSetup.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ExpressSetup.ts b/src/ExpressSetup.ts index b088e4e..3adb861 100644 --- a/src/ExpressSetup.ts +++ b/src/ExpressSetup.ts @@ -86,8 +86,10 @@ export class ExpressSetup { // eslint happy about the async get handler const handler = async (req: express.Request, res: express.Response) => { res.setTimeout(RESPONSE_TIMEOUT_MS, () => { - res.status(504); - res.send("Gateway Timeout"); + if (res.headersSent || res.writableEnded) { + return; + } + res.status(504).send("Gateway Timeout"); }); try { await thumbnailApi.handle(req, res); From 282c5c3b2bae0ffb92aea5e2374e84be3beec492 Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Tue, 14 Apr 2026 02:04:00 -0400 Subject: [PATCH 4/5] fix: destroy stalled response when timeout fires after headers sent When headersSent is true but writableEnded is false (response started but body stream stalled), returning from the timeout callback leaves the socket open indefinitely. Explicitly call res.destroy() to close the connection in that case. Co-Authored-By: Claude Sonnet 4.6 --- src/ExpressSetup.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ExpressSetup.ts b/src/ExpressSetup.ts index 3adb861..00a2b12 100644 --- a/src/ExpressSetup.ts +++ b/src/ExpressSetup.ts @@ -86,7 +86,11 @@ export class ExpressSetup { // eslint happy about the async get handler const handler = async (req: express.Request, res: express.Response) => { res.setTimeout(RESPONSE_TIMEOUT_MS, () => { - if (res.headersSent || res.writableEnded) { + if (res.writableEnded) { + return; + } + if (res.headersSent) { + res.destroy(); return; } res.status(504).send("Gateway Timeout"); From 8843b47a05d687fb67b524bc38c472ae3c09cffb Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Tue, 14 Apr 2026 10:08:43 -0400 Subject: [PATCH 5/5] fix: assert FETCH_TIMEOUT_MS < RESPONSE_TIMEOUT_MS at startup Co-Authored-By: Claude Sonnet 4.6 --- src/timeoutConfig.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/timeoutConfig.ts b/src/timeoutConfig.ts index de0b8bd..2f92764 100644 --- a/src/timeoutConfig.ts +++ b/src/timeoutConfig.ts @@ -9,3 +9,9 @@ export const RESPONSE_TIMEOUT_MS = 10_000; // sendError(404) completes before res.setTimeout sends a 504, // preventing ERR_HTTP_HEADERS_SENT. export const FETCH_TIMEOUT_MS = 5_000; + +if (FETCH_TIMEOUT_MS >= RESPONSE_TIMEOUT_MS) { + throw new Error( + `Invalid timeout config: FETCH_TIMEOUT_MS (${FETCH_TIMEOUT_MS}) must be less than RESPONSE_TIMEOUT_MS (${RESPONSE_TIMEOUT_MS})`, + ); +}