diff --git a/server/.env.example b/server/.env.example index 7d166d0..86480c1 100644 --- a/server/.env.example +++ b/server/.env.example @@ -21,6 +21,17 @@ DATABASE_POOL_IDLE_TIMEOUT_MS=300000 DATABASE_POOL_CONNECTION_TIMEOUT_MS=15000 DATABASE_QUERY_TIMEOUT_MS=1000000 DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS=300000 +# Recycle pooled clients after N seconds to dodge stale connections (NAT +# timeouts, server-side resource creep). 0 = never expire. +# DATABASE_POOL_MAX_LIFETIME_SECONDS=0 +# Optional server-side query timeout (ms). Defense in depth alongside +# DATABASE_QUERY_TIMEOUT_MS, which is client-side. Unset => Postgres default +# (no limit). +# DATABASE_STATEMENT_TIMEOUT_MS=1000000 +# TCP keepalive on idle sockets. On by default; set to `false` to disable. +# DATABASE_KEEPALIVE=true +# Delay (ms) before sending the first keepalive probe. Unset => OS default. +# DATABASE_KEEPALIVE_INITIAL_DELAY_MS=0 # Log every Kysely query (SQL, params, duration). Errors always log regardless. DATABASE_PRINT_LOGS=true diff --git a/server/api.ts b/server/api.ts index 7d3e66f..fba4980 100644 --- a/server/api.ts +++ b/server/api.ts @@ -1,4 +1,3 @@ -/* eslint-disable max-lines */ import os from 'node:os'; import path from 'path'; import { ApolloServer } from '@apollo/server'; @@ -33,7 +32,7 @@ import { authSchemaWrapper } from './graphql/utils/authorization.js'; import { buildPassportContext } from './graphql/utils/passportContext.js'; import { safeDepthLimit } from './graphql/utils/safeDepthLimit.js'; import { type Dependencies } from './iocContainer/index.js'; -import { isEnvTrue, safeGetEnvInt } from './iocContainer/utils.js'; +import { safeGetEnvInt } from './iocContainer/utils.js'; import controllers from './routes/index.js'; import { createBodySchemaValidator } from './utils/bodySchemaValidation.js'; import { jsonStringify } from './utils/encoding.js'; @@ -91,7 +90,7 @@ const sessionStore = connectPgSimple(session); export default async function makeApiServer(deps: Dependencies) { const app = express(); - const { KyselyPg } = deps; + const { KyselyPg, KyselyPgPool } = deps; app.use(cors()); @@ -127,29 +126,10 @@ export default async function makeApiServer(deps: Dependencies) { /** * Passport & User Session Configuration */ - const { - DATABASE_HOST, - DATABASE_PORT = 5432, - DATABASE_NAME, - DATABASE_USER, - DATABASE_PASSWORD, - } = process.env; - - const conObject = { - host: DATABASE_HOST, - port: Number(DATABASE_PORT), - user: DATABASE_USER, - password: DATABASE_PASSWORD, - database: DATABASE_NAME, - // NB: `rejectUnauthorized: false` keeps the connection encrypted but skips - // certificate validation. - ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, - }; - app.use( session({ secret: process.env.SESSION_SECRET!, - store: new sessionStore({ conObject }), + store: new sessionStore({ pool: KyselyPgPool }), cookie: { secure: process.env.NODE_ENV === 'production', httpOnly: true, diff --git a/server/iocContainer/createPgPool.test.ts b/server/iocContainer/createPgPool.test.ts new file mode 100644 index 0000000..9d507ed --- /dev/null +++ b/server/iocContainer/createPgPool.test.ts @@ -0,0 +1,51 @@ +import { createPgPool } from './createPgPool.js'; + +describe('createPgPool', () => { + test('attaches an idle-client `error` listener so pg errors do not crash the process', async () => { + const pool = createPgPool({ host: '127.0.0.1', port: 1 }); + try { + expect(pool.listenerCount('error')).toBeGreaterThan(0); + } finally { + await pool.end(); + } + }); + + test('logs and swallows idle-client errors instead of re-throwing', async () => { + const pool = createPgPool({ host: '127.0.0.1', port: 1 }); + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + + try { + const simulatedError = new Error( + 'connection terminated unexpectedly (simulated)', + ); + + expect(() => pool.emit('error', simulatedError)).not.toThrow(); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + + const loggedPayload = consoleErrorSpy.mock.calls[0]?.[0] as string; + expect(typeof loggedPayload).toBe('string'); + expect(loggedPayload).toContain('Postgres pool idle-client error'); + expect(loggedPayload).toContain('connection terminated unexpectedly'); + } finally { + consoleErrorSpy.mockRestore(); + await pool.end(); + } + }); + + test('forwards caller config to pg.Pool without mutating it', async () => { + const config = { host: '127.0.0.1', port: 1, keepAlive: false }; + const snapshot = structuredClone(config); + const pool = createPgPool(config); + try { + expect(config).toEqual(snapshot); + expect( + (pool as unknown as { options: { keepAlive?: boolean } }).options + .keepAlive, + ).toBe(false); + } finally { + await pool.end(); + } + }); +}); diff --git a/server/iocContainer/createPgPool.ts b/server/iocContainer/createPgPool.ts new file mode 100644 index 0000000..b3d08c8 --- /dev/null +++ b/server/iocContainer/createPgPool.ts @@ -0,0 +1,26 @@ +import pg from 'pg'; + +import { logErrorJson } from '../utils/logging.js'; + +/** + * Wraps `new pg.Pool` with an idle-client `'error'` listener. + * + * Without a listener, `pg` escalates idle-client errors (Postgres restart, + * dropped LB connection, etc.) to `uncaughtException`, which kills the + * process. Logging the error lets the pool replace the dead client on the + * next checkout, which is the recovery path `pg` already supports. + */ +export function createPgPool(config: pg.PoolConfig): pg.Pool { + const pool = new pg.Pool(config); + + // Log-only; do NOT re-throw. + pool.on('error', (err) => { + // eslint-disable-next-line no-restricted-syntax -- boot-time logger; runs before tracer init + logErrorJson({ + message: 'Postgres pool idle-client error (pool will reconnect lazily)', + error: err, + }); + }); + + return pool; +} diff --git a/server/iocContainer/index.ts b/server/iocContainer/index.ts index 3425c3e..8f7099a 100644 --- a/server/iocContainer/index.ts +++ b/server/iocContainer/index.ts @@ -11,7 +11,7 @@ import IORedis, { type Cluster } from 'ioredis'; import { Kysely, PostgresDialect } from 'kysely'; import _ from 'lodash'; import { DynamicPool } from 'node-worker-threads-pool'; -import pg from 'pg'; +import type pg from 'pg'; import Cursor from 'pg-cursor'; import { type JsonObject, type ReadonlyDeep } from 'type-fest'; import { v1 as uuidv1 } from 'uuid'; @@ -240,9 +240,15 @@ import { type NonEmptyArray, type Satisfies, } from '../utils/typescript-types.js'; +import { createPgPool } from './createPgPool.js'; import { registerGqlDataSources } from './services/gqlDataSources.js'; import { registerWorkersAndJobs } from './services/workersAndJobs.js'; -import { isEnvTrue, register, safeGetEnvVar } from './utils.js'; +import { + isEnvTrue, + register, + safeGetEnvNonNegativeInt, + safeGetEnvVar, +} from './utils.js'; // the otel instrumentation currently intercepts require statements. support for // esm support is experimental so we should wait until it is stable @@ -304,6 +310,11 @@ export interface Dependencies { // eslint-disable-next-line @typescript-eslint/no-explicit-any KyselyPgReadReplica: Kysely; + // The shared master `pg.Pool` that `KyselyPg` is built on. Exposed so + // non-Kysely Postgres consumers (e.g., the express-session store) can + // share the same pool instead of opening their own. + KyselyPgPool: pg.Pool; + // Similar to our Kysely services, we register the services as Scylla so // that each dependent service can type its arg more specifically with the set // of tables it is responsible for / allowed to query. @@ -432,22 +443,59 @@ export type PublicInterface = { [K in keyof T]: T[K] }; export default async function getBottle() { // Pool / client tuning shared by both Kysely pools. Defaults preserve our // pre-Kysely behavior; env var names are generic. - const getPgPoolTuning = () => ({ - // pg's default is 10s, which churns connections during quiet periods. - idleTimeoutMillis: parseInt( - process.env.DATABASE_POOL_IDLE_TIMEOUT_MS ?? '300000', - ), - // pg's default is 0 (wait forever); fail fast if the db is unreachable. - connectionTimeoutMillis: parseInt( - process.env.DATABASE_POOL_CONNECTION_TIMEOUT_MS ?? '15000', - ), - // Bound long-running queries instead of letting them hold a pool slot. - query_timeout: parseInt(process.env.DATABASE_QUERY_TIMEOUT_MS ?? '1000000'), - // Kill sessions sitting idle inside an open transaction (holding locks). - idle_in_transaction_session_timeout: parseInt( - process.env.DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS ?? '300000', - ), - }); + const getPgPoolTuning = () => { + const statementTimeoutMs = + process.env.DATABASE_STATEMENT_TIMEOUT_MS?.trim(); + const keepAliveInitialDelayMs = + process.env.DATABASE_KEEPALIVE_INITIAL_DELAY_MS?.trim(); + return { + // pg's default is 10s, which churns connections during quiet periods. + idleTimeoutMillis: safeGetEnvNonNegativeInt( + 'DATABASE_POOL_IDLE_TIMEOUT_MS', + 300000, + ), + // pg's default is 0 (wait forever); fail fast if the db is unreachable. + connectionTimeoutMillis: safeGetEnvNonNegativeInt( + 'DATABASE_POOL_CONNECTION_TIMEOUT_MS', + 15000, + ), + // Client-side bound on long-running queries. + query_timeout: safeGetEnvNonNegativeInt( + 'DATABASE_QUERY_TIMEOUT_MS', + 1000000, + ), + // Optional server-side bound; defense in depth alongside `query_timeout`. + // Unset => Postgres' own default (no limit). + ...(statementTimeoutMs && { + statement_timeout: safeGetEnvNonNegativeInt( + 'DATABASE_STATEMENT_TIMEOUT_MS', + 0, + ), + }), + // Kill sessions sitting idle inside an open transaction (holding locks). + idle_in_transaction_session_timeout: safeGetEnvNonNegativeInt( + 'DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS', + 300000, + ), + // Recycle each client after N seconds to dodge stale connections. + // 0 = never expire (default). + maxLifetimeSeconds: safeGetEnvNonNegativeInt( + 'DATABASE_POOL_MAX_LIFETIME_SECONDS', + 0, + ), + // TCP keepalive surfaces NAT/LB connection drops as pool errors + // (handled by `createPgPool`) rather than as hung queries. Defaults on; + // set DATABASE_KEEPALIVE=false to disable. + keepAlive: + process.env.DATABASE_KEEPALIVE?.trim().toLowerCase() !== 'false', + ...(keepAliveInitialDelayMs && { + keepAliveInitialDelayMillis: safeGetEnvNonNegativeInt( + 'DATABASE_KEEPALIVE_INITIAL_DELAY_MS', + 0, + ), + }), + }; + }; // NB: this is a function because safeGetEnvVar can throw, so we only want to // try to look up the env vars (and throw if they're missing) _if someone @@ -483,18 +531,26 @@ export default async function getBottle() { // Pg services. // + // - 'KyselyPgPool' is the shared `pg.Pool` for the primary (writable) db. + // Non-Kysely Postgres consumers (e.g. the express-session store in + // `api.ts`) inject this so every connection passes through `createPgPool`. + // // - 'KyselyPg' is for issuing raw pg queries w/o sequelize (e.g., the queries // that some of the our "services" issue to pg, to the non-public schemas). // These queries go to our primary db, which accepts writes. // // - KyselyPgReadReplica gives us the same type safety, but sends queries to our // replicas, for when we only need reads and we're ok w/ eventual consistency. + bottle.factory('KyselyPgPool', () => + createPgPool(getPgMasterConnectionInfo()), + ); + bottle.factory( 'KyselyPg', - () => + (container) => new Kysely({ dialect: new PostgresDialect({ - pool: new pg.Pool(getPgMasterConnectionInfo()), + pool: container.KyselyPgPool, cursor: Cursor, }), log: kyselyLogLevels, @@ -506,7 +562,7 @@ export default async function getBottle() { () => new Kysely({ dialect: new PostgresDialect({ - pool: new pg.Pool({ + pool: createPgPool({ ...getPgMasterConnectionInfo(), max: parseInt(process.env.DATABASE_READ_POOL_MAX ?? '150'), host: safeGetEnvVar('DATABASE_READ_ONLY_HOST'),