diff --git a/src/ExpressSetup.ts b/src/ExpressSetup.ts index de08f39..00a2b12 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,9 +85,15 @@ 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.status(504); - res.send("Gateway Timeout"); + res.setTimeout(RESPONSE_TIMEOUT_MS, () => { + if (res.writableEnded) { + return; + } + if (res.headersSent) { + res.destroy(); + return; + } + res.status(504).send("Gateway Timeout"); }); try { await thumbnailApi.handle(req, res); diff --git a/src/ResponseHelper.ts b/src/ResponseHelper.ts index 6b2f574..e34c763 100644 --- a/src/ResponseHelper.ts +++ b/src/ResponseHelper.ts @@ -2,11 +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 { - FETCH_TIMEOUT = 10 * 1000; // 10 seconds; + FETCH_TIMEOUT = FETCH_TIMEOUT_MS; async pipe(body: ReadableStream, expressResponse: express.Response): Promise { try { @@ -99,7 +100,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([ diff --git a/src/timeoutConfig.ts b/src/timeoutConfig.ts new file mode 100644 index 0000000..2f92764 --- /dev/null +++ b/src/timeoutConfig.ts @@ -0,0 +1,17 @@ +// 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; + +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})`, + ); +}