From 324584552c158450cefaed50a1bc8fe2805da6bc Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 28 Jul 2026 09:10:17 -0400 Subject: [PATCH] =?UTF-8?q?=E2=8F=B1=EF=B8=8F=20feat:=20Configurable=20HTT?= =?UTF-8?q?P=20Server=20Timeouts=20(#14481)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * http server config added * Fix TypeScript compatibility by accepting NodeJS.ProcessEnv directly when applying optional HTTP server timeout configuration. * fix(api): configure HTTP server timeouts for clustered workers * 🕰️ fix: Warn When HTTP Timeouts Are Not Enforced Codex review of the rebased contributor work surfaced two ways these settings silently do nothing. Both reproduce, and neither was reported to the operator. Bun accepts the four property assignments and reflects them back, but does not enforce them: with keepAliveTimeout=100 and buffer=1000, Bun 1.3.13 held a keep-alive connection past 3s where Node 24 closed it at 1101ms. Since `b:api` runs the server under Bun, the existing info log confirmed a configuration that was not in effect. Warn instead. Node sweeps header/request timeouts on `connectionsCheckingInterval`, a createServer option that `app.listen()` leaves at 30s, so sub-30s values round up to it: headersTimeout=2000 returned 408 at 30004ms by default versus 2010ms with a 250ms interval. Warn on values below the sweep interval rather than restructure server construction, since every documented value and both Node defaults already sit well above it. keepAliveTimeout is socket-driven and stays exact, so it is excluded. Both caveats documented in .env.example. * 🩹 fix: Inject Runtime Versions Instead of Mutating `process.versions` The spec deleted `process.versions.bun` to reset between cases, which failed typecheck with TS2790: `@types/bun` is a packages/api dependency and augments NodeJS.ProcessVersions with a required `bun: string`, so the property is not optional and cannot be deleted. Assigning undefined would fail for the same reason. That augmentation also made the production check dishonest: TypeScript saw `process.versions.bun` as always a string, so `!= null` read as a no-op branch even though it is correct at runtime under Node. Both resolved by taking runtime versions as a third injectable parameter, matching the existing `environment` parameter. Callers in api/server are unchanged, the narrow `{ bun?: string }` type restores honest narrowing, and the tests no longer mutate global state, so they assert the same behavior whether the suite runs under Node or `bun jest`. * 📏 fix: Stop Claiming a Ceiling on Sweep-Delayed Timeouts The warning added in 9adc3eb1c said the effective timeout is "up to 30000ms", which promises a bound that does not hold. Node detects header/request expiry only on the next connection sweep, so the delay is relative to the deadline rather than capped by the interval: measured against the default 30s sweep, a 2000ms headersTimeout closed at 30004ms, 15x the configured value, and cases where the timeout is near the interval did not fire within a 9s window at all. Reworded to state the mechanism without asserting a ceiling, and to point operators at values of 30000ms or above for predictable enforcement. Same correction applied to the .env.example note, which claimed short values "round up" to the interval. * ⏳ fix: Clamp Headers Timeout to the Request Timeout Setting only HTTP_REQUEST_TIMEOUT_MS below the 60s headers default left headersTimeout > requestTimeout, a pairing createServer rejects outright with ERR_OUT_OF_RANGE. Assigning the properties after construction skips that validation, and the mismatch silently defeats the request timeout for a stalled body: with requestTimeout=4000 and headersTimeout at its 60000 default, a client that completed its headers and then stopped mid-body was still connected after 12s. Clamping headersTimeout to 4000 closes the same connection at 4017ms. An earlier round dismissed this after testing partial *headers*, where requestTimeout does evict on time. The gap only appears once headers are complete and the body stalls, which is the case these timeouts exist to bound. Mirrors Node's own rule rather than its constructor default: zero on either side means disabled and is left alone, and an explicitly configured HTTP_HEADERS_TIMEOUT_MS that conflicts is warned about before being clamped instead of failing startup over a config typo. --------- Co-authored-by: Peter Rothlaender --- .env.example | 13 +++ api/server/experimental.js | 11 +- api/server/experimental.spec.js | 15 +++ api/server/index.js | 9 ++ api/server/index.spec.js | 12 ++ packages/api/src/app/index.ts | 1 + packages/api/src/app/server.spec.ts | 175 ++++++++++++++++++++++++++++ packages/api/src/app/server.ts | 92 +++++++++++++++ 8 files changed, 327 insertions(+), 1 deletion(-) create mode 100644 api/server/experimental.spec.js create mode 100644 packages/api/src/app/server.spec.ts create mode 100644 packages/api/src/app/server.ts diff --git a/.env.example b/.env.example index 8ffb4cd1f4..a51cf71664 100644 --- a/.env.example +++ b/.env.example @@ -14,6 +14,19 @@ HOST=localhost PORT=3080 +# Optional Node.js HTTP server timeouts in milliseconds. When unset, Node.js defaults apply. +# For an ALB, set the application keep-alive timeout above the ALB idle timeout. +# Requires Node.js: Bun accepts these values but does not enforce them. +# Header and request timeout expiry is only detected on a 30s connection sweep, so values +# below 30000 take effect late and are not enforced at the precision configured. +# The header timeout is clamped to the request timeout when the latter is lower, since Node +# does not enforce a request timeout that a longer header timeout sits above. +# Keep-alive is socket-driven and remains exact at any value. +# HTTP_KEEP_ALIVE_TIMEOUT_MS=70000 +# HTTP_KEEP_ALIVE_TIMEOUT_BUFFER_MS=5000 +# HTTP_HEADERS_TIMEOUT_MS=80000 +# HTTP_REQUEST_TIMEOUT_MS=300000 + MONGO_URI=mongodb://127.0.0.1:27017/LibreChat #The maximum number of connections in the connection pool. */ MONGO_MAX_POOL_SIZE= diff --git a/api/server/experimental.js b/api/server/experimental.js index dadb87dd0b..2632c2d2bc 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -23,6 +23,7 @@ const { loadToolApprovalHooks, maybeInjectQueryDevtoolsBootstrap, preAuthTenantMiddleware, + configureServerTimeouts, } = require('@librechat/api'); const { connectDb, indexSync } = require('~/db'); const initializeOAuthReconnectManager = require('./services/initializeOAuthReconnectManager'); @@ -450,7 +451,7 @@ if (cluster.isMaster) { app.use(ErrorController); /** Start listening on shared port (cluster will distribute connections) */ - app.listen(port, host, async (err) => { + const server = app.listen(port, host, async (err) => { if (err) { logger.error(`Worker ${process.pid} failed to start server:`, err); process.exit(1); @@ -479,6 +480,14 @@ if (cluster.isMaster) { process.exit(1); } }); + + configureServerTimeouts(server); + logger.info(`Worker ${process.pid} HTTP server timeout configuration`, { + keepAliveTimeout: server.keepAliveTimeout, + keepAliveTimeoutBuffer: server.keepAliveTimeoutBuffer, + headersTimeout: server.headersTimeout, + requestTimeout: server.requestTimeout, + }); }; startServer().catch((err) => { diff --git a/api/server/experimental.spec.js b/api/server/experimental.spec.js new file mode 100644 index 0000000000..db58e7c20c --- /dev/null +++ b/api/server/experimental.spec.js @@ -0,0 +1,15 @@ +const fs = require('fs'); +const path = require('path'); + +describe('Experimental server configuration', () => { + const source = fs.readFileSync(path.join(__dirname, 'experimental.js'), 'utf8'); + + it('configures HTTP timeouts for each cluster worker server', () => { + const listenIndex = source.indexOf('const server = app.listen'); + const timeoutConfigIndex = source.indexOf('configureServerTimeouts(server);'); + + expect(listenIndex).toBeGreaterThan(-1); + expect(timeoutConfigIndex).toBeGreaterThan(-1); + expect(listenIndex).toBeLessThan(timeoutConfigIndex); + }); +}); diff --git a/api/server/index.js b/api/server/index.js index be3b8a1fe0..eca877e8de 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -29,6 +29,7 @@ const { maybeInjectQueryDevtoolsBootstrap, preAuthTenantMiddleware, registerShutdownTask, + configureServerTimeouts, setupGracefulShutdown, updateInterfacePermissions, } = require('@librechat/api'); @@ -363,6 +364,14 @@ const startServer = async () => { } }); + configureServerTimeouts(server); + logger.info('HTTP server timeout configuration', { + keepAliveTimeout: server.keepAliveTimeout, + keepAliveTimeoutBuffer: server.keepAliveTimeoutBuffer, + headersTimeout: server.headersTimeout, + requestTimeout: server.requestTimeout, + }); + setupGracefulShutdown(server); }; diff --git a/api/server/index.spec.js b/api/server/index.spec.js index e73ee40f60..9945c51253 100644 --- a/api/server/index.spec.js +++ b/api/server/index.spec.js @@ -126,6 +126,18 @@ describe('Startup readiness wiring', () => { expect(shutdownRegistrationIndex).toBeLessThan(listenIndex); }); + it('configures HTTP timeouts before graceful shutdown handling', () => { + const listenIndex = source.indexOf('const server = app.listen'); + const timeoutConfigIndex = source.indexOf('configureServerTimeouts(server);'); + const shutdownIndex = source.indexOf('setupGracefulShutdown(server);'); + + expect(listenIndex).toBeGreaterThan(-1); + expect(timeoutConfigIndex).toBeGreaterThan(-1); + expect(shutdownIndex).toBeGreaterThan(-1); + expect(listenIndex).toBeLessThan(timeoutConfigIndex); + expect(timeoutConfigIndex).toBeLessThan(shutdownIndex); + }); + it('mounts the chat-start readiness gate before agent routes', () => { const readinessGateIndex = source.indexOf( "app.use('/api/agents/chat', rejectChatStartsUntilReady);", diff --git a/packages/api/src/app/index.ts b/packages/api/src/app/index.ts index b5380612d2..b5962a45f7 100644 --- a/packages/api/src/app/index.ts +++ b/packages/api/src/app/index.ts @@ -6,5 +6,6 @@ export * from './cdn'; export * from './checks'; export * from './resolve'; export * from './shutdown'; +export * from './server'; export { resolveBuildInfo } from './build'; export type { BuildInfo } from './build'; diff --git a/packages/api/src/app/server.spec.ts b/packages/api/src/app/server.spec.ts new file mode 100644 index 0000000000..498dd73658 --- /dev/null +++ b/packages/api/src/app/server.spec.ts @@ -0,0 +1,175 @@ +import { createServer } from 'node:http'; +import { logger } from '@librechat/data-schemas'; +import { configureServerTimeouts } from './server'; + +describe('configureServerTimeouts', () => { + const NODE = {}; + const BUN = { bun: '1.3.13' }; + + let warn: jest.SpyInstance; + + beforeEach(() => { + warn = jest.spyOn(logger, 'warn').mockImplementation(() => logger); + }); + + afterEach(() => { + warn.mockRestore(); + }); + + it('preserves Node.js defaults when variables are unset', () => { + const server = createServer(); + const defaults = { + keepAliveTimeout: server.keepAliveTimeout, + keepAliveTimeoutBuffer: server.keepAliveTimeoutBuffer, + headersTimeout: server.headersTimeout, + requestTimeout: server.requestTimeout, + }; + + configureServerTimeouts(server, {}); + + expect(server.keepAliveTimeout).toBe(defaults.keepAliveTimeout); + expect(server.keepAliveTimeoutBuffer).toBe(defaults.keepAliveTimeoutBuffer); + expect(server.headersTimeout).toBe(defaults.headersTimeout); + expect(server.requestTimeout).toBe(defaults.requestTimeout); + }); + + it('applies configured timeout values', () => { + const server = createServer(); + + configureServerTimeouts(server, { + HTTP_KEEP_ALIVE_TIMEOUT_MS: '70000', + HTTP_KEEP_ALIVE_TIMEOUT_BUFFER_MS: '5000', + HTTP_HEADERS_TIMEOUT_MS: '80000', + HTTP_REQUEST_TIMEOUT_MS: '300000', + }); + + expect(server.keepAliveTimeout).toBe(70_000); + expect(server.keepAliveTimeoutBuffer).toBe(5_000); + expect(server.headersTimeout).toBe(80_000); + expect(server.requestTimeout).toBe(300_000); + }); + + it('ignores invalid values and permits zero to disable a timeout', () => { + const server = createServer(); + const defaultKeepAliveTimeout = server.keepAliveTimeout; + const defaultHeadersTimeout = server.headersTimeout; + + configureServerTimeouts(server, { + HTTP_KEEP_ALIVE_TIMEOUT_MS: '-1', + HTTP_KEEP_ALIVE_TIMEOUT_BUFFER_MS: '0', + HTTP_HEADERS_TIMEOUT_MS: 'not-a-number', + HTTP_REQUEST_TIMEOUT_MS: '0', + }); + + expect(server.keepAliveTimeout).toBe(defaultKeepAliveTimeout); + expect(server.keepAliveTimeoutBuffer).toBe(0); + expect(server.headersTimeout).toBe(defaultHeadersTimeout); + expect(server.requestTimeout).toBe(0); + }); + + it('clamps the headers timeout when only a lower request timeout is configured', () => { + const server = createServer(); + + configureServerTimeouts(server, { HTTP_REQUEST_TIMEOUT_MS: '45000' }, NODE); + + expect(server.requestTimeout).toBe(45_000); + expect(server.headersTimeout).toBe(45_000); + expect(warn).not.toHaveBeenCalled(); + }); + + it('leaves the headers timeout alone when it already fits the request timeout', () => { + const server = createServer(); + + configureServerTimeouts(server, { HTTP_REQUEST_TIMEOUT_MS: '300000' }, NODE); + + expect(server.headersTimeout).toBe(60_000); + }); + + it('warns and clamps when both timeouts are configured in conflict', () => { + const server = createServer(); + + configureServerTimeouts( + server, + { HTTP_HEADERS_TIMEOUT_MS: '80000', HTTP_REQUEST_TIMEOUT_MS: '45000' }, + NODE, + ); + + expect(server.headersTimeout).toBe(45_000); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('a pairing Node rejects')); + }); + + it('treats zero as disabled on either side rather than clamping', () => { + const disabledRequest = createServer(); + configureServerTimeouts( + disabledRequest, + { HTTP_HEADERS_TIMEOUT_MS: '5000', HTTP_REQUEST_TIMEOUT_MS: '0' }, + NODE, + ); + expect(disabledRequest.headersTimeout).toBe(5_000); + + const disabledHeaders = createServer(); + configureServerTimeouts( + disabledHeaders, + { HTTP_HEADERS_TIMEOUT_MS: '0', HTTP_REQUEST_TIMEOUT_MS: '10000' }, + NODE, + ); + expect(disabledHeaders.headersTimeout).toBe(0); + }); + + it('produces a pairing Node itself accepts', () => { + const server = createServer(); + + configureServerTimeouts(server, { HTTP_REQUEST_TIMEOUT_MS: '45000' }, NODE); + + expect(() => + createServer({ + headersTimeout: server.headersTimeout, + requestTimeout: server.requestTimeout, + }).close(), + ).not.toThrow(); + }); + + it('warns that Bun does not enforce the configured timeouts', () => { + configureServerTimeouts(createServer(), { HTTP_KEEP_ALIVE_TIMEOUT_MS: '70000' }, BUN); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Bun does not enforce them')); + }); + + it('stays quiet under Bun when no timeout is configured', () => { + configureServerTimeouts(createServer(), {}, BUN); + + expect(warn).not.toHaveBeenCalled(); + }); + + it('does not warn about the runtime under Node.js', () => { + configureServerTimeouts(createServer(), { HTTP_KEEP_ALIVE_TIMEOUT_MS: '70000' }, NODE); + + expect(warn).not.toHaveBeenCalled(); + }); + + it('warns when header or request timeouts fall below the connection sweep interval', () => { + configureServerTimeouts( + createServer(), + { HTTP_HEADERS_TIMEOUT_MS: '5000', HTTP_REQUEST_TIMEOUT_MS: '10000' }, + NODE, + ); + + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('HTTP_HEADERS_TIMEOUT_MS, HTTP_REQUEST_TIMEOUT_MS'), + ); + }); + + it('does not warn about sweep resolution for zero or above-interval timeouts', () => { + configureServerTimeouts( + createServer(), + { + HTTP_KEEP_ALIVE_TIMEOUT_MS: '1000', + HTTP_HEADERS_TIMEOUT_MS: '80000', + HTTP_REQUEST_TIMEOUT_MS: '0', + }, + NODE, + ); + + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/api/src/app/server.ts b/packages/api/src/app/server.ts new file mode 100644 index 0000000000..23c18980f8 --- /dev/null +++ b/packages/api/src/app/server.ts @@ -0,0 +1,92 @@ +import { logger } from '@librechat/data-schemas'; +import type { Server } from 'node:http'; + +/** + * Node detects header/request timeout expiry only on `connectionsCheckingInterval`, a + * `createServer` option that `app.listen()` leaves at its 30s default, so short values + * take effect late rather than at the configured deadline. `keepAliveTimeout` is + * socket-driven and stays exact. + */ +const TIMEOUT_SWEEP_RESOLUTION_MS = 30_000; + +/** + * `createServer` throws ERR_OUT_OF_RANGE unless `headersTimeout <= requestTimeout`, treating + * zero on either side as disabled. Assigning the properties afterwards skips that check, and + * the resulting mismatch leaves a stalled request body open past the request timeout. + */ +const clampHeadersToRequestTimeout = (server: Server, configuredHeadersTimeout?: number): void => { + if (server.requestTimeout === 0 || server.headersTimeout === 0) { + return; + } + if (server.headersTimeout <= server.requestTimeout) { + return; + } + + const clamped = server.requestTimeout; + if (configuredHeadersTimeout != null) { + logger.warn( + `HTTP_HEADERS_TIMEOUT_MS (${configuredHeadersTimeout}ms) exceeds HTTP_REQUEST_TIMEOUT_MS (${clamped}ms), a pairing Node rejects; clamped to the request timeout.`, + ); + } + server.headersTimeout = clamped; +}; + +const parseTimeout = (value?: string): number | undefined => { + if (value == null || value.trim() === '') { + return undefined; + } + + const timeout = Number(value); + return Number.isSafeInteger(timeout) && timeout >= 0 ? timeout : undefined; +}; + +export const configureServerTimeouts = ( + server: Server, + environment: NodeJS.ProcessEnv = process.env, + versions: { bun?: string } = process.versions, +): void => { + const keepAliveTimeout = parseTimeout(environment.HTTP_KEEP_ALIVE_TIMEOUT_MS); + const keepAliveTimeoutBuffer = parseTimeout(environment.HTTP_KEEP_ALIVE_TIMEOUT_BUFFER_MS); + const headersTimeout = parseTimeout(environment.HTTP_HEADERS_TIMEOUT_MS); + const requestTimeout = parseTimeout(environment.HTTP_REQUEST_TIMEOUT_MS); + + if (keepAliveTimeout != null) { + server.keepAliveTimeout = keepAliveTimeout; + } + if (keepAliveTimeoutBuffer != null) { + server.keepAliveTimeoutBuffer = keepAliveTimeoutBuffer; + } + if (headersTimeout != null) { + server.headersTimeout = headersTimeout; + } + if (requestTimeout != null) { + server.requestTimeout = requestTimeout; + } + + clampHeadersToRequestTimeout(server, headersTimeout); + + const configured = [keepAliveTimeout, keepAliveTimeoutBuffer, headersTimeout, requestTimeout]; + if (configured.every((value) => value == null)) { + return; + } + + /** Bun accepts and reflects these assignments back without enforcing them. */ + if (versions.bun != null) { + logger.warn( + 'HTTP server timeouts are configured but Bun does not enforce them; run under Node.js for these settings to take effect.', + ); + } + + const belowResolution = [ + { name: 'HTTP_HEADERS_TIMEOUT_MS', value: headersTimeout }, + { name: 'HTTP_REQUEST_TIMEOUT_MS', value: requestTimeout }, + ] + .filter(({ value }) => value != null && value > 0 && value < TIMEOUT_SWEEP_RESOLUTION_MS) + .map(({ name }) => name); + + if (belowResolution.length > 0) { + logger.warn( + `${belowResolution.join(', ')} set below the ${TIMEOUT_SWEEP_RESOLUTION_MS}ms connection sweep interval; expiry is only detected on the next sweep, so the connection can stay open well past the configured deadline. Use values at or above ${TIMEOUT_SWEEP_RESOLUTION_MS}ms for predictable enforcement.`, + ); + } +};