mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
⏱️ feat: Configurable HTTP Server Timeouts (#14481)
* 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 <peter.rothlaender@ginkgo.com>
This commit is contained in:
parent
f7bc50ae5b
commit
324584552c
8 changed files with 327 additions and 1 deletions
13
.env.example
13
.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=
|
||||
|
|
|
|||
|
|
@ -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) => {
|
||||
|
|
|
|||
15
api/server/experimental.spec.js
Normal file
15
api/server/experimental.spec.js
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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);
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -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);",
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
|
|
|||
175
packages/api/src/app/server.spec.ts
Normal file
175
packages/api/src/app/server.spec.ts
Normal file
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
92
packages/api/src/app/server.ts
Normal file
92
packages/api/src/app/server.ts
Normal file
|
|
@ -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.`,
|
||||
);
|
||||
}
|
||||
};
|
||||
Loading…
Add table
Add a link
Reference in a new issue