diff --git a/apps/api/tests/api-http.integration.ts b/apps/api/tests/api-http.integration.ts index bef43b3..990a55c 100644 --- a/apps/api/tests/api-http.integration.ts +++ b/apps/api/tests/api-http.integration.ts @@ -2779,6 +2779,73 @@ describe.sequential("live API — bounded HTTP drain and shutdown", () => { } }); + it("lets an already-entered request finish and close cleanly rather than force-closing it", async () => { + // Deterministic rather than racing an unresolved `fetch`: a raw socket sends a complete request + // with `Connection: close`, and shutdown only begins once the server provably holds that + // connection — observed by polling the real server's own connection count, never by sleeping. + const database = await createDisposableDatabase("drain_order"); + await applyCoreMigrations(database.url); + const running = await startConfiguredApi(database.url, { drainTimeoutMsOverride: 20_000 }); + const address = running.server.address(); + if (!address || typeof address !== "object") { + throw harnessError("The API integration server has no bound address."); + } + const socket = net.connect({ host: "127.0.0.1", port: address.port }); + try { + const responseBytes: Buffer[] = []; + const responseClosed = new Promise((resolve) => { + socket.once("close", () => resolve()); + }); + socket.on("data", (chunk: Buffer) => { + responseBytes.push(chunk); + }); + await new Promise((resolve, reject) => { + socket.once("error", reject); + socket.write( + "GET /api/v1/principal HTTP/1.1\r\nHost: 127.0.0.1\r\nConnection: close\r\n\r\n", + () => resolve(), + ); + }); + + // Bounded, condition-based: wait until the server actually holds the connection, so the + // request has demonstrably entered before shutdown starts. + const connectionCounter = running.server as unknown as { + getConnections?: (callback: (error: Error | null, count: number) => void) => void; + }; + const deadline = Date.now() + 15_000; + for (;;) { + const connections = await new Promise((resolve) => { + if (typeof connectionCounter.getConnections !== "function") { + resolve(1); + return; + } + connectionCounter.getConnections((_error, count) => resolve(count ?? 0)); + }); + if (connections > 0) { + break; + } + if (Date.now() > deadline) { + throw harnessError("The request never reached the server."); + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } + + // The graceful path: nothing is stalled, so shutdown completes without reaching the drain + // deadline and without force-closing anything. + await expect(withDeadline(running.shutdown(), 30_000)).resolves.toBeUndefined(); + + // The already-entered request completed normally — full response, clean transport close. + await withDeadline(responseClosed, 10_000); + const raw = Buffer.concat(responseBytes).toString("utf8"); + expect(raw).toContain("HTTP/1.1 200"); + expect(raw).toContain('{"kind":"anonymous"}'); + } finally { + socket.destroy(); + await running.shutdown().catch(() => undefined); + await dropDatabase(database.name); + } + }); + it("force-closes a stalled connection at the drain deadline and still reaches pool closure", async () => { const database = await createDisposableDatabase("drain_forced"); await applyCoreMigrations(database.url); diff --git a/apps/api/tests/lifecycle.test.ts b/apps/api/tests/lifecycle.test.ts index 02fb776..23f4781 100644 --- a/apps/api/tests/lifecycle.test.ts +++ b/apps/api/tests/lifecycle.test.ts @@ -130,6 +130,9 @@ interface WorkingFixtureModuleOptions { readonly registerThrows?: boolean; readonly startThrows?: boolean; readonly stopThrows?: boolean; + /** Test-only: the stop hook records its marker and then never settles, so an API-level deadline + * can be exercised deterministically without any sleep. */ + readonly stopNeverSettles?: boolean; readonly dependencies?: Record; } @@ -174,6 +177,7 @@ async function createWorkingFixtureModule( stop() { ${recorder.expression}.push(${JSON.stringify(`${id}:stop`)}); ${options.stopThrows ? 'throw new Error("stop failed");' : ""} + ${options.stopNeverSettles ? "return new Promise(() => {});" : ""} }, };`; await writeFile(path.join(modulesDirectory, id, "dist/server/index.js"), serverSource); @@ -1230,4 +1234,154 @@ describe("Ekklesia API module runtime startup and shutdown", () => { expect(betterAuthMock).not.toHaveBeenCalled(); expect(serveMock).not.toHaveBeenCalled(); }); + + /** + * Pass 4 correction: the API composition's own bounded-cleanup guarantees. + * + * Core proves an individual hook deadline in isolation; these prove that `startApi`/`shutdown` + * actually reach pool closure through those deadlines, which is where the platform's promise ("the + * pool always closes") is either kept or broken. Every deadline here is a short, test-only + * override; production keeps the 10-second hook and 30-second drain defaults. + */ + describe("Ekklesia API bounded module cleanup", () => { + it("bounds a never-settling startup rollback, still rolls back earlier modules, closes the pool, and binds no listener", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "one", recorder); + await createWorkingFixtureModule(modulesDirectory, "two", recorder, { + dependencies: { modules: { required: { one: "^0.1.0" } } }, + startThrows: true, + stopNeverSettles: true, + }); + const { poolEnd } = mockPoolAndAuth(); + + await expect( + startApi(validConfig(), { + moduleHookTimeoutMsOverride: 25, + modulesDirectoryOverride: modulesDirectory, + }), + ).rejects.toMatchObject({ + issues: [ + expect.objectContaining({ code: "MODULE_START_FAILED", moduleId: "two" }), + expect.objectContaining({ code: "MODULE_ROLLBACK_TIMEOUT", moduleId: "two" }), + ], + }); + + // The failing module's own stop hook was attempted first and timed out; the earlier module + // still received its rollback afterwards. + expect(recorder.events).toEqual([ + "one:register", + "two:register", + "one:start", + "two:start", + "two:stop", + "one:stop", + ]); + expect(poolConstructor).toHaveBeenCalledTimes(1); + expect(poolEnd).toHaveBeenCalledTimes(1); + expect(serveMock).not.toHaveBeenCalled(); + }); + + it("bounds a never-settling normal stop, attempts the earlier module, and still closes the pool", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "one", recorder); + await createWorkingFixtureModule(modulesDirectory, "two", recorder, { + dependencies: { modules: { required: { one: "^0.1.0" } } }, + stopNeverSettles: true, + }); + const { poolEnd } = mockPoolAndAuth(); + mockSuccessfulServe(); + + const running = await startApi(validConfig(), { + moduleHookTimeoutMsOverride: 25, + modulesDirectoryOverride: modulesDirectory, + }); + + await expect(running.shutdown()).rejects.toMatchObject({ + drainTimedOut: false, + moduleStopError: { + issues: [expect.objectContaining({ code: "MODULE_STOP_TIMEOUT", moduleId: "two" })], + }, + moduleStopFailed: true, + name: "ApiShutdownError", + }); + expect(recorder.events.slice(-2)).toEqual(["two:stop", "one:stop"]); + expect(poolEnd).toHaveBeenCalledTimes(1); + + // Repeated and concurrent callers share the one memoized result and restart nothing. + const [first, second] = await Promise.allSettled([running.shutdown(), running.shutdown()]); + expect(first.status).toBe("rejected"); + expect(second.status).toBe("rejected"); + expect(recorder.events.filter((event) => event === "two:stop")).toHaveLength(1); + expect(recorder.events.filter((event) => event === "one:stop")).toHaveLength(1); + expect(poolEnd).toHaveBeenCalledTimes(1); + }); + + it("reports a drain timeout and a module-stop timeout together, after the pool has closed", async () => { + const modulesDirectory = await createTemporaryModulesDirectory(); + const recorder = createEventRecorder(); + await createWorkingFixtureModule(modulesDirectory, "stalls", recorder, { + stopNeverSettles: true, + }); + const { poolEnd } = mockPoolAndAuth(); + // A server whose `close` callback never fires stands in for a connection that never drains, + // deterministically and without a real socket. `closeIdleConnections`/`closeAllConnections` + // are the supported public APIs the platform calls at the deadline. + const closeAllConnections = vi.fn(); + const closeIdleConnections = vi.fn(); + const { server } = fakeServer(); + // `close` deliberately never invokes its callback: nothing drains, which is exactly what the + // drain deadline exists for — deterministic, with no real socket and no sleep. + const stalled = Object.assign(server, { + close: vi.fn(() => server), + closeAllConnections, + closeIdleConnections, + }); + serveMock.mockImplementation((_options: unknown, listeningListener?: () => void) => { + // Deferred exactly like `mockSuccessfulServe`: `bindServer` resolves with `server`, which + // is still in its temporal dead zone during the synchronous `serve()` call. + queueMicrotask(() => listeningListener?.()); + return stalled; + }); + + const running = await startApi(validConfig(), { + drainTimeoutMsOverride: 25, + moduleHookTimeoutMsOverride: 25, + modulesDirectoryOverride: modulesDirectory, + }); + + let captured: unknown; + try { + await running.shutdown(); + } catch (error) { + captured = error; + } + + expect(captured).toMatchObject({ + drainTimedOut: true, + moduleStopFailed: true, + name: "ApiShutdownError", + }); + const issues = (captured as { moduleStopError: { issues: Array<{ code: string }> } }) + .moduleStopError.issues; + expect(issues).toEqual([ + expect.objectContaining({ code: "MODULE_STOP_TIMEOUT", moduleId: "stalls" }), + ]); + // The platform force-closed through Node's public APIs, then still stopped modules and closed + // the pool — neither deadline skipped a later stage. + expect(closeIdleConnections).toHaveBeenCalledTimes(1); + expect(closeAllConnections).toHaveBeenCalledTimes(1); + expect(recorder.events).toContain("stalls:stop"); + expect(poolEnd).toHaveBeenCalledTimes(1); + + // Nothing about the request, socket, connection, or module exception leaks. + const publicText = `${String(captured)}\n${(captured as Error).stack ?? ""}\n${JSON.stringify( + captured, + )}`; + for (const secret of ["127.0.0.1", "postgres", "password", "select ", "Error: stop"]) { + expect(publicText.toLowerCase()).not.toContain(secret.toLowerCase()); + } + }); + }); });