mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
324584552c
* 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>
15 lines
585 B
JavaScript
15 lines
585 B
JavaScript
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);
|
|
});
|
|
});
|