From 9adc3eb1c58f207051073b04bf46151fe4dea185 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 28 Jul 2026 08:14:53 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=95=B0=EF=B8=8F=20fix:=20Warn=20When=20HT?= =?UTF-8?q?TP=20Timeouts=20Are=20Not=20Enforced?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .env.example | 3 ++ packages/api/src/app/server.spec.ts | 50 ++++++++++++++++++++++++++++- packages/api/src/app/server.ts | 33 +++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/.env.example b/.env.example index 1212ad64d5..0cc8f2fa6c 100644 --- a/.env.example +++ b/.env.example @@ -16,6 +16,9 @@ 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 timeouts are only swept every 30s, so values below 30000 round up to it. +# 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 diff --git a/packages/api/src/app/server.spec.ts b/packages/api/src/app/server.spec.ts index e9d65ba9b4..9788c6b16c 100644 --- a/packages/api/src/app/server.spec.ts +++ b/packages/api/src/app/server.spec.ts @@ -1,8 +1,19 @@ import { createServer } from 'node:http'; - +import { logger } from '@librechat/data-schemas'; import { configureServerTimeouts } from './server'; describe('configureServerTimeouts', () => { + let warn: jest.SpyInstance; + + beforeEach(() => { + warn = jest.spyOn(logger, 'warn').mockImplementation(() => logger); + }); + + afterEach(() => { + warn.mockRestore(); + delete process.versions.bun; + }); + it('preserves Node.js defaults when variables are unset', () => { const server = createServer(); const defaults = { @@ -53,4 +64,41 @@ describe('configureServerTimeouts', () => { expect(server.headersTimeout).toBe(defaultHeadersTimeout); expect(server.requestTimeout).toBe(0); }); + + it('warns that Bun does not enforce the configured timeouts', () => { + process.versions.bun = '1.3.13'; + + configureServerTimeouts(createServer(), { HTTP_KEEP_ALIVE_TIMEOUT_MS: '70000' }); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Bun does not enforce them')); + }); + + it('stays quiet under Bun when no timeout is configured', () => { + process.versions.bun = '1.3.13'; + + configureServerTimeouts(createServer(), {}); + + 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', + }); + + 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', + }); + + expect(warn).not.toHaveBeenCalled(); + }); }); diff --git a/packages/api/src/app/server.ts b/packages/api/src/app/server.ts index 45ac8a1dda..f6d1622f3e 100644 --- a/packages/api/src/app/server.ts +++ b/packages/api/src/app/server.ts @@ -1,5 +1,13 @@ +import { logger } from '@librechat/data-schemas'; import type { Server } from 'node:http'; +/** + * Node sweeps header/request timeouts on `connectionsCheckingInterval`, which is a + * `createServer` option that `app.listen()` leaves at its 30s default. Values below this + * are rounded up to it. `keepAliveTimeout` is socket-driven and stays exact. + */ +const TIMEOUT_SWEEP_RESOLUTION_MS = 30_000; + const parseTimeout = (value?: string): number | undefined => { if (value == null || value.trim() === '') { return undefined; @@ -30,4 +38,29 @@ export const configureServerTimeouts = ( if (requestTimeout != null) { server.requestTimeout = requestTimeout; } + + const configured = [keepAliveTimeout, keepAliveTimeoutBuffer, headersTimeout, requestTimeout]; + if (configured.every((value) => value == null)) { + return; + } + + /** Bun accepts and reflects these assignments back without enforcing them. */ + if (process.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 once per sweep, so the effective timeout is up to ${TIMEOUT_SWEEP_RESOLUTION_MS}ms.`, + ); + } };