fix(server): stop Sentry from re-crashing the process on the neutralized postgres fault

Sentry's default `OnUncaughtException` integration registered its own
`uncaughtException` listener. Node calls every registered listener, not
just the first one, so that listener still ended the process on the
known, neutralized postgres driver crash even after the guard's own
listener logged and returned. The initializer now removes that default
integration, and the guard reports every uncaught exception it does not
neutralize to Sentry itself (through `captureException`, then
`shutdownSentry` to flush) before it ends the process. Documents
POSTGRES_NULL_SOCKET_GUARD_ENABLED in doc/observability.md.

Co-authored-by: Paperclip <noreply@paperclip.ing>

doc/observability.md

@@ -382,13 +382,17 @@ integration list of `@sentry/node@10.71.0` and `@sentry/browser@10.71.0`.
 
 **Server events the default integrations add**
 
-- `OnUncaughtException` — each uncaught exception on the main thread, at
-  level `fatal`. The process still exits.
 - `OnUnhandledRejection` — each unhandled promise rejection. The mode is
   `strict`, so the process exits after the capture.
 - `ChildProcess` — one event for each worker-thread `error`.
 - `LinkedErrors` — the `error.cause` chain of each captured error.
 
+An uncaught exception on the main thread is a server event too, but it does
+not come from a default Sentry integration. See "PostgreSQL Null-Socket
+Write Guard" below: that guard, not `OnUncaughtException`, owns the
+process's `uncaughtException` decision, and it reports the exception to
+Sentry itself before the process exits.
+
 **Server context the kept integrations attach**
 
 - `RequestData` — attaches nothing under this feature's configuration. See
@@ -409,6 +413,9 @@ integration list of `@sentry/node@10.71.0` and `@sentry/browser@10.71.0`.
 - `ContextLines` — 7 local source lines around each stack frame.
 - The outbound breadcrumb of `Http` — outbound request URLs and query
   strings.
+- `OnUncaughtException` — removed so it never registers its own
+  `uncaughtException` listener. See "PostgreSQL Null-Socket Write Guard"
+  below for why one listener, not two, must own that event.
 
 **Browser events this feature adds**
 
@@ -450,6 +457,64 @@ Two controls belong to the operator. This feature ships neither one.
    an internal-only host name fails silently for the browser even when it
    works for the server.
 
+## PostgreSQL Null-Socket Write Guard
+
+The `postgres` driver, at the version this repository pins
+(`postgres@3.4.9`), has a known defect. When a database backend closes a
+connection that a transaction still holds, a later query on that same
+connection can still start. The driver defers that query's wire write to a
+timer callback. By the time the callback runs, the driver has already set
+its socket reference to `null`, and the write throws a `TypeError`. No
+application code sits between that timer callback and the process, so
+Node ends the process by default.
+
+The server adds one process-level guard against this exact fault. The
+guard does not patch the driver and does not change the driver version.
+It adds one `uncaughtException` listener, early in server startup, before
+every other startup step. For the one known, matching `TypeError`, the
+listener logs one structured line with a fixed marker
+(`postgres_null_socket_write_guard_neutralized`) and the error name, then
+lets the process keep running. For every other uncaught exception, the
+listener keeps today's default behavior: it logs the error, reports it to
+Sentry when Sentry is enabled (see "Sentry Error Monitoring" above), waits
+for that report to flush, then ends the process with exit code 1.
+
+### `POSTGRES_NULL_SOCKET_GUARD_ENABLED`
+
+This environment variable is the guard's one operator control.
+
+- **Default:** the guard is on. An unset variable, or an empty string,
+  means enabled.
+- **Accepted values:** `true` or `1` keep the guard on. `false` or `0`
+  turn it off. The comparison ignores letter case and leading or trailing
+  spaces.
+- **An unrecognized value stops the server at startup.** A value other
+  than the six listed above (for example `yes` or `disabled`) throws an
+  error that names the variable and the value the operator set, instead
+  of silently guessing what the operator meant.
+- **Operational effect of turning the guard off:** the server registers
+  no `uncaughtException` listener of its own for this fault, so Node's
+  default behavior returns for it: the known, otherwise-neutralized
+  `TypeError` ends the server process the same way it did before this
+  guard existed. Turn the guard off only to compare behavior against the
+  unguarded process, or once the operator has upgraded past the
+  underlying driver defect.
+
+### Sentry interaction
+
+An operator who also enables server-side Sentry error monitoring gets no
+second, independent `uncaughtException` listener from Sentry. `sentry.ts`
+removes Sentry's default `OnUncaughtException` integration for exactly
+this reason: Node calls every listener registered for an event, not only
+the first one, so a second listener would still end the process on the
+known, neutralized fault, regardless of which listener runs first. This
+guard is the sole owner of the process's `uncaughtException` decision. It
+reports every exception it does not neutralize to Sentry itself, through
+the same `captureException` and `shutdownSentry` calls a server startup
+failure uses (see "Sentry Error Monitoring" above). A deployment with no
+Sentry DSN set sees no change from this interaction: both calls are
+no-ops.
+
 ## Sandbox Startup Trace Spans
 
 Paperclip opens OpenTelemetry spans on the sandbox start path. These spans are

server/src/__tests__/postgres-null-socket-guard.test.ts

@@ -5,6 +5,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
 const mockLoggerError = vi.hoisted(() => vi.fn());
 const mockLoggerFatal = vi.hoisted(() => vi.fn());
+const mockCaptureException = vi.hoisted(() => vi.fn());
+const mockShutdownSentry = vi.hoisted(() => vi.fn(async () => {}));
 
 vi.mock("../middleware/logger.js", () => ({
   logger: {
@@ -13,6 +15,11 @@ vi.mock("../middleware/logger.js", () => ({
   },
 }));
 
+vi.mock("../sentry.js", () => ({
+  captureException: mockCaptureException,
+  shutdownSentry: mockShutdownSentry,
+}));
+
 const {
   handlePostgresNullSocketGuardException,
   isGuardEnabled,
@@ -167,6 +174,9 @@ describe("handlePostgresNullSocketGuardException", () => {
   beforeEach(() => {
     mockLoggerError.mockReset();
     mockLoggerFatal.mockReset();
+    mockCaptureException.mockReset();
+    mockShutdownSentry.mockReset();
+    mockShutdownSentry.mockImplementation(async () => {});
     exitSpy = vi.spyOn(process, "exit").mockImplementation((() => undefined) as unknown as never);
   });
 
@@ -174,10 +184,10 @@ describe("handlePostgresNullSocketGuardException", () => {
     exitSpy.mockRestore();
   });
 
-  it("logs a stable marker and the error name, and does not exit, on a matching crash", () => {
+  it("logs a stable marker and the error name, and does not exit, on a matching crash", async () => {
     const error = makeDriverCrashError("Cannot read properties of null (reading 'write')");
 
-    handlePostgresNullSocketGuardException(error);
+    await handlePostgresNullSocketGuardException(error);
 
     expect(exitSpy).not.toHaveBeenCalled();
     expect(mockLoggerError).toHaveBeenCalledTimes(1);
@@ -188,7 +198,7 @@ describe("handlePostgresNullSocketGuardException", () => {
     });
   });
 
-  it("never logs the query text, the message, or the stack for a matching crash", () => {
+  it("never logs the query text, the message, or the stack for a matching crash", async () => {
     const secret = "select * from accounts where token = 'hunter2-secret-token'";
     const error = makeDriverCrashError(`Cannot read properties of null (reading 'write') — ${secret}`);
     // The classifier only matches the exact V8 message, so force a match by
@@ -197,7 +207,7 @@ describe("handlePostgresNullSocketGuardException", () => {
     // and the fixed error name.
     error.message = "Cannot read properties of null (reading 'write')";
 
-    handlePostgresNullSocketGuardException(error);
+    await handlePostgresNullSocketGuardException(error);
 
     const [fields] = mockLoggerError.mock.calls[0] as [Record<string, unknown>];
     expect(Object.keys(fields).sort()).toEqual(["errorName", "marker"]);
@@ -206,19 +216,48 @@ describe("handlePostgresNullSocketGuardException", () => {
     }
   });
 
-  it("logs and exits non-zero for a non-matching error", () => {
+  it("never reports a matching crash to Sentry — the process survives on its own", async () => {
+    const error = makeDriverCrashError("Cannot read properties of null (reading 'write')");
+
+    await handlePostgresNullSocketGuardException(error);
+
+    expect(mockCaptureException).not.toHaveBeenCalled();
+    expect(mockShutdownSentry).not.toHaveBeenCalled();
+  });
+
+  it("logs, reports to Sentry, flushes, and exits non-zero for a non-matching error", async () => {
     const error = new RangeError("some unrelated fatal condition");
 
-    handlePostgresNullSocketGuardException(error);
+    await handlePostgresNullSocketGuardException(error);
 
     expect(mockLoggerError).not.toHaveBeenCalled();
     expect(mockLoggerFatal).toHaveBeenCalledTimes(1);
+    expect(mockCaptureException).toHaveBeenCalledWith(error);
+    expect(mockShutdownSentry).toHaveBeenCalledTimes(1);
     expect(exitSpy).toHaveBeenCalledWith(1);
   });
 
-  it("exits non-zero for a non-Error thrown value", () => {
-    handlePostgresNullSocketGuardException("a thrown string");
+  it("waits for the Sentry client to flush before it exits", async () => {
+    const error = new RangeError("some unrelated fatal condition");
+    let shutdownResolved = false;
+    mockShutdownSentry.mockImplementation(async () => {
+      await Promise.resolve();
+      shutdownResolved = true;
+    });
+    exitSpy.mockImplementation((() => {
+      expect(shutdownResolved).toBe(true);
+      return undefined;
+    }) as unknown as never);
 
+    await handlePostgresNullSocketGuardException(error);
+
+    expect(exitSpy).toHaveBeenCalledWith(1);
+  });
+
+  it("exits non-zero for a non-Error thrown value", async () => {
+    await handlePostgresNullSocketGuardException("a thrown string");
+
+    expect(mockCaptureException).toHaveBeenCalledWith(expect.any(Error));
     expect(exitSpy).toHaveBeenCalledWith(1);
   });
 });

server/src/__tests__/sentry.test.ts

@@ -403,7 +403,7 @@ describe("buildSentryInitOptions", () => {
     expect(resolved.filter((i) => i.name === "OnUnhandledRejection")).toHaveLength(1);
   });
 
-  it("the resolved server integration list holds no Console integration and no ContextLines integration", async () => {
+  it("the resolved server integration list holds no Console, ContextLines, or OnUncaughtException integration", async () => {
     const { buildSentryInitOptions } = await importFreshSentry();
 
     const options = buildSentryInitOptions("https://public@o0.ingest.sentry.io/1", {
@@ -415,9 +415,14 @@ describe("buildSentryInitOptions", () => {
 
     expect(names).not.toContain("Console");
     expect(names).not.toContain("ContextLines");
+    // Removed so this default integration never registers its own
+    // `uncaughtException` listener — see the module comment in sentry.ts.
+    // `postgres-null-socket-guard.ts` is the sole owner of that decision,
+    // and it reports non-neutralized exceptions to Sentry itself.
+    expect(names).not.toContain("OnUncaughtException");
   });
 
-  it("the resolved server integration list keeps OnUncaughtException, OnUnhandledRejection, LinkedErrors, and RequestData", async () => {
+  it("the resolved server integration list keeps OnUnhandledRejection, LinkedErrors, and RequestData", async () => {
     const { buildSentryInitOptions } = await importFreshSentry();
 
     const options = buildSentryInitOptions("https://public@o0.ingest.sentry.io/1", {
@@ -427,11 +432,10 @@ describe("buildSentryInitOptions", () => {
     const resolved = options.integrations(DEFAULT_INTEGRATION_NAMES.map((name) => ({ name })));
     const names = resolved.map((i) => i.name);
 
-    // The full error-capture set the plan requires, not just the four named
+    // The full error-capture set the plan requires, not just the three named
     // in this test's title.
     expect(names).toEqual(
       expect.arrayContaining([
-        "OnUncaughtException",
         "OnUnhandledRejection",
         "ChildProcess",
         "LinkedErrors",
@@ -565,6 +569,23 @@ describe.skipIf(!sentryPackage)("captured event shape against the real @sentry/n
     Sentry.init(options);
   }
 
+  /**
+   * Proves the fix for the finding that this feature could keep a
+   * Sentry-enabled deployment from surviving the known, neutralized
+   * postgres driver crash: `Sentry.init` must add no `uncaughtException`
+   * listener of its own. `postgres-null-socket-guard.ts` is the sole owner
+   * of that event; a second, independent listener from Sentry's default
+   * `OnUncaughtException` integration would still end the process on the
+   * known fault, no matter which listener Node calls first.
+   */
+  it("adds no uncaughtException listener", async () => {
+    const before = process.listenerCount("uncaughtException");
+
+    await initRealSentryForTest(() => {});
+
+    expect(process.listenerCount("uncaughtException")).toBe(before);
+  });
+
   it("a server event captured after a console.error call carries no console breadcrumb", async () => {
     const Sentry = sentryPackage!;
     let captured: Record<string, unknown> | null = null;

server/src/index.ts

@@ -156,9 +156,10 @@ export interface StartedServer {
 }
 
 export async function startServer(): Promise<StartedServer> {
-  // Registered before every other startup step, including Sentry, so this
-  // listener runs first on any later uncaught exception — see
-  // postgres-null-socket-guard.ts for why that order matters.
+  // Registered before every other startup step, so it is the process's
+  // `uncaughtException` handler for the entire startup sequence, not just
+  // once the server is listening — see postgres-null-socket-guard.ts for
+  // how it owns that decision, Sentry reporting included.
   registerPostgresNullSocketGuard();
 
   warnIfUnsupportedNodeVersion(process.versions.node, (message) => logger.warn(message));

server/src/postgres-null-socket-guard.ts

@@ -14,22 +14,27 @@
 // the one known, neutralized fault below — otherwise an unrelated bug would
 // leave the process running in a state nobody chose.
 //
-// This module reports no telemetry event and calls no external
-// error-reporting client. It logs one structured line, with a fixed marker
-// and the error name only — never a query, a query parameter, a connection
-// URL, a host name, or a database name.
+// For the known, neutralized fault, this module reports no telemetry event
+// and calls no external error-reporting client. It logs one structured
+// line, with a fixed marker and the error name only — never a query, a
+// query parameter, a connection URL, a host name, or a database name.
 //
-// Interaction with Sentry: `./sentry.js` registers no `uncaughtException`
-// listener today. `@sentry/node` is an optional peer dependency this
-// repository does not install (`bootstrapSentry` fails its own version
-// check and returns before it calls `Sentry.init`), so Sentry never adds a
-// listener of its own. If an operator later installs `@sentry/node` and
-// sets a Sentry DSN, the default `OnUncaughtException` integration is not
-// among the integrations `sentry.ts` removes, so it would register its own
-// listener too. This module registers first (from the server entry point,
-// before `sentryReady`), so it runs first on every uncaught exception.
+// Interaction with Sentry: this module is the sole owner of the process's
+// `uncaughtException` decision. `sentry.ts` removes the default
+// `OnUncaughtException` integration from `Sentry.init`, so an operator who
+// installs `@sentry/node` and sets a Sentry DSN gets no second,
+// independent `uncaughtException` listener — Node calls every registered
+// listener for an event, not just the first one, so a second listener
+// would still end the process on the known, neutralized fault regardless
+// of listener order. Instead, for every uncaught exception this module
+// does not neutralize, it reports the error to Sentry itself (through
+// `captureException`) and waits for the client to flush (through
+// `shutdownSentry`) before it ends the process — the same sequence
+// `index.ts` uses for a startup failure. A deployment with no Sentry DSN
+// set sees no change: both calls are no-ops.
 
 import { logger } from "./middleware/logger.js";
+import { captureException, shutdownSentry } from "./sentry.js";
 
 const GUARD_ENABLED_ENV_VAR = "POSTGRES_NULL_SOCKET_GUARD_ENABLED";
 const GUARD_MARKER = "postgres_null_socket_write_guard_neutralized";
@@ -75,12 +80,12 @@ export function isGuardEnabled(env: NodeJS.ProcessEnv): boolean {
 
 /**
  * Handles one uncaught exception. The known driver fault logs one
- * structured, searchable line and returns — the process survives. Every
- * other error keeps today's default behavior: log, then end the process.
- * Exported so a test can drive it directly instead of dispatching a real
- * process event.
+ * structured, searchable line and returns — the process survives, and
+ * Sentry hears nothing about it. Every other error keeps today's default
+ * behavior: log, report to Sentry, then end the process. Exported so a
+ * test can drive it directly instead of dispatching a real process event.
  */
-export function handlePostgresNullSocketGuardException(error: unknown): void {
+export async function handlePostgresNullSocketGuardException(error: unknown): Promise<void> {
   if (isPostgresNullSocketWriteCrash(error)) {
     logger.error(
       { marker: GUARD_MARKER, errorName: (error as Error).name },
@@ -91,6 +96,8 @@ export function handlePostgresNullSocketGuardException(error: unknown): void {
 
   const rootError = error instanceof Error ? error : new Error(String(error));
   logger.fatal({ err: rootError }, "uncaught exception; process exiting");
+  captureException(rootError);
+  await shutdownSentry();
   process.exit(1);
 }
 

server/src/sentry.ts

@@ -36,6 +36,19 @@
 // `mode: "strict"`: Sentry still captures the event, then exits the process,
 // so the existing crash-and-restart behavior stays.
 //
+// `OnUncaughtException` is a default integration too, and the initializer
+// removes it. `postgres-null-socket-guard.ts` registers its own
+// `uncaughtException` listener before the server does anything else,
+// including this Sentry gate — the process must survive one known,
+// neutralized driver defect and end for every other uncaught exception. A
+// second, independent listener from this default integration would end the
+// process for that known defect too, regardless of registration order,
+// because Node calls every registered `uncaughtException` listener, not
+// just the first one. So this repository gives one module exclusive
+// ownership of the decision: `postgres-null-socket-guard.ts` calls this
+// module's `captureException` itself for every uncaught exception it does
+// not neutralize, before it ends the process.
+//
 // Before it imports the package, the bootstrap checks the installed
 // `@sentry/node` version against the exact version this manifest's
 // `peerDependencies` declares — the same audited version documented in
@@ -154,7 +167,10 @@ export function buildSentryInitOptions(
           integration.name !== "Console" &&
           integration.name !== "ContextLines" &&
           integration.name !== "Http" &&
-          integration.name !== "OnUnhandledRejection",
+          integration.name !== "OnUnhandledRejection" &&
+          // Removed so this default integration never registers its own
+          // `uncaughtException` listener. See the module comment above.
+          integration.name !== "OnUncaughtException",
       );
       return [
         ...kept,