mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
feat(server): add last-resort error handlers, a draining readiness probe and proxy keep-alive
Three small gaps in how the process behaves at its edges: - An unhandled rejection or uncaught exception crashed the process with the stack on stderr only. index.ts now logs it to trek.log and runs the same orderly shutdown a SIGTERM gets, exiting with 1; a second fatal error during that shutdown exits at once. - /api/health/ready kept answering 200 while a SIGTERM was draining the process, so an orchestrator sent traffic for another probe period. A shutdown now marks the new ReadinessService first (and Nest's own shutdown hook marks it too), and the probe answers 503 from then on. - Node closes an idle keep-alive connection after 5 seconds while reverse proxies keep upstream connections for about a minute, which shows up as intermittent 502s. The server now keeps them for 65 seconds, configurable through HTTP_KEEP_ALIVE_TIMEOUT_MS (owned by the httpConfig token), with the headers timeout one second above.
This commit is contained in:
@@ -24,6 +24,7 @@ ALLOWED_ORIGINS=https://trek.example.com # Comma-separated origins for CORS and
|
||||
FORCE_HTTPS=false # Optional. When true: HTTPS redirect + HSTS + CSP upgrade-insecure-requests + secure cookies. Only behind a TLS proxy.
|
||||
# HSTS_INCLUDE_SUBDOMAINS=false # When true: adds includeSubDomains to the HSTS header. Only effective when HSTS is active (FORCE_HTTPS=true or NODE_ENV=production). Leave false if you run other services on sibling subdomains over plain HTTP.
|
||||
COOKIE_SECURE=true # Auto-derived (true when NODE_ENV=production or FORCE_HTTPS=true). Set false to force cookies over plain HTTP.
|
||||
# HTTP_KEEP_ALIVE_TIMEOUT_MS=65000 # How long an idle keep-alive connection stays open. Keep it above your reverse proxy's upstream idle timeout (60 s for nginx and most load balancers, 90 s for Traefik) or the proxy answers some requests with a 502. 1000 to 290000. (default: 65000)
|
||||
TRUST_PROXY=1 # Trusted proxy hops (parseInt or 1). Active in production by default; off in dev unless set. Needed for FORCE_HTTPS.
|
||||
ALLOW_INTERNAL_NETWORK=false # Allow outbound requests to private/RFC1918 IPs (e.g. Immich hosted on your LAN). Loopback and link-local addresses are always blocked.
|
||||
# ALLOW_LINK_LOCAL_IPS=169.254.1.2 # Single link-local addresses TREK may reach after all, comma-separated, e.g. the host gateway of a rootless Podman container. OIDC reaches a listed address directly; other integrations also need ALLOW_INTERNAL_NETWORK=true. 169.254.169.x and 169.254.170.x (cloud metadata) cannot be listed.
|
||||
|
||||
@@ -96,6 +96,7 @@ SQLite is the engine today and Postgres is the planned second one. Code under `s
|
||||
- **Auth is default-deny**: the global auth guard and the MFA policy guard run as `APP_GUARD`s, followed by the managed-install guard (`managed.guard.ts`), which refuses the routes a centrally administered install withholds from its own admin — it runs last so strangers still get a 401, never a route-revealing 403. Routes opt out with `@Public()`/`@OptionalAuth()`. The global guard stands down for routes declaring their own guard chain, so `@UseGuards(JwtAuthGuard, TripAccessGuard)` + `@RequirePermission` stacks work. Session is a cookie (`trek_session`); `@CurrentUser()` reads the user. **Never gate a multipart upload with a guard** — guards run before the parser and the client sees ECONNRESET instead of your 403; check in the handler.
|
||||
- **Logging**: `src/nest/audit/audit-log.logger.ts` (`logInfo`/`logWarn`/`logError`/`logDebug`) prints each line to the console and queues it for `data/logs/trek.log`, which `log-file.ts` appends in batches off the event loop and rotates at 10 MB over five files; `index.ts` flushes what is queued on exit. Never write the log file synchronously from a request path.
|
||||
- **Request correlation** (`src/nest/common/request-correlation.ts`): every HTTP request runs under an id (a well-formed incoming `X-Request-Id`, else a fresh UUID) that the response carries in `X-Request-Id`, and every log line written inside it is tagged `[http <id>]`. The exception filter leaves a 5xx on `res.locals` so the access log writes the request line and the stack as one entry. The other entry points (MCP tool/resource/prompt calls via the registry's `around` seam, WebSocket messages in `TrekWsAdapter`, plugin RPC dispatches in the supervisor, cron ticks in `CronRegistrarService`) each run under their own id through `traceEntry()` (`nest/audit/entry-trace.logger.ts`), which logs one line per call: debug on success, warn or error with the reason on failure. A new entry point goes through `traceEntry()` too.
|
||||
- **Process safety nets** (`src/index.ts`, `src/shutdown.ts`): `unhandledRejection` and `uncaughtException` are logged to trek.log and end in the same orderly shutdown as SIGTERM, with exit code 1; a second fatal error during it exits at once. A shutdown first marks `ReadinessService` (`nest/health/`) as draining, so `/api/health/ready` answers 503 for the whole drain. The HTTP server keeps idle keep-alive connections for `HTTP_KEEP_ALIVE_TIMEOUT_MS` (65 s, above the usual proxy idle timeout) with the headers timeout one second above it (`applyProxyTimeouts` in `bootstrap.ts`).
|
||||
- **`src/middleware/globalMiddleware.ts`** is the only Express middleware left (helmet/CSP, CORS, HSTS, forced-HTTPS, logging, cookies).
|
||||
|
||||
## Tests
|
||||
|
||||
@@ -8,7 +8,15 @@
|
||||
* each side touches. A value that pre-container code needs, or that tests
|
||||
* mutate mid-lifetime, belongs in derive.ts instead.
|
||||
*/
|
||||
import { parseBool } from './parsers';
|
||||
import { parseBool, positiveIntOr } from './parsers';
|
||||
|
||||
/**
|
||||
* Longer than the idle timeout of the common reverse proxies and load
|
||||
* balancers (60 s), so the proxy closes an idle upstream connection before
|
||||
* Node does. The other way round the proxy reuses a socket Node just closed
|
||||
* and answers that request with a 502.
|
||||
*/
|
||||
export const DEFAULT_KEEP_ALIVE_TIMEOUT_MS = 65_000;
|
||||
import type { RawEnv } from './derive';
|
||||
|
||||
/** What the pre-init Express layer and the HTTP server freeze when buildApp() runs. */
|
||||
@@ -20,6 +28,8 @@ export function deriveHttpBoot(raw: RawEnv) {
|
||||
trustProxyRaw: raw.TRUST_PROXY,
|
||||
trustProxy: Number.isFinite(trustProxyHops) ? trustProxyHops : 1,
|
||||
hstsIncludeSubdomains: parseBool(raw.HSTS_INCLUDE_SUBDOMAINS) === true,
|
||||
/** How long the HTTP server keeps an idle keep-alive connection open. */
|
||||
keepAliveTimeoutMs: positiveIntOr(raw.HTTP_KEEP_ALIVE_TIMEOUT_MS, DEFAULT_KEEP_ALIVE_TIMEOUT_MS),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -79,6 +79,9 @@ export const envSchema = z.object({
|
||||
FORCE_HTTPS: boolStr,
|
||||
HSTS_INCLUDE_SUBDOMAINS: boolStr,
|
||||
TRUST_PROXY: integer(0, 2 ** 31, 'must be an integer (number of trusted proxy hops)'),
|
||||
// Capped below Node's 300 s request timeout, which the headers timeout (one
|
||||
// second above this) has to stay under.
|
||||
HTTP_KEEP_ALIVE_TIMEOUT_MS: integer(1_000, 290_000, 'must be a whole number of milliseconds between 1000 and 290000'),
|
||||
ALLOW_INTERNAL_NETWORK: boolStr,
|
||||
ALLOW_LINK_LOCAL_IPS: optionalWith(
|
||||
(v) => parseLinkLocalAllowList(v).invalid.length === 0,
|
||||
|
||||
+16
-2
@@ -79,6 +79,19 @@ export function getHttpServer(): nodeHttp.Server {
|
||||
return boundHttpServer;
|
||||
}
|
||||
|
||||
/**
|
||||
* Keep-alive behind a reverse proxy. Node's default is to drop an idle
|
||||
* keep-alive connection after 5 s, while proxies keep their upstream
|
||||
* connections for about a minute: a proxy that reuses a socket Node has just
|
||||
* closed answers that request with a 502, intermittently and with nothing in
|
||||
* TREK's log. The headers timeout stays above the keep-alive one, so a
|
||||
* connection waiting for its next request is never cut by the wrong timer.
|
||||
*/
|
||||
export function applyProxyTimeouts(server: nodeHttp.Server, keepAliveTimeoutMs: number): void {
|
||||
server.keepAliveTimeout = keepAliveTimeoutMs;
|
||||
server.headersTimeout = keepAliveTimeoutMs + 1_000;
|
||||
}
|
||||
|
||||
export async function buildApp(): Promise<INestApplication> {
|
||||
// rawBody keeps the unparsed request bytes on req.rawBody so a plugin webhook
|
||||
// route can verify a provider's HMAC signature over the exact payload (the
|
||||
@@ -106,12 +119,13 @@ export async function buildApp(): Promise<INestApplication> {
|
||||
// boot succeeds, the gateway logs as registered, every test passes, and no
|
||||
// browser can connect. Callers take the server from getHttpServer() below.
|
||||
boundHttpServer = nodeHttp.createServer(instance);
|
||||
app.useWebSocketAdapter(new TrekWsAdapter(boundHttpServer, orm));
|
||||
// ConfigModule.forRoot's load factories already ran inside NestFactory.create,
|
||||
// so the boot-stable snapshot is resolvable here, BEFORE app.init() — this is
|
||||
// so the boot-stable snapshot is resolvable here, BEFORE app.init(); this is
|
||||
// the one bridge that lets the pre-init Express layer consume the validated
|
||||
// config instead of reading process.env itself.
|
||||
const http = app.get<ConfigType<typeof httpConfig>>(httpConfig.KEY);
|
||||
applyProxyTimeouts(boundHttpServer, http.keepAliveTimeoutMs);
|
||||
app.useWebSocketAdapter(new TrekWsAdapter(boundHttpServer, orm));
|
||||
// Same pre-init bridge: a self-hosted routing engine has to be named in connect-src, or
|
||||
// the browser blocks every request to it without an error the app could report. Both
|
||||
// engines go through the same door — the second one answers the avoidance questions
|
||||
|
||||
+26
-5
@@ -18,7 +18,9 @@ import type { DatabaseLifecycle } from './nest/database/database-lifecycle.servi
|
||||
import { getAppUrl, getMcpSafeUrl, readEnv } from './app-config';
|
||||
import { resolveDataPaths } from './app-config/data-paths';
|
||||
import { resolveDbPath } from './db/db-path';
|
||||
import { flushLogFileSync } from './nest/audit/audit-log.logger';
|
||||
import { flushLogFileSync, logError } from './nest/audit/audit-log.logger';
|
||||
import { ReadinessService } from './nest/health/readiness.service';
|
||||
import { createFatalHandler } from './shutdown';
|
||||
|
||||
// data/tmp is the driver-agnostic global scratch dir (restore-upload spool,
|
||||
// mirror stream staging) and stays boot-created here. Driver-owned roots — the
|
||||
@@ -171,9 +173,13 @@ bootstrap().catch((err) => {
|
||||
// in short, #2193: nothing here could ever release a WebSocket, so `docker
|
||||
// stop` always ended in SIGKILL and exit 137.
|
||||
let shuttingDown = false;
|
||||
function shutdown(signal: string): void {
|
||||
// A second signal — the SIGINT that follows a Ctrl-C, or an impatient
|
||||
// orchestrator sending SIGTERM twice — must not start a second teardown on
|
||||
// Raised by a fatal error even when a signal's shutdown is already running, so
|
||||
// the process still leaves with a failure code the orchestrator can see.
|
||||
let exitCode = 0;
|
||||
function shutdown(signal: string, code = 0): void {
|
||||
exitCode = Math.max(exitCode, code);
|
||||
// A second signal (the SIGINT that follows a Ctrl-C, or an impatient
|
||||
// orchestrator sending SIGTERM twice) must not start a second teardown on
|
||||
// top of the first one.
|
||||
if (shuttingDown) return;
|
||||
shuttingDown = true;
|
||||
@@ -198,7 +204,11 @@ function shutdown(signal: string): void {
|
||||
},
|
||||
logInfo: sLogInfo,
|
||||
logError: sLogError,
|
||||
exit: (code: number) => process.exit(code),
|
||||
// Readiness answers 503 from here on; strict: false because the provider
|
||||
// lives in HealthModule, not in the root module.
|
||||
markDraining: () => nestApp?.get(ReadinessService, { strict: false }).markDraining(),
|
||||
exitCode,
|
||||
exit: (code: number) => process.exit(Math.max(code, exitCode)),
|
||||
}).catch((err: unknown) => {
|
||||
// Fire-and-forget would make this an unhandled rejection, which on Node 22
|
||||
// is a crash — a worse ending than the one we are here to fix.
|
||||
@@ -212,5 +222,16 @@ function shutdown(signal: string): void {
|
||||
// synchronously here: an 'exit' listener cannot wait for a promise.
|
||||
process.on('exit', () => flushLogFileSync());
|
||||
|
||||
// Last-resort handlers. Node would crash on either anyway, printing the stack
|
||||
// to stderr only; this way it lands in trek.log first, and the process goes
|
||||
// through the same orderly shutdown as on SIGTERM before it exits with 1.
|
||||
const onFatal = createFatalHandler({
|
||||
logError,
|
||||
shutdown: (reason, code) => shutdown(reason, code),
|
||||
exit: (code) => process.exit(code),
|
||||
});
|
||||
process.on('unhandledRejection', (reason) => onFatal('Unhandled promise rejection', reason));
|
||||
process.on('uncaughtException', (error) => onFatal('Uncaught exception', error));
|
||||
|
||||
process.on('SIGTERM', () => shutdown('SIGTERM'));
|
||||
process.on('SIGINT', () => shutdown('SIGINT'));
|
||||
|
||||
@@ -5,6 +5,7 @@ import { KitineraryExtractorService } from '../booking-import/kitinerary-extract
|
||||
import { AddonsService } from '../addons/addons.service';
|
||||
import { ADDON_IDS } from '../../addons';
|
||||
import { Public } from '../auth/public.decorator';
|
||||
import { ReadinessService } from './readiness.service';
|
||||
|
||||
/** Exposes the container probe and the server feature flags consumed by the
|
||||
* frontend to show/hide optional UI. */
|
||||
@@ -15,6 +16,7 @@ export class FeaturesController {
|
||||
private readonly extractor: KitineraryExtractorService,
|
||||
private readonly addons: AddonsService,
|
||||
private readonly maintenance: MaintenanceRepository,
|
||||
private readonly readiness: ReadinessService,
|
||||
) {}
|
||||
|
||||
/** The container/uptime probe. The forced-HTTPS redirect and HSTS exempt this
|
||||
@@ -28,13 +30,18 @@ export class FeaturesController {
|
||||
|
||||
/**
|
||||
* The readiness probe: 503 while the database does not answer (a restore
|
||||
* swapping it, a boot still migrating), so an orchestrator stops sending
|
||||
* traffic without killing the process. Liveness stays on the plain probe
|
||||
* above, which a long restore must not fail.
|
||||
* swapping it, a boot still migrating) and once a shutdown has started, so
|
||||
* an orchestrator stops sending traffic without killing the process.
|
||||
* Liveness stays on the plain probe above, which a long restore must not
|
||||
* fail.
|
||||
*/
|
||||
@Get('ready')
|
||||
async ready(@Res() res: Response): Promise<void> {
|
||||
res.setHeader('Cache-Control', 'no-store, must-revalidate');
|
||||
if (this.readiness.isDraining()) {
|
||||
res.status(503).json({ status: 'unavailable' });
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await this.maintenance.ping();
|
||||
res.json({ status: 'ready' });
|
||||
|
||||
@@ -3,6 +3,7 @@ import { FeaturesController } from './features.controller';
|
||||
import { KitineraryExtractorModule } from '../booking-import/kitinerary-extractor.module';
|
||||
import { AddonsModule } from '../addons/addons.module';
|
||||
import { MaintenanceModule } from '../database/maintenance.module';
|
||||
import { ReadinessService } from './readiness.service';
|
||||
|
||||
/** Server capability reporting. `GET /api/health/features` tells the client which
|
||||
* optional server-side features are usable, so it can hide the affordances it
|
||||
@@ -12,5 +13,7 @@ import { MaintenanceModule } from '../database/maintenance.module';
|
||||
@Module({
|
||||
imports: [KitineraryExtractorModule, AddonsModule, MaintenanceModule],
|
||||
controllers: [FeaturesController],
|
||||
providers: [ReadinessService],
|
||||
exports: [ReadinessService],
|
||||
})
|
||||
export class HealthModule {}
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
import { Injectable, type BeforeApplicationShutdown } from '@nestjs/common';
|
||||
|
||||
/**
|
||||
* Whether this process still wants traffic. It stops wanting it the moment a
|
||||
* shutdown starts: index.ts marks it before the drain begins, and Nest's own
|
||||
* shutdown hook marks it too, for a close that does not come through
|
||||
* index.ts. The readiness probe answers 503 from then on, so an orchestrator
|
||||
* moves traffic away during the drain instead of one probe period later.
|
||||
*/
|
||||
@Injectable()
|
||||
export class ReadinessService implements BeforeApplicationShutdown {
|
||||
private draining = false;
|
||||
|
||||
markDraining(): void {
|
||||
this.draining = true;
|
||||
}
|
||||
|
||||
isDraining(): boolean {
|
||||
return this.draining;
|
||||
}
|
||||
|
||||
beforeApplicationShutdown(): void {
|
||||
this.markDraining();
|
||||
}
|
||||
}
|
||||
+49
-3
@@ -32,6 +32,13 @@ export interface ShutdownDeps {
|
||||
logInfo: (message: string) => void;
|
||||
logError: (message: string) => void;
|
||||
exit: (code: number) => void;
|
||||
/**
|
||||
* Flip readiness to "not ready" before anything else, so the probe sends
|
||||
* traffic elsewhere for the whole drain rather than one probe period later.
|
||||
*/
|
||||
markDraining?: () => void;
|
||||
/** The code a completed shutdown exits with: 0 for a signal, 1 after a fatal error. */
|
||||
exitCode?: number;
|
||||
/** How long a socket may take the polite way out before it is destroyed. */
|
||||
drainMs?: number;
|
||||
/** Last-resort exit, deliberately below Docker's 10s stop grace. */
|
||||
@@ -92,8 +99,8 @@ export const FORCED_EXIT_MS = 5_000;
|
||||
export async function runShutdown(signal: string, deps: ShutdownDeps): Promise<void> {
|
||||
const {
|
||||
server, closeNestApp, getWsClients, closeMcpSessions, closeDb,
|
||||
logInfo, logError, exit,
|
||||
drainMs = SOCKET_DRAIN_MS, forcedMs = FORCED_EXIT_MS,
|
||||
logInfo, logError, exit, markDraining,
|
||||
exitCode = 0, drainMs = SOCKET_DRAIN_MS, forcedMs = FORCED_EXIT_MS,
|
||||
} = deps;
|
||||
|
||||
logInfo(`${signal} received — shutting down gracefully...`);
|
||||
@@ -108,6 +115,8 @@ export async function runShutdown(signal: string, deps: ShutdownDeps): Promise<v
|
||||
}
|
||||
};
|
||||
|
||||
if (markDraining) guard('markDraining', markDraining);
|
||||
|
||||
let finished = false;
|
||||
/** The single exit path, so the DB is closed exactly once and never skipped. */
|
||||
const finish = (code: number, message: string): void => {
|
||||
@@ -169,5 +178,42 @@ export async function runShutdown(signal: string, deps: ShutdownDeps): Promise<v
|
||||
}
|
||||
clearTimeout(drain);
|
||||
clearTimeout(forced);
|
||||
finish(0, 'Shutdown complete');
|
||||
finish(exitCode, 'Shutdown complete');
|
||||
}
|
||||
|
||||
/** What the last-resort handlers need, handed in like the shutdown itself. */
|
||||
export interface FatalDeps {
|
||||
logError: (message: string) => void;
|
||||
/** Start the orderly shutdown with this reason and exit code. */
|
||||
shutdown: (reason: string, exitCode: number) => void;
|
||||
exit: (code: number) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* The handler behind `process.on('unhandledRejection')` and
|
||||
* `process.on('uncaughtException')`.
|
||||
*
|
||||
* Node would crash on either anyway, with the stack on stderr only. This
|
||||
* writes it to the app log first, then runs the same orderly shutdown a
|
||||
* SIGTERM gets (sockets released, database closed, bounded by the forced
|
||||
* exit) and leaves with exit code 1, so the orchestrator restarts the
|
||||
* process. A second fatal error while that shutdown runs stops the process
|
||||
* at once: whatever is failing is failing the shutdown too.
|
||||
*/
|
||||
export function createFatalHandler(deps: FatalDeps): (kind: string, error: unknown) => void {
|
||||
let fired = false;
|
||||
return (kind, error) => {
|
||||
const detail = error instanceof Error ? (error.stack ?? error.message) : String(error);
|
||||
try {
|
||||
deps.logError(`${kind}: ${detail}`);
|
||||
} catch {
|
||||
// Logging is best effort here: the exit below must happen regardless.
|
||||
}
|
||||
if (fired) {
|
||||
deps.exit(1);
|
||||
return;
|
||||
}
|
||||
fired = true;
|
||||
deps.shutdown(kind, 1);
|
||||
};
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.
|
||||
|
||||
import { db as testDb } from '../../src/db/database';
|
||||
import { resetTestDb } from '../helpers/test-db';
|
||||
import { buildApp } from '../../src/bootstrap';
|
||||
import { buildApp, getHttpServer } from '../../src/bootstrap';
|
||||
import { httpConfig, RuntimeEnvService } from '../../src/nest/app-config';
|
||||
import type { ConfigType } from '@nestjs/config';
|
||||
|
||||
@@ -46,6 +46,19 @@ describe('AppConfigModule in the real buildApp()', () => {
|
||||
else process.env.HSTS_INCLUDE_SUBDOMAINS = prevHsts;
|
||||
if (prevDemo === undefined) delete process.env.DEMO_MODE;
|
||||
else process.env.DEMO_MODE = prevDemo;
|
||||
delete process.env.HTTP_KEEP_ALIVE_TIMEOUT_MS;
|
||||
});
|
||||
|
||||
it("the HTTP server outlasts a proxy's idle timeout, with the headers timeout above it", async () => {
|
||||
app = await buildApp();
|
||||
expect(getHttpServer().keepAliveTimeout).toBe(65_000);
|
||||
expect(getHttpServer().headersTimeout).toBe(66_000);
|
||||
await app.close();
|
||||
|
||||
process.env.HTTP_KEEP_ALIVE_TIMEOUT_MS = '95000';
|
||||
app = await buildApp();
|
||||
expect(getHttpServer().keepAliveTimeout).toBe(95_000);
|
||||
expect(getHttpServer().headersTimeout).toBe(96_000);
|
||||
});
|
||||
|
||||
it('boot-stable snapshots re-derive per app build (mutate → rebuild → new value)', async () => {
|
||||
|
||||
@@ -76,6 +76,11 @@ describe('deriveHttp', () => {
|
||||
expect(deriveHttpBoot({}).trustProxy).toBe(1);
|
||||
});
|
||||
|
||||
it("HTTP_KEEP_ALIVE_TIMEOUT_MS: 65 s unless set (above the common proxies' 60 s idle timeout)", () => {
|
||||
expect(deriveHttpBoot({}).keepAliveTimeoutMs).toBe(65_000);
|
||||
expect(deriveHttpBoot({ HTTP_KEEP_ALIVE_TIMEOUT_MS: '95000' }).keepAliveTimeoutMs).toBe(95_000);
|
||||
});
|
||||
|
||||
it('boolean switches accept the whole boolean-like family (unified semantics)', () => {
|
||||
expect(deriveHttp({ FORCE_HTTPS: 'TRUE' }).forceHttps).toBe(true);
|
||||
expect(deriveHttp({ FORCE_HTTPS: 'on' }).forceHttps).toBe(true);
|
||||
|
||||
@@ -207,6 +207,8 @@ describe('validateEnvAtBoot', () => {
|
||||
it.each([
|
||||
['PORT', '70000'],
|
||||
['TRUST_PROXY', '1.5'],
|
||||
['HTTP_KEEP_ALIVE_TIMEOUT_MS', '500'],
|
||||
['HTTP_KEEP_ALIVE_TIMEOUT_MS', '300000'],
|
||||
['DEFAULT_LANGUAGE', 'klingon'],
|
||||
['LOG_LEVEL', 'verbose'],
|
||||
['SESSION_DURATION_REMEMBER', '-1d'],
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { FeaturesController } from '../../../src/nest/health/features.controller';
|
||||
import { ReadinessService } from '../../../src/nest/health/readiness.service';
|
||||
import { HealthModule } from '../../../src/nest/health/health.module';
|
||||
import { KitineraryExtractorModule } from '../../../src/nest/booking-import/kitinerary-extractor.module';
|
||||
import { KitineraryExtractorService } from '../../../src/nest/booking-import/kitinerary-extractor.service';
|
||||
@@ -18,7 +19,12 @@ function make(available: boolean, aiEnabled: boolean) {
|
||||
return {
|
||||
extractor,
|
||||
addons,
|
||||
controller: new FeaturesController(extractor as Extractor as KitineraryExtractorService, addons as never, {} as never),
|
||||
controller: new FeaturesController(
|
||||
extractor as Extractor as KitineraryExtractorService,
|
||||
addons as never,
|
||||
{} as never,
|
||||
new ReadinessService(),
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -97,7 +103,12 @@ describe('FeaturesController (GET /api/health/features)', () => {
|
||||
const execute = vi.fn().mockResolvedValue([]);
|
||||
const em = { getConnection: () => ({ execute }), getContext: () => em };
|
||||
const extractor = { isAvailable: vi.fn(() => true) };
|
||||
const controller = new FeaturesController(extractor as Extractor as KitineraryExtractorService, {} as never, new MaintenanceRepository(em as never));
|
||||
const controller = new FeaturesController(
|
||||
extractor as Extractor as KitineraryExtractorService,
|
||||
{} as never,
|
||||
new MaintenanceRepository(em as never),
|
||||
new ReadinessService(),
|
||||
);
|
||||
|
||||
const ok = response();
|
||||
await controller.ready(ok as never);
|
||||
@@ -111,5 +122,26 @@ describe('FeaturesController (GET /api/health/features)', () => {
|
||||
expect(down.status).toHaveBeenCalledWith(503);
|
||||
expect(down.json).toHaveBeenCalledWith({ status: 'unavailable' });
|
||||
});
|
||||
|
||||
it('FEAT-011: answers 503 once a shutdown has started, without asking the database', async () => {
|
||||
const execute = vi.fn().mockResolvedValue([]);
|
||||
const em = { getConnection: () => ({ execute }), getContext: () => em };
|
||||
const readiness = new ReadinessService();
|
||||
const controller = new FeaturesController({} as never, {} as never, new MaintenanceRepository(em as never), readiness);
|
||||
|
||||
readiness.markDraining();
|
||||
const draining = response();
|
||||
await controller.ready(draining as never);
|
||||
expect(draining.status).toHaveBeenCalledWith(503);
|
||||
expect(draining.json).toHaveBeenCalledWith({ status: 'unavailable' });
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('FEAT-012: Nest closing the app marks the process as draining too', () => {
|
||||
const readiness = new ReadinessService();
|
||||
expect(readiness.isDraining()).toBe(false);
|
||||
readiness.beforeApplicationShutdown();
|
||||
expect(readiness.isDraining()).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
* fires inside Docker's 10s grace rather than on top of it.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { runShutdown, SOCKET_DRAIN_MS, FORCED_EXIT_MS } from '../../src/shutdown';
|
||||
import { runShutdown, createFatalHandler, SOCKET_DRAIN_MS, FORCED_EXIT_MS } from '../../src/shutdown';
|
||||
|
||||
type Deps = Parameters<typeof runShutdown>[1];
|
||||
|
||||
@@ -186,3 +186,76 @@ describe('runShutdown', () => {
|
||||
expect(deps.exit).toHaveBeenCalledWith(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('runShutdown: readiness and the exit code', () => {
|
||||
it('SHUTDOWN-020 marks the process as draining before anything else happens', async () => {
|
||||
const order: string[] = [];
|
||||
const deps = makeDeps({
|
||||
markDraining: vi.fn(() => { order.push('draining'); }),
|
||||
closeMcpSessions: vi.fn(() => { order.push('mcp'); }),
|
||||
getWsClients: () => { order.push('ws'); return null; },
|
||||
});
|
||||
const run = runShutdown('SIGTERM', deps);
|
||||
(deps.server as unknown as ReturnType<typeof makeServer>).finishClose();
|
||||
await run;
|
||||
expect(order[0]).toBe('draining');
|
||||
expect(order).toEqual(['draining', 'mcp', 'ws']);
|
||||
});
|
||||
|
||||
it('SHUTDOWN-021 a readiness flag that cannot be set is logged and the shutdown goes on', async () => {
|
||||
const deps = makeDeps({ markDraining: () => { throw new Error('no app yet'); } });
|
||||
const run = runShutdown('SIGTERM', deps);
|
||||
(deps.server as unknown as ReturnType<typeof makeServer>).finishClose();
|
||||
await run;
|
||||
expect(deps.logError).toHaveBeenCalledWith('markDraining failed during shutdown: no app yet');
|
||||
expect(deps.exit).toHaveBeenCalledWith(0);
|
||||
});
|
||||
|
||||
it('SHUTDOWN-022 a shutdown started by a fatal error exits with its code', async () => {
|
||||
const deps = makeDeps({ exitCode: 1 });
|
||||
const run = runShutdown('Uncaught exception', deps);
|
||||
(deps.server as unknown as ReturnType<typeof makeServer>).finishClose();
|
||||
await run;
|
||||
expect(deps.closeDb).toHaveBeenCalledTimes(1);
|
||||
expect(deps.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('createFatalHandler', () => {
|
||||
function fatalDeps() {
|
||||
return { logError: vi.fn(), shutdown: vi.fn(), exit: vi.fn() };
|
||||
}
|
||||
|
||||
it('FATAL-001 logs the stack and starts the orderly shutdown with exit code 1', () => {
|
||||
const deps = fatalDeps();
|
||||
const onFatal = createFatalHandler(deps);
|
||||
const error = new Error('boom');
|
||||
onFatal('Uncaught exception', error);
|
||||
expect(deps.logError).toHaveBeenCalledWith(`Uncaught exception: ${error.stack}`);
|
||||
expect(deps.shutdown).toHaveBeenCalledWith('Uncaught exception', 1);
|
||||
expect(deps.exit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('FATAL-002 a rejection with a non-error reason is logged as text', () => {
|
||||
const deps = fatalDeps();
|
||||
createFatalHandler(deps)('Unhandled promise rejection', 'plain reason');
|
||||
expect(deps.logError).toHaveBeenCalledWith('Unhandled promise rejection: plain reason');
|
||||
});
|
||||
|
||||
it('FATAL-003 a second fatal error during the shutdown exits at once', () => {
|
||||
const deps = fatalDeps();
|
||||
const onFatal = createFatalHandler(deps);
|
||||
onFatal('Uncaught exception', new Error('first'));
|
||||
onFatal('Unhandled promise rejection', new Error('second'));
|
||||
expect(deps.shutdown).toHaveBeenCalledTimes(1);
|
||||
expect(deps.exit).toHaveBeenCalledWith(1);
|
||||
expect(deps.logError).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('FATAL-004 a logger that throws does not stop the shutdown', () => {
|
||||
const deps = fatalDeps();
|
||||
deps.logError.mockImplementation(() => { throw new Error('disk gone'); });
|
||||
createFatalHandler(deps)('Uncaught exception', new Error('boom'));
|
||||
expect(deps.shutdown).toHaveBeenCalledWith('Uncaught exception', 1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -170,6 +170,7 @@ full explanation.
|
||||
| `FORCE_HTTPS` | When `true`: 301-redirects HTTP→HTTPS, sends HSTS (`max-age=31536000`), adds CSP `upgrade-insecure-requests`, forces cookie `secure` flag. Only useful behind a TLS proxy. Your proxy must send `X-Forwarded-Proto: https`. | `false` |
|
||||
| `HSTS_INCLUDE_SUBDOMAINS` | When `true`: adds the `includeSubDomains` directive to the HSTS header, extending HTTPS enforcement to all subdomains. Only effective when HSTS is active (`FORCE_HTTPS=true` or `NODE_ENV=production`). Leave `false` if you run other services on sibling subdomains over plain HTTP. | `false` |
|
||||
| `TRUST_PROXY` | Number of trusted proxy hops. Tells Express how far into `X-Forwarded-For` to look for the real client IP, and where to read `X-Forwarded-Proto`. Count your hops: with two proxies in front of TREK and `TRUST_PROXY=1`, the IP on every audit row is the inner proxy's. `0` trusts nothing and always uses the socket address. Not required for the `FORCE_HTTPS` redirect, which reads the `X-Forwarded-Proto` header directly — set it for correct client IPs in the audit log, and for the `COOKIE_SECURE` auto-derivation, which goes through `req.secure`. | `1` (production) |
|
||||
| `HTTP_KEEP_ALIVE_TIMEOUT_MS` | How long (in milliseconds) the server keeps an idle keep-alive connection open; the headers timeout follows one second above it. Keep it above your reverse proxy's upstream idle timeout (60 s for nginx and most load balancers, 90 s for Traefik), otherwise the proxy reuses a connection the server just closed and answers that request with a 502. Between `1000` and `290000`. | `65000` |
|
||||
| `COOKIE_SECURE` | Controls the `secure` flag on the `trek_session` cookie. Auto-derived as `true` when `NODE_ENV=production`, when `FORCE_HTTPS=true`, or when the request itself arrived over TLS on the outermost hop (`X-Forwarded-Proto: https` with `TRUST_PROXY` set). Set to `false` only as an escape hatch for LAN testing without TLS — not recommended in production. | auto |
|
||||
|
||||
> **Warning:** `FORCE_HTTPS=true` behind a proxy that does not forward `X-Forwarded-Proto: https` causes a
|
||||
|
||||
@@ -202,7 +202,7 @@ Startup, liveness and readiness probes are configurable under `probes`:
|
||||
|
||||
- **startup** hits `/api/health` and gives a first boot, a long migration or a restore at boot up to ten minutes (`failureThreshold: 60` × `periodSeconds: 10`) before liveness takes over.
|
||||
- **liveness** hits `/api/health`, which only says the process answers. A long restore does not fail it.
|
||||
- **readiness** hits `/api/health/ready`, which also asks the database. While the database does not answer, the pod stops getting traffic without being restarted.
|
||||
- **readiness** hits `/api/health/ready`, which also asks the database. While the database does not answer, the pod stops getting traffic without being restarted. It also answers 503 as soon as the pod starts shutting down, so traffic moves away for the whole drain.
|
||||
|
||||
```bash
|
||||
helm install trek trek/trek --set probes.startup.failureThreshold=120 --set probes.readiness.periodSeconds=20
|
||||
|
||||
Reference in New Issue
Block a user