diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index dd7c6079..2549c85f 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -19,7 +19,7 @@ pnpm run test:cov pnpm run test:ci # Debug open handles -pnpm run test:e2e-doh +npx vitest run --config vitest-e2e.config.ts --reporter=hanging-process # Clean up leftover test artifacts (.txt, .bin files from failed file upload tests) pnpm run test:cleanup @@ -27,8 +27,9 @@ pnpm run test:cleanup ## Test Environment -- Environment: `NODE_ENV=local` -- Database: Local MongoDB instance (`mongodb://127.0.0.1/nest-server-local`) +- Environment: `NODE_ENV=e2e` (via `pnpm test` → `vitest-e2e.config.ts`) +- Database: **one unique database per run** (`nest-server-e2e-run--p`), created by `tests/global-setup.ts` so concurrent runs cannot interfere with each other +- DB lifecycle (`tests/db-lifecycle.reporter.ts`): run passes → DB dropped immediately + stale run DBs from crashed/failed runs collected; run fails → DB kept for debugging, removed by the next successful run. An externally set `MONGODB_URI` (CI) opts out of the scheme. - Test helper: `src/test/test.helper.ts` - Coverage: Collected from `src/**/*.{ts,js}` @@ -115,6 +116,6 @@ Full documentation for TestHelper (REST, GraphQL, Cookie support): ## Common Test Issues - **Tests timeout**: Ensure MongoDB is running -- **Open handles**: Use `pnpm run test:e2e-doh` to debug +- **Open handles**: Run vitest with `--reporter=hanging-process` to debug - **Data conflicts**: Use unique identifiers per test - **"NestApplication successfully started" log**: Use `httpServer.listen()` instead of `app.listen()` diff --git a/CLAUDE.md b/CLAUDE.md index 7789c848..0c285b49 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -108,7 +108,7 @@ pnpm run start:dev # Development mode with watch # Testing (ALWAYS run before completing changes) pnpm test # Run E2E tests (Vitest) pnpm run test:cov # With coverage -pnpm run test:e2e-doh # Debug open handles +npx vitest run --config vitest-e2e.config.ts --reporter=hanging-process # Debug open handles pnpm run test:cleanup # Remove leftover test artifacts (.txt, .bin) # Linting & Formatting @@ -218,7 +218,7 @@ The migration store uses `NSC__MONGOOSE__URI` env var (not `config.env.ts`) for | Tests timeout | Ensure MongoDB running on localhost:27017 | | GraphQL introspection fails | Check `config.env.ts` introspection setting | | Module not found after adding | Verify export in `src/index.ts`, run `pnpm run build` | -| Open handles in tests | Run `pnpm run test:e2e-doh` | +| Open handles in tests | Run vitest with `--reporter=hanging-process` | ## Best Practices diff --git a/FRAMEWORK-API.md b/FRAMEWORK-API.md index a8a5288f..41234078 100644 --- a/FRAMEWORK-API.md +++ b/FRAMEWORK-API.md @@ -1,6 +1,6 @@ # @lenne.tech/nest-server — Framework API Reference -> Auto-generated from source code on 2026-06-22 (v11.27.2) +> Auto-generated from source code on 2026-07-03 (v11.27.3) > File: `FRAMEWORK-API.md` — compact, machine-readable API surface for Claude Code ## CoreModule.forRoot() diff --git a/migration-guides/11.27.2-to-11.27.3.md b/migration-guides/11.27.2-to-11.27.3.md new file mode 100644 index 00000000..ae38b002 --- /dev/null +++ b/migration-guides/11.27.2-to-11.27.3.md @@ -0,0 +1,177 @@ +# Migration Guide: 11.27.2 → 11.27.3 + +## Overview + +| Category | Details | +|----------|---------| +| **Breaking Changes** | None | +| **New Features** | None (one new internal helper export: `sendAuthEmailSafely`) | +| **Bugfixes** | A failed verification-email send (e.g. SMTP not configured, delivery failure) no longer crashes the Node process via an unhandled promise rejection. The send stays fire-and-forget (timing-attack mitigation) but every failure — sync throw or async rejection — is now caught and logged instead. | +| **Migration Effort** | 0 minutes (automatic) — `pnpm update` is enough. | + +This is a **stability bugfix release**. No source-code or config changes are +required in consuming projects. + +--- + +## Quick Migration + +No code changes required. + +```bash +# Update package +pnpm add @lenne.tech/nest-server@11.27.3 + +# Verify build +pnpm run build + +# Run tests +pnpm test +``` + +--- + +## What's Fixed in 11.27.3 + +### Process crash on failed verification-email send + +**The bug:** BetterAuth's `emailVerification.sendVerificationEmail` callback +invoked the email send fire-and-forget (intentionally not awaited, per +Better-Auth's recommendation, so response timing cannot leak whether an +account exists). But the detached promise had **no rejection handler**. When +the send failed — SMTP not configured, provider outage, delivery rejection — +the rejection became an *unhandled promise rejection*, which terminates the +Node process under Node's default `--unhandled-rejections=throw` (Node ≥ 15). + +**Observed trigger:** signing up an unverified user and then signing in +(`sendOnSignUp` / `sendOnSignIn` are enabled by default) while no working +SMTP transport was configured took the whole API down. + +**The fix:** sends are now routed through a small hardened wrapper: + +```typescript +// src/core/modules/better-auth/better-auth.config.ts +export function sendAuthEmailSafely(send: () => unknown, onError: (error: unknown) => void): void { + void Promise.resolve() + .then(send) + .catch((error) => { + try { + onError(error); + } catch { + // A throwing error handler must not crash the process either. + } + }); +} +``` + +Behavior after the fix: + +- The send remains **non-blocking** — response timing is unchanged + (timing-attack mitigation preserved). +- Sync throws and async rejections are both caught and logged via the NestJS + logger (context `BetterAuthConfig`), including the **stack trace**: + `Failed to send verification email: `. +- Even a throwing error handler (e.g. a custom logger transport failing) + cannot re-introduce the crash. +- The API response to the client is unchanged (it never reflected send + failures — this matches Better-Auth's own default behavior). + +**Covered by regression tests:** `tests/unit/better-auth-email-safe.spec.ts` +(async rejection, sync throw, success path, non-blocking guarantee, no +unhandled rejection, throwing error handler). + +--- + +## Breaking Changes + +None. + +--- + +## Compatibility Notes + +- **`IServerOptions` / `CoreModule.forRoot()`:** unchanged. +- **`SendVerificationEmailCallback` contract:** unchanged (`{ token, url, user }` + → `Promise`). Projects supplying their own callback need no changes — + their callback is now additionally protected by the wrapper. +- **Monitoring / operations:** if you relied on the process crash (supervisor + restart loops) as an implicit signal for a broken mail setup, switch your + alerting to the error log line `Failed to send verification email: …` + (logger context `BetterAuthConfig`). Note that the email-verification + service additionally logs delivery failures with a masked recipient address + before rethrowing — a real outage produces both lines. +- **Workarounds can be removed:** a custom `process.on('unhandledRejection')` + handler added to a project's `main.ts` solely to survive this crash is no + longer needed for this path. +- **Better-Auth standard mechanism:** the framework deliberately detaches only + the sends it owns instead of enabling Better-Auth's global + `advanced.backgroundTasks` option (which would detach ALL background tasks — + OTP, invites, password reset — and route failures to Better-Auth's internal + logger instead of the NestJS logger). Projects that want the native + mechanism can still enable it via the `betterAuth.options.advanced` + passthrough. +- **Vendor-mode consumers:** the fix lives in + `src/core/modules/better-auth/better-auth.config.ts` and is picked up by the + next core sync (`/lt-dev:backend:update-nest-server-core`). The regression + test file (`tests/unit/`) is framework-repo-only and is not part of the + vendored file set. +- **Custom controllers / resolvers extending Core\* classes:** no impact — no + Core method signatures changed. + +--- + +## Troubleshooting + +### Verification emails silently don't arrive after updating + +Nothing regressed — before this fix the same situation crashed the API +instead. Check the logs for `Failed to send verification email: …` +(context `BetterAuthConfig`) and fix the underlying SMTP/Brevo configuration +(`email.smtp` / provider settings in `config.env.ts`). + +### I see two error log lines for one failed send + +Expected. The email-verification service logs the delivery failure with a +masked recipient address and rethrows; the wrapper logs the same failure once +more (with stack trace) as the final safety net. The wrapper line additionally +covers failures thrown outside the service's own try/catch. + +--- + +## Repo-Internal Tooling Changes (no consumer action) + +These affect only this repository's development workflow — listed here because +some projects copied `scripts/check.mjs` from 11.27.1+: + +- **`scripts/check.mjs` idle watchdog:** a step whose child produces no output + for 300s (configurable via `--idle-timeout=` / `CHECK_IDLE_TIMEOUT`, + `0` disables) is now killed — including its whole process tree — and the + check fails with a clear `[watchdog]` reason. Previously a deadlocked test + run (workers idle at 0% CPU) spun the live view forever. If you copied + `check.mjs` into your project, update your copy to get the watchdog. +- **Per-run test databases:** `tests/global-setup.ts` now gives every vitest + run a unique database (`-run--p`) instead of dropping + a shared fixed-name DB — concurrent runs (second terminal, IDE runner) can + no longer wipe each other's users/sessions mid-flight. A new reporter + (`tests/db-lifecycle.reporter.ts`) drops the DB after a successful run and + collects stale leftovers; after a failed run the DB is kept for debugging + until the next successful run. An externally set `MONGODB_URI` (CI) keeps + the previous behavior. + +--- + +## Module Documentation + +### BetterAuth + +- **README:** [src/core/modules/better-auth/README.md](../src/core/modules/better-auth/README.md) +- **Integration Checklist:** [src/core/modules/better-auth/INTEGRATION-CHECKLIST.md](../src/core/modules/better-auth/INTEGRATION-CHECKLIST.md) +- **Key File:** `src/core/modules/better-auth/better-auth.config.ts` — + `sendAuthEmailSafely()` helper + `buildEmailVerificationConfig()` wiring + +--- + +## References + +- [Migration Guide 11.27.1 → 11.27.2](./11.27.1-to-11.27.2.md) — Previous release (chain-faithful audit step + security overrides) +- [nest-server-starter](https://github.com/lenneTech/nest-server-starter) — reference implementation diff --git a/package.json b/package.json index 43143eb3..7baf910a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@lenne.tech/nest-server", - "version": "11.27.2", + "version": "11.27.3", "description": "Modern, fast, powerful Node.js web framework in TypeScript based on Nest with a GraphQL API and a connection to MongoDB (or other databases).", "keywords": [ "node", diff --git a/scripts/check.mjs b/scripts/check.mjs index ffc9bdb6..4a0aa016 100644 --- a/scripts/check.mjs +++ b/scripts/check.mjs @@ -25,7 +25,7 @@ * Exit code: 0 when every step passed, 1 otherwise (preserves the contract the * lt-dev `running-check-script` skill relies on: non-zero === failed). */ -import { spawn } from "node:child_process"; +import { execSync, spawn } from "node:child_process"; import { readdirSync, readFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -37,6 +37,17 @@ const NO_FIX = process.argv.includes("--no-fix"); const PROJECT_FILTERS = process.argv .filter((a) => a.startsWith("--project=")) .map((a) => a.slice("--project=".length)); +// Watchdog: kill a step whose child produces NO output for this long. A wedged +// test run (workers deadlocked at 0% CPU) otherwise spins the live view +// forever — the spinner only proves the child process exists, not that it +// progresses. Override with --idle-timeout= or CHECK_IDLE_TIMEOUT +// (seconds); 0 disables the watchdog. +const IDLE_TIMEOUT_MS = (() => { + const flag = process.argv.find((a) => a.startsWith("--idle-timeout=")); + const raw = flag ? flag.slice("--idle-timeout=".length) : process.env.CHECK_IDLE_TIMEOUT; + const seconds = raw === undefined || raw === "" ? 300 : Number(raw); + return Number.isFinite(seconds) && seconds > 0 ? seconds * 1000 : 0; +})(); // Verbose streams raw output, so the in-place live view is disabled there. const TTY = Boolean(process.stdout.isTTY) && !VERBOSE; @@ -144,19 +155,72 @@ async function runAudit(auditCmd) { // ── command runner ───────────────────────────────────────────────────────── const RUNNING = new Set(); + +// Best-effort kill of a child's whole process tree (sh → pnpm → vitest → +// fork workers). Killing only the direct child orphans the tree — exactly the +// zombie workers a deadlock leaves behind. Children are collected via pgrep +// and killed leaves-first. +function killTree(child, signal = "SIGTERM") { + const pids = []; + const collect = (pid) => { + pids.push(pid); + let out = ""; + try { + out = execSync(`pgrep -P ${pid}`, { stdio: ["ignore", "pipe", "ignore"] }) + .toString() + .trim(); + } catch { + /* no children */ + } + if (out) for (const p of out.split("\n")) collect(Number(p)); + }; + collect(child.pid); + for (const pid of pids.reverse()) { + try { + process.kill(pid, signal); + } catch { + /* already gone */ + } + } +} + function capture(cmd, cwd) { return new Promise((resolve) => { const child = spawn(cmd, { cwd, shell: true }); RUNNING.add(child); let out = ""; + let idleTimer = null; + let watchdogHit = false; + // Any output resets the watchdog — only complete silence for the full + // window counts as wedged. Escalate to SIGKILL for processes that ignore + // SIGTERM (deadlocked event loops usually still honor TERM, but be sure). + const armWatchdog = () => { + if (!IDLE_TIMEOUT_MS) return; + clearTimeout(idleTimer); + idleTimer = setTimeout(() => { + watchdogHit = true; + killTree(child); + setTimeout(() => killTree(child, "SIGKILL"), 5000).unref(); + }, IDLE_TIMEOUT_MS); + }; const onData = (d) => { out += d; + armWatchdog(); if (VERBOSE) process.stdout.write(d); }; + armWatchdog(); child.stdout.on("data", onData); child.stderr.on("data", onData); const done = (code, extra) => { + clearTimeout(idleTimer); RUNNING.delete(child); + if (watchdogHit) { + const note = + `[watchdog] step produced no output for ${Math.round(IDLE_TIMEOUT_MS / 1000)}s — ` + + "process tree killed as deadlocked. This is a hang (workers idle at 0% CPU), " + + "not a slow run. Re-run the step directly to debug, e.g. `pnpm run vitest`."; + return resolve({ code: 1, out: `${out}\n${note}` }); + } resolve({ code, out: extra ? `${out}\n${extra}` : out }); }; child.on("close", (code) => done(code ?? 1)); @@ -166,7 +230,7 @@ function capture(cmd, cwd) { function killAll() { for (const child of RUNNING) { try { - child.kill("SIGTERM"); + killTree(child); } catch { /* already gone */ } @@ -361,7 +425,9 @@ async function main() { console.log( C.dim( `${projects.length} project(s) · ${stepCount} steps · ${mode} · audit: ${auditCmd ?? "none"}` + - `${NO_FIX ? "" : " · auto-fix format+lint"}${VERBOSE ? " · verbose" : ""}\n`, + `${NO_FIX ? "" : " · auto-fix format+lint"}` + + ` · watchdog: ${IDLE_TIMEOUT_MS ? fmtDuration(IDLE_TIMEOUT_MS) : "off"}` + + `${VERBOSE ? " · verbose" : ""}\n`, ), ); diff --git a/spectaql.yml b/spectaql.yml index bcd94b69..0bf44a57 100644 --- a/spectaql.yml +++ b/spectaql.yml @@ -19,7 +19,7 @@ servers: info: title: lT Nest Server description: Modern, fast, powerful Node.js web framework in TypeScript based on Nest with a GraphQL API and a connection to MongoDB (or other databases). - version: 11.27.2 + version: 11.27.3 contact: name: lenne.Tech GmbH url: https://lenne.tech diff --git a/src/core/modules/better-auth/better-auth.config.ts b/src/core/modules/better-auth/better-auth.config.ts index a934cb44..d90f84c9 100644 --- a/src/core/modules/better-auth/better-auth.config.ts +++ b/src/core/modules/better-auth/better-auth.config.ts @@ -15,6 +15,11 @@ import { detectCookiePrefixDrift, resolveBetterAuthCookiePrefix } from './better */ export type BetterAuthInstance = ReturnType; +/** + * Shared logger for all functions in this config module + */ +const logger = new Logger('BetterAuthConfig'); + // --------------------------------------------------------------------------- // Performance-optimized password hashing using Node.js native crypto.scrypt // @@ -174,6 +179,40 @@ export type SendVerificationEmailCallback = (options: { user: { email: string; id: string; name?: null | string }; }) => Promise; +/** + * Invoke an auth-email send (verification / password-reset) fire-and-forget. + * + * Better-Auth recommends NOT awaiting these sends so the response time does not + * leak whether an account exists (timing attack). The catch here is essential: + * a rejected send (e.g. SMTP not configured / delivery failure) must never + * become an unhandled promise rejection — that crashes the Node process (an + * unverified sign-up + sign-in took the whole API down in dev). This wrapper + * keeps the send non-blocking while routing every failure (sync throw or async + * rejection) to `onError` instead. `onError` itself is guarded too: if the + * handler throws, the error is swallowed rather than crashing the process. + * + * Deliberate divergence from Better-Auth's own mechanism: Better-Auth awaits + * these callbacks via `runInBackgroundOrAwait` and offers + * `advanced.backgroundTasks` as its native non-blocking path. That option is + * global (it detaches ALL background tasks — OTP, invites, password reset, …) + * and routes failures to Better-Auth's internal logger instead of the NestJS + * logger, so this wrapper detaches only the sends the framework owns. Projects + * can still opt into `advanced.backgroundTasks` via the `options` passthrough. + */ +export function sendAuthEmailSafely(send: () => unknown, onError: (error: unknown) => void): void { + void Promise.resolve() + .then(send) + .catch((error) => { + try { + onError(error); + } catch { + // Last resort: the error handler itself failed. Swallowing here keeps + // the no-unhandled-rejection guarantee — a throwing handler must not + // crash the process this helper exists to protect. + } + }); +} + /** * Better-Auth field type definition * Matches the DBFieldType from better-auth @@ -264,7 +303,6 @@ export interface CreateBetterAuthResult { } export function createBetterAuthInstance(options: CreateBetterAuthOptions): CreateBetterAuthResult | null { - const logger = new Logger('BetterAuthConfig'); const { config, db, fallbackSecrets, onEmailVerified, sendVerificationEmail, serverEnv } = options; // Return null only if better-auth is explicitly disabled @@ -495,9 +533,19 @@ function buildEmailVerificationConfig( data: { token: string; url: string; user: { email: string; id: string; name?: null | string } }, _request?: Request, ) => { - // Don't await to prevent timing attacks (as recommended by Better-Auth docs) - - sendVerificationEmail(data); + // Fire-and-forget (timing-attack mitigation, per Better-Auth docs) — but a + // failed send must be logged, never crash the process (see sendAuthEmailSafely). + // Note: delivery failures are also logged (with masked recipient) by the + // email-verification service before rethrowing; this line additionally + // covers failures thrown outside the service's own try/catch. + sendAuthEmailSafely( + () => sendVerificationEmail(data), + (error) => + logger.error( + `Failed to send verification email: ${error instanceof Error ? error.message : 'Unknown error'}`, + error instanceof Error ? error.stack : undefined, + ), + ); }; } diff --git a/tests/db-lifecycle.reporter.ts b/tests/db-lifecycle.reporter.ts new file mode 100644 index 00000000..fee7d36a --- /dev/null +++ b/tests/db-lifecycle.reporter.ts @@ -0,0 +1,155 @@ +import { createHash } from 'crypto'; +import { MongoClient } from 'mongodb'; + +/** + * Matches the per-run database names created by tests/global-setup.ts, + * e.g. `nest-server-e2e-run-1783062000000-p12345`. + * Capture groups: [1] start timestamp (ms), [2] PID of the vitest main process. + */ +export const RUN_DB_PATTERN = /-run-(\d+)-p(\d+)$/; + +/** + * Age limit for stale run databases. Normally staleness is detected via a dead + * PID; this cap only exists for the rare case of PID recycling (the old PID now + * belongs to an unrelated long-lived process, so the DB would never be + * collected by the PID check alone). + */ +const STALE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; + +/** + * Split a MongoDB URI into server part, database name, and query string. + * `mongodb://127.0.0.1/foo?bar=1` → { dbName: 'foo', query: '?bar=1', serverUri: 'mongodb://127.0.0.1' } + */ +export function splitMongoUri(uri: string): { dbName: string; query: string; serverUri: string } { + const match = uri.match(/^(.*)\/([^/?]+)(\?.*)?$/); + if (!match) { + return { dbName: '', query: '', serverUri: uri }; + } + return { dbName: match[2], query: match[3] || '', serverUri: match[1] }; +} + +/** + * Derive an additional per-run database URI for tests that need their own, + * separate database (e.g. multi-tenancy or plugin isolation). NEVER hardcode a + * fixed database name in a test — fixed names are shared between concurrent + * runs (mutual interference) and their leftovers are collected by nothing. + * + * The name is derived from this run's unique database, so the db-lifecycle + * cleanup collects it automatically (success → dropped with the run; aborted + * run → collected by the next successful run via the dead-PID rule). + * + * MongoDB caps database names at 63 characters — longer derived names are + * truncated and made unique again with a short deterministic hash. + */ +export function deriveTestDbUri(suffix: string): string { + const baseUri = process.env.MONGODB_URI || 'mongodb://127.0.0.1/nest-server-e2e'; + const { dbName, query, serverUri } = splitMongoUri(baseUri); + let name = `${dbName}-${suffix}`; + if (name.length > 63) { + const hash = createHash('sha1').update(name).digest('hex').slice(0, 8); + name = `${name.slice(0, 54)}-${hash}`; + } + return `${serverUri}/${name}${query}`; +} + +function isPidAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +/** + * Vitest reporter managing the lifecycle of per-run test databases + * (created by tests/global-setup.ts): + * + * - Run PASSED → drop this run's database immediately AND collect leftovers: + * run databases whose creating process is dead (crashed or failed earlier + * runs), run databases older than 7 days (PID-recycling fallback), and the + * legacy fixed-name database from before the unique-name scheme. + * - Run FAILED or was interrupted → KEEP this run's database so the state can + * be inspected for debugging; the next successful run removes it. + * + * Cleanup is strictly best-effort: any error here must never affect the test + * exit code. Note: running vitest with an explicit `--reporter` CLI flag + * replaces the config reporters (including this one) — leftovers from such + * runs are collected by the next regular successful run. + */ +export default class DbLifecycleReporter { + async onTestRunEnd( + _testModules: unknown, + unhandledErrors: readonly unknown[], + reason: 'failed' | 'interrupted' | 'passed', + ): Promise { + const uri = process.env.MONGODB_URI; + if (!uri) { + return; + } + const { dbName, serverUri } = splitMongoUri(uri); + if (!RUN_DB_PATTERN.test(dbName)) { + // Externally pinned database (e.g. CI service container) — not managed here. + return; + } + + if (reason !== 'passed' || unhandledErrors.length > 0) { + console.info( + `\n⚠ Test database kept for debugging: ${dbName}` + + `\n URI: ${serverUri}/${dbName}` + + '\n It will be removed automatically by the next successful test run.', + ); + return; + } + + try { + const connection = await MongoClient.connect(uri); + try { + const dropped: string[] = []; + await connection.db(dbName).dropDatabase(); + dropped.push(dbName); + + const base = dbName.replace(RUN_DB_PATTERN, ''); + // Legacy pre-unique-scheme leftovers carrying a trailing timestamp, + // e.g. `-setup-skip-1783062745355` from aborted runs of older + // code. The 1h age guard protects a concurrently running old-code suite. + const legacyTimestamped = new RegExp(`^${base}-.+-(\\d{13})$`); + const { databases } = await connection.db().admin().listDatabases({ nameOnly: true }); + for (const { name } of databases) { + if (name === dbName) { + continue; + } + let stale = false; + if (name.startsWith(`${dbName}-`)) { + // DB derived from THIS run's name (e.g. its setup-skip DB) — the run is over. + stale = true; + } else if (name === base) { + // Legacy fixed-name test DB (pre unique-name scheme) — nothing writes it anymore. + stale = true; + } else if (name.startsWith(`${base}-run-`)) { + // Another run's DB (or a DB derived from it): stale when its + // creating process is dead or it exceeded the age cap. + const match = name.match(/-run-(\d+)-p(\d+)/); + stale = match + ? !isPidAlive(Number(match[2])) || Date.now() - Number(match[1]) > STALE_MAX_AGE_MS + : false; + } else { + const legacy = name.match(legacyTimestamped); + stale = legacy ? Date.now() - Number(legacy[1]) > 60 * 60 * 1000 : false; + } + if (stale) { + await connection.db(name).dropDatabase(); + dropped.push(name); + } + } + console.info(`Test database cleanup: dropped ${dropped.join(', ')}`); + } finally { + await connection.close(); + } + } catch (error) { + console.warn( + `Test database cleanup skipped: ${error instanceof Error ? error.message : 'Unknown error'}`, + ); + } + } +} diff --git a/tests/error-code-scenarios.e2e-spec.ts b/tests/error-code-scenarios.e2e-spec.ts index 3615f238..b3fa6ac5 100644 --- a/tests/error-code-scenarios.e2e-spec.ts +++ b/tests/error-code-scenarios.e2e-spec.ts @@ -31,6 +31,7 @@ import { mergeErrorCodes, TestHelper, } from '../src'; +import { deriveTestDbUri } from './db-lifecycle.reporter'; // ============================================================================= // Test Error Registries @@ -120,7 +121,7 @@ const getTestConfig = (dbSuffix: string) => ({ signInOptions: { expiresIn: '15m' as const }, }, mongoose: { - uri: `mongodb://127.0.0.1/nest-server-${dbSuffix}`, + uri: deriveTestDbUri(dbSuffix), }, port: 0, }); diff --git a/tests/global-setup.ts b/tests/global-setup.ts index f132e006..f802df88 100644 --- a/tests/global-setup.ts +++ b/tests/global-setup.ts @@ -1,15 +1,43 @@ import { MongoClient } from 'mongodb'; import envConfig from '../src/config.env'; +import { splitMongoUri } from './db-lifecycle.reporter'; /** - * Vitest global setup: Drop the test database before running tests - * to ensure a clean state with no leftovers from previous runs. + * Vitest global setup: give every test run its OWN database. + * + * Why not a fixed name + dropDatabase: two suites running at the same time + * (second terminal, IDE runner, parallel agent session) would share one DB, + * and the later run's drop wipes the earlier run's users/sessions mid-flight — + * observed as sudden 401s and even wedged app bootstraps. A unique name per + * run makes concurrent runs fully isolated. + * + * The name is set via process.env.MONGODB_URI BEFORE the fork workers spawn, + * so config.env.ts inside every worker resolves to this run's database. + * + * Lifecycle (see tests/db-lifecycle.reporter.ts): + * - run PASSES → its database is dropped right away, and stale run databases + * from earlier crashed/failed runs (dead PID or older than 7 days) plus the + * legacy fixed-name database are collected too; + * - run FAILS → its database is KEPT for debugging and removed automatically + * by the next successful run. + * + * An externally provided MONGODB_URI (e.g. CI service container) opts out of + * the unique-name scheme: that URI is used as-is and dropped up front, exactly + * like the previous behavior. */ export async function setup() { - const connection = await MongoClient.connect(envConfig.mongoose.uri); - const db = connection.db(); - await db.dropDatabase(); - console.info(`Dropped database: ${db.databaseName}`); - await connection.close(); + if (process.env.MONGODB_URI) { + const connection = await MongoClient.connect(process.env.MONGODB_URI); + const db = connection.db(); + await db.dropDatabase(); + console.info(`Dropped externally configured test database: ${db.databaseName}`); + await connection.close(); + return; + } + + const { dbName, query, serverUri } = splitMongoUri(envConfig.mongoose.uri); + const runDbName = `${dbName}-run-${Date.now()}-p${process.pid}`; + process.env.MONGODB_URI = `${serverUri}/${runDbName}${query}`; + console.info(`Test database for this run: ${runDbName}`); } diff --git a/tests/migrate/mongo-state-store.e2e-spec.ts b/tests/migrate/mongo-state-store.e2e-spec.ts index d6b8315e..e4c5bdd0 100644 --- a/tests/migrate/mongo-state-store.e2e-spec.ts +++ b/tests/migrate/mongo-state-store.e2e-spec.ts @@ -25,9 +25,12 @@ import { MongoClient } from 'mongodb'; import { promisify } from 'util'; import { MigrationOptions, MongoStateStore, synchronizedMigration } from '../../src'; +import { deriveTestDbUri } from '../db-lifecycle.reporter'; describe('MongoDB State Store for Migrations (e2e)', () => { - const mongoUrl = process.env.MONGODB_URL || 'mongodb://127.0.0.1/nest-server-local'; + // NEVER default to nest-server-local here — that is the developer's LOCAL DEV + // database, and this suite manipulates its `migrations` collection. + const mongoUrl = process.env.MONGODB_URL || deriveTestDbUri('migrate'); const defaultCollectionName = 'migrations'; // List of all collections used in tests for comprehensive cleanup diff --git a/tests/mongoose-plugins.e2e-spec.ts b/tests/mongoose-plugins.e2e-spec.ts index f72088b0..6df6907e 100644 --- a/tests/mongoose-plugins.e2e-spec.ts +++ b/tests/mongoose-plugins.e2e-spec.ts @@ -12,6 +12,7 @@ import { hashPassword, mongoosePasswordPlugin } from '../src/core/common/plugins import { mongooseRoleGuardPlugin } from '../src/core/common/plugins/mongoose-role-guard.plugin'; import { ConfigService } from '../src/core/common/services/config.service'; import { IRequestContext, RequestContext } from '../src/core/common/services/request-context.service'; +import { deriveTestDbUri } from './db-lifecycle.reporter'; // ============================================================================= // Test Schema: PluginTestUser (has password, roles, createdBy, updatedBy) @@ -39,7 +40,7 @@ const PluginTestUserSchema = SchemaFactory.createForClass(PluginTestUser); // ============================================================================= // Test Module // ============================================================================= -const TEST_DB_URI = 'mongodb://127.0.0.1/nest-server-plugins-test'; +const TEST_DB_URI = deriveTestDbUri('plugins'); @Module({ imports: [ diff --git a/tests/multi-tenancy.e2e-spec.ts b/tests/multi-tenancy.e2e-spec.ts index 5ecb19e4..2a1445fb 100644 --- a/tests/multi-tenancy.e2e-spec.ts +++ b/tests/multi-tenancy.e2e-spec.ts @@ -9,6 +9,7 @@ import { Model } from 'mongoose'; import { ConfigService } from '../src/core/common/services/config.service'; import { IRequestContext, RequestContext } from '../src/core/common/services/request-context.service'; import { mongooseTenantPlugin } from '../src/core/common/plugins/mongoose-tenant.plugin'; +import { deriveTestDbUri } from './db-lifecycle.reporter'; // ============================================================================= // Test Schema: TenantItem (has tenantId → plugin activates) @@ -38,7 +39,7 @@ const GlobalItemSchema = SchemaFactory.createForClass(GlobalItem); // ============================================================================= // Test Module // ============================================================================= -const TEST_DB_URI = 'mongodb://127.0.0.1/nest-server-mt-test'; +const TEST_DB_URI = deriveTestDbUri('mt'); @Module({ imports: [ @@ -672,7 +673,7 @@ describe('Multi-Tenancy Plugin Disabled (e2e)', () => { @Module({ imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-mt-disabled-test', { + MongooseModule.forRoot(deriveTestDbUri('mt-off'), { connectionFactory: (connection) => { // Plugin still registered globally, but config says disabled // The plugin itself checks tenantId field, but resolveTenantId reads config diff --git a/tests/stories/system-setup.e2e-spec.ts b/tests/stories/system-setup.e2e-spec.ts index 6941fd63..de454b1d 100644 --- a/tests/stories/system-setup.e2e-spec.ts +++ b/tests/stories/system-setup.e2e-spec.ts @@ -401,7 +401,11 @@ describe('Story: System Setup - Auto-Creation via Config', () => { // ============================================================================= describe('Story: System Setup - Auto-Creation skipped with existing users', () => { - const SKIP_DB = `nest-server-e2e-setup-skip-${Date.now()}`; + // Derive from the current (per-run) test DB name so an aborted run's leftover + // is collected by the db-lifecycle reporter of the next successful run. + // Short suffix only: the run DB name is already unique, and MongoDB caps + // database names at 63 characters. + const SKIP_DB = `${envConfig.mongoose.uri.split('/').pop().split('?')[0]}-skip`; const SKIP_ADMIN_EMAIL = `skip-admin-${Date.now()}@test.com`; let app; diff --git a/tests/tenant-guard.e2e-spec.ts b/tests/tenant-guard.e2e-spec.ts index b233572b..0ad2930e 100644 --- a/tests/tenant-guard.e2e-spec.ts +++ b/tests/tenant-guard.e2e-spec.ts @@ -29,6 +29,7 @@ import { isSystemRole, } from '../src/core/modules/tenant/core-tenant.helpers'; import { CoreTenantService } from '../src/core/modules/tenant/core-tenant.service'; +import { deriveTestDbUri } from './db-lifecycle.reporter'; // ============================================================================= // Test Schema: TenantMember @@ -215,7 +216,7 @@ class TestController { // ============================================================================= // Test Module // ============================================================================= -const TEST_DB_URI = 'mongodb://127.0.0.1/nest-server-tenant-guard-test'; +const TEST_DB_URI = deriveTestDbUri('tg'); @Module({ controllers: [TestController, AdminFallbackController], @@ -1323,7 +1324,7 @@ describe('CoreTenantService (e2e)', () => { const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-svc-test'), + MongooseModule.forRoot(deriveTestDbUri('tg-svc')), MongooseModule.forFeature([{ name: 'TenantMember', schema: TenantMemberSchema }]), ], providers: [CoreTenantService], @@ -1632,7 +1633,7 @@ describe('CoreTenantModule.forRoot() customization', () => { @Module({ controllers: [TestController], imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-custom-model-test'), + MongooseModule.forRoot(deriveTestDbUri('tg-cmodel')), CoreTenantModule.forRoot({ modelName: 'OrgMember' }), ], }) @@ -1709,7 +1710,7 @@ describe('CoreTenantModule.forRoot() customization', () => { @Module({ controllers: [TestController], imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-custom-svc-test'), + MongooseModule.forRoot(deriveTestDbUri('tg-csvc')), CoreTenantModule.forRoot({ service: CustomTenantService }), ], }) @@ -2172,7 +2173,7 @@ describe('CoreTenantGuard with multiTenancy disabled', () => { const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-disabled-test', { + MongooseModule.forRoot(deriveTestDbUri('tg-off'), { connectionFactory: (connection) => { connection.plugin(mongooseTenantPlugin); return connection; @@ -2781,7 +2782,7 @@ describe('CoreTenantGuard: BetterAuth auto-skip with header present (regression) @Module({ controllers: [IamTestController], imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-ba-skip-test', { + MongooseModule.forRoot(deriveTestDbUri('tg-ba-skip'), { connectionFactory: (connection) => { connection.plugin(mongooseTenantPlugin); return connection; @@ -2916,7 +2917,7 @@ describe('CoreTenantGuard: BetterAuth skipTenantCheck: false (opt-out)', () => { @Module({ controllers: [IamOptOutController], imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-ba-optout-test', { + MongooseModule.forRoot(deriveTestDbUri('tg-ba-optout'), { connectionFactory: (connection) => { connection.plugin(mongooseTenantPlugin); return connection; @@ -3003,7 +3004,7 @@ describe('CoreTenantGuard: Non-BetterAuth controller does not auto-skip', () => @Module({ controllers: [RegularController], imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-nonskip-test', { + MongooseModule.forRoot(deriveTestDbUri('tg-nonskip'), { connectionFactory: (connection) => { connection.plugin(mongooseTenantPlugin); return connection; @@ -3117,7 +3118,7 @@ describe('CoreTenantGuard: BetterAuth config missing — safe default skip', () @Module({ controllers: [IamNoConfigController], imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-ba-noconfig-test', { + MongooseModule.forRoot(deriveTestDbUri('tg-ba-nocfg'), { connectionFactory: (connection) => { connection.plugin(mongooseTenantPlugin); return connection; @@ -3221,7 +3222,7 @@ describe('CoreTenantGuard: BetterAuth resolver subclass auto-skip', () => { @Module({ controllers: [IamResolverTestController], imports: [ - MongooseModule.forRoot('mongodb://127.0.0.1/nest-server-tenant-ba-resolver-test', { + MongooseModule.forRoot(deriveTestDbUri('tg-ba-resolver'), { connectionFactory: (connection) => { connection.plugin(mongooseTenantPlugin); return connection; diff --git a/tests/unit/better-auth-email-safe.spec.ts b/tests/unit/better-auth-email-safe.spec.ts new file mode 100644 index 00000000..e501f919 --- /dev/null +++ b/tests/unit/better-auth-email-safe.spec.ts @@ -0,0 +1,97 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { sendAuthEmailSafely } from '../../src/core/modules/better-auth/better-auth.config'; + +/** + * Regression guard: an auth email (verification / password-reset) is sent + * fire-and-forget so it cannot leak timing information about whether an account + * exists. But a rejected send must NEVER surface as an unhandled promise + * rejection — that crashes the Node process (observed in dev: signing up an + * unverified user then signing in took the whole API down when SMTP was not + * configured and the verification-mail send rejected). + * + * `sendAuthEmailSafely` must therefore: stay non-blocking, catch BOTH sync + * throws and async rejections, routing them to the onError logger instead — + * and never crash even when onError itself throws. + */ +describe('sendAuthEmailSafely', () => { + // Macrotask boundary (not just a microtask flush): Node emits + // 'unhandledRejection' only after the microtask queue has drained, so the + // unhandled-rejection tests below need a real task hop to observe it. + const flush = () => new Promise((resolve) => setTimeout(resolve, 0)); + + it('routes an async rejection to onError instead of leaving it unhandled', async () => { + const onError = vi.fn(); + const err = new Error('SMTP down'); + sendAuthEmailSafely(() => Promise.reject(err), onError); + await flush(); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith(err); + }); + + it('catches a synchronous throw from the send callback', async () => { + const onError = vi.fn(); + sendAuthEmailSafely(() => { + throw new Error('boom'); + }, onError); + await flush(); + expect(onError).toHaveBeenCalledTimes(1); + }); + + it('does not call onError when the send succeeds', async () => { + const onError = vi.fn(); + sendAuthEmailSafely(() => Promise.resolve('sent'), onError); + await flush(); + expect(onError).not.toHaveBeenCalled(); + }); + + it('is non-blocking — it returns before a slow send resolves', async () => { + const onError = vi.fn(); + let sendResolved = false; + sendAuthEmailSafely( + () => + new Promise((resolve) => { + setTimeout(() => { + sendResolved = true; + resolve(undefined); + }, 50); + }), + onError, + ); + // The helper returned synchronously; the send has NOT completed yet. + expect(sendResolved).toBe(false); + await flush(); + expect(onError).not.toHaveBeenCalled(); + }); + + it('does not produce an unhandled rejection for a rejecting send', async () => { + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => unhandled.push(reason); + process.on('unhandledRejection', onUnhandled); + try { + sendAuthEmailSafely(() => Promise.reject(new Error('no smtp')), vi.fn()); + await flush(); + expect(unhandled).toHaveLength(0); + } finally { + process.off('unhandledRejection', onUnhandled); + } + }); + + it('does not produce an unhandled rejection when onError itself throws', async () => { + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => unhandled.push(reason); + process.on('unhandledRejection', onUnhandled); + try { + sendAuthEmailSafely( + () => Promise.reject(new Error('send failed')), + () => { + throw new Error('logger transport failed'); + }, + ); + await flush(); + expect(unhandled).toHaveLength(0); + } finally { + process.off('unhandledRejection', onUnhandled); + } + }); +}); diff --git a/vitest-e2e.config.ts b/vitest-e2e.config.ts index e2f8ba68..4cd80283 100644 --- a/vitest-e2e.config.ts +++ b/vitest-e2e.config.ts @@ -17,12 +17,16 @@ export default defineConfig({ test: { environment: 'node', // Exclude type-only test files (run these with `npx tsc --noEmit` instead) - exclude: ['tests/types/**/*.ts', 'tests/global-setup.ts', 'tests/setup.ts'], + // and test-infrastructure modules (global setup, worker setup, DB reporter) + exclude: ['tests/types/**/*.ts', 'tests/global-setup.ts', 'tests/setup.ts', 'tests/db-lifecycle.reporter.ts'], // Enable parallel file execution for speed fileParallelism: true, globalSetup: ['tests/global-setup.ts'], globals: true, - hookTimeout: 60000, + // Hooks are NOT covered by `retry` (tests only) — a beforeAll that boots a + // full Nest app can exceed 60s under load (parallel transform/import), + // which failed whole files without any retry. Be generous here. + hookTimeout: 120000, include: ['tests/**/*.ts'], // Runs in every test worker before test files are imported (filters expected log/warn noise) setupFiles: ['tests/setup.ts'], @@ -35,7 +39,9 @@ export default defineConfig({ maxConcurrency: 3, // Use forks instead of threads for better NestJS performance pool: 'forks', - reporters: ['default'], + // db-lifecycle: drops this run's unique DB on success (+ collects stale + // run DBs), keeps it for debugging on failure — see tests/db-lifecycle.reporter.ts + reporters: ['default', './tests/db-lifecycle.reporter.ts'], // Retry flaky tests up to 3 times before failing // This handles intermittent MongoDB race conditions retry: 5,