mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
fix(server): keep Sentry reporting for uncaught exceptions when the postgres guard is off
Sentry's default `OnUncaughtException` integration was removed unconditionally, but the postgres null-socket write guard only owns the process's `uncaughtException` decision while it is enabled. With POSTGRES_NULL_SOCKET_GUARD_ENABLED=false, no listener took its place, so an unrelated uncaught exception reached no error reporter. `sentry.ts` now reads the same opt-out through a new leaf module, postgres-null-socket-guard-env.ts, and keeps the default OnUncaughtException integration active whenever the guard is disabled. Splitting the opt-out check into its own module avoids a circular import between sentry.ts and postgres-null-socket-guard.ts. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
5 files changed
+185
-68
No files matched your search
+35
-22
@@ -387,11 +387,13 @@ integration list of `@sentry/node@10.71.0` and `@sentry/browser@10.71.0`.
|
||||
- `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.
|
||||
An uncaught exception on the main thread is a server event too. While the
|
||||
PostgreSQL null-socket write guard is on, 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. When an operator turns that guard off, `OnUncaughtException` stays
|
||||
active and reports the exception instead.
|
||||
|
||||
**Server context the kept integrations attach**
|
||||
|
||||
@@ -413,9 +415,10 @@ Sentry itself before the process exits.
|
||||
- `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.
|
||||
- `OnUncaughtException` — removed while the PostgreSQL null-socket write
|
||||
guard is on, so it never registers a second, independent
|
||||
`uncaughtException` listener. Kept active when an operator turns that
|
||||
guard off. See "PostgreSQL Null-Socket Write Guard" below.
|
||||
|
||||
**Browser events this feature adds**
|
||||
|
||||
@@ -496,24 +499,34 @@ This environment variable is the guard's one operator control.
|
||||
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.
|
||||
guard existed. When Sentry error monitoring is also on, Sentry's own
|
||||
default `OnUncaughtException` integration stays active in this case (see
|
||||
"Sentry interaction" below), so the exception still reaches Sentry
|
||||
before the process ends. 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.
|
||||
second, independent `uncaughtException` listener from Sentry while this
|
||||
guard is on. `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. While the guard is on, it 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.
|
||||
|
||||
When an operator turns the guard off
|
||||
(`POSTGRES_NULL_SOCKET_GUARD_ENABLED=false`), no module registers an
|
||||
`uncaughtException` listener on the guard's behalf, so `sentry.ts` keeps
|
||||
Sentry's default `OnUncaughtException` integration active instead. An
|
||||
unrelated uncaught exception then reaches Sentry through that default
|
||||
integration rather than through the guard's own reporting call.
|
||||
|
||||
## Sandbox Startup Trace Spans
|
||||
|
||||
|
||||
@@ -23,9 +23,11 @@ import * as sentryModule from "../sentry.js";
|
||||
const DSN_ENV = "SENTRY_DSN";
|
||||
const FRONTEND_DSN_ENV = "SENTRY_DSN_FRONTEND";
|
||||
const BACKEND_DSN_ENV = "SENTRY_DSN_BACKEND";
|
||||
const GUARD_ENABLED_ENV = "POSTGRES_NULL_SOCKET_GUARD_ENABLED";
|
||||
const originalDsn = process.env[DSN_ENV];
|
||||
const originalFrontendDsn = process.env[FRONTEND_DSN_ENV];
|
||||
const originalBackendDsn = process.env[BACKEND_DSN_ENV];
|
||||
const originalGuardEnabled = process.env[GUARD_ENABLED_ENV];
|
||||
|
||||
async function importFreshSentry() {
|
||||
vi.resetModules();
|
||||
@@ -96,6 +98,7 @@ beforeEach(() => {
|
||||
delete process.env[DSN_ENV];
|
||||
delete process.env[FRONTEND_DSN_ENV];
|
||||
delete process.env[BACKEND_DSN_ENV];
|
||||
delete process.env[GUARD_ENABLED_ENV];
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -105,6 +108,8 @@ afterEach(() => {
|
||||
else process.env[FRONTEND_DSN_ENV] = originalFrontendDsn;
|
||||
if (originalBackendDsn === undefined) delete process.env[BACKEND_DSN_ENV];
|
||||
else process.env[BACKEND_DSN_ENV] = originalBackendDsn;
|
||||
if (originalGuardEnabled === undefined) delete process.env[GUARD_ENABLED_ENV];
|
||||
else process.env[GUARD_ENABLED_ENV] = originalGuardEnabled;
|
||||
vi.restoreAllMocks();
|
||||
vi.doUnmock("@sentry/node");
|
||||
vi.doUnmock("../peer-version-check.js");
|
||||
@@ -422,6 +427,26 @@ describe("buildSentryInitOptions", () => {
|
||||
expect(names).not.toContain("OnUncaughtException");
|
||||
});
|
||||
|
||||
it("keeps OnUncaughtException in the resolved list when the guard does not own it", async () => {
|
||||
const { buildSentryInitOptions } = await importFreshSentry();
|
||||
|
||||
const options = buildSentryInitOptions(
|
||||
"https://public@o0.ingest.sentry.io/1",
|
||||
{
|
||||
httpIntegration: () => ({ name: "Http" }),
|
||||
onUnhandledRejectionIntegration: () => ({ name: "OnUnhandledRejection" }),
|
||||
},
|
||||
false,
|
||||
);
|
||||
const resolved = options.integrations(DEFAULT_INTEGRATION_NAMES.map((name) => ({ name })));
|
||||
|
||||
// The postgres null-socket write guard registers no `uncaughtException`
|
||||
// listener when an operator disables it, so Sentry's own default
|
||||
// integration must stay active — otherwise an unrelated uncaught
|
||||
// exception would reach no error reporter at all.
|
||||
expect(resolved.map((i) => i.name)).toContain("OnUncaughtException");
|
||||
});
|
||||
|
||||
it("the resolved server integration list keeps OnUnhandledRejection, LinkedErrors, and RequestData", async () => {
|
||||
const { buildSentryInitOptions } = await importFreshSentry();
|
||||
|
||||
@@ -480,6 +505,40 @@ describe("with @sentry/node mocked", () => {
|
||||
await shutdownSentry();
|
||||
expect(mocks.close).toHaveBeenCalledWith(5_000);
|
||||
});
|
||||
|
||||
it("removes the default OnUncaughtException integration when the postgres null-socket write guard is on", async () => {
|
||||
process.env[DSN_ENV] = "https://public@o0.ingest.sentry.io/1";
|
||||
const mocks = mockSentryPackage();
|
||||
|
||||
const { sentryReady } = await importFreshSentry();
|
||||
await sentryReady;
|
||||
|
||||
const initOptions = mocks.init.mock.calls[0][0] as {
|
||||
integrations: (defaults: Array<{ name: string }>) => Array<{ name: string }>;
|
||||
};
|
||||
const resolved = initOptions.integrations([{ name: "OnUncaughtException" }]);
|
||||
|
||||
expect(resolved.map((i) => i.name)).not.toContain("OnUncaughtException");
|
||||
});
|
||||
|
||||
it("keeps the default OnUncaughtException integration when an operator disables the guard", async () => {
|
||||
process.env[DSN_ENV] = "https://public@o0.ingest.sentry.io/1";
|
||||
process.env[GUARD_ENABLED_ENV] = "false";
|
||||
const mocks = mockSentryPackage();
|
||||
|
||||
const { sentryReady } = await importFreshSentry();
|
||||
await sentryReady;
|
||||
|
||||
const initOptions = mocks.init.mock.calls[0][0] as {
|
||||
integrations: (defaults: Array<{ name: string }>) => Array<{ name: string }>;
|
||||
};
|
||||
const resolved = initOptions.integrations([{ name: "OnUncaughtException" }]);
|
||||
|
||||
// The guard registers no `uncaughtException` listener when disabled, so
|
||||
// Sentry's own default integration must stay active — otherwise an
|
||||
// unrelated uncaught exception reaches no error reporter at all.
|
||||
expect(resolved.map((i) => i.name)).toContain("OnUncaughtException");
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
// The `POSTGRES_NULL_SOCKET_GUARD_ENABLED` env-var contract, split into its
|
||||
// own leaf module so both `postgres-null-socket-guard.ts` (to decide whether
|
||||
// to register its `uncaughtException` listener) and `sentry.ts` (to decide
|
||||
// whether Sentry's own `OnUncaughtException` integration must stay active)
|
||||
// can read the same value without importing each other.
|
||||
|
||||
const GUARD_ENABLED_ENV_VAR = "POSTGRES_NULL_SOCKET_GUARD_ENABLED";
|
||||
|
||||
/**
|
||||
* Reads the runtime opt-out. An operator sets `false` or `0` to disable the
|
||||
* guard; every other value, including an unset variable, keeps it enabled.
|
||||
* Follows the same boolean convention as `envBoolean` in
|
||||
* `packages/db/src/client.ts`: a value other than a recognized true/false
|
||||
* spelling is a configuration mistake, so this throws instead of guessing.
|
||||
*/
|
||||
export function isGuardEnabled(env: NodeJS.ProcessEnv): boolean {
|
||||
const value = env[GUARD_ENABLED_ENV_VAR]?.trim().toLowerCase();
|
||||
if (value === undefined || value === "") return true;
|
||||
if (value === "true" || value === "1") return true;
|
||||
if (value === "false" || value === "0") return false;
|
||||
throw new Error(`${GUARD_ENABLED_ENV_VAR} must be "true" or "false", got: ${env[GUARD_ENABLED_ENV_VAR]}`);
|
||||
}
|
||||
@@ -19,24 +19,32 @@
|
||||
// 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: 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
|
||||
// Interaction with Sentry: while this guard is enabled, it is the sole
|
||||
// owner of the process's `uncaughtException` decision. `sentry.ts` reads
|
||||
// the same opt-out this module reads (`postgres-null-socket-guard-env.ts`)
|
||||
// and removes the default `OnUncaughtException` integration from
|
||||
// `Sentry.init` only in that case, 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.
|
||||
//
|
||||
// An operator who disables this guard
|
||||
// (`POSTGRES_NULL_SOCKET_GUARD_ENABLED=false`) registers no listener here,
|
||||
// so `sentry.ts` keeps the default `OnUncaughtException` integration active
|
||||
// instead — an unrelated uncaught exception still reaches Sentry through
|
||||
// that integration rather than Node's silent default handler.
|
||||
|
||||
import { logger } from "./middleware/logger.js";
|
||||
import { captureException, shutdownSentry } from "./sentry.js";
|
||||
import { isGuardEnabled } from "./postgres-null-socket-guard-env.js";
|
||||
|
||||
const GUARD_ENABLED_ENV_VAR = "POSTGRES_NULL_SOCKET_GUARD_ENABLED";
|
||||
const GUARD_MARKER = "postgres_null_socket_write_guard_neutralized";
|
||||
|
||||
// Both V8 message spellings for a read of a property on `null`. The wording
|
||||
@@ -63,20 +71,10 @@ export function isPostgresNullSocketWriteCrash(error: unknown): boolean {
|
||||
return stack.split("\n").some((line) => NEXT_WRITE_FRAME_PATTERN.test(line));
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads the runtime opt-out. An operator sets `false` or `0` to disable the
|
||||
* guard; every other value, including an unset variable, keeps it enabled.
|
||||
* Follows the same boolean convention as `envBoolean` in
|
||||
* `packages/db/src/client.ts`: a value other than a recognized true/false
|
||||
* spelling is a configuration mistake, so this throws instead of guessing.
|
||||
*/
|
||||
export function isGuardEnabled(env: NodeJS.ProcessEnv): boolean {
|
||||
const value = env[GUARD_ENABLED_ENV_VAR]?.trim().toLowerCase();
|
||||
if (value === undefined || value === "") return true;
|
||||
if (value === "true" || value === "1") return true;
|
||||
if (value === "false" || value === "0") return false;
|
||||
throw new Error(`${GUARD_ENABLED_ENV_VAR} must be "true" or "false", got: ${env[GUARD_ENABLED_ENV_VAR]}`);
|
||||
}
|
||||
// Re-exported so existing callers (including tests) can keep importing the
|
||||
// opt-out check from this module. See postgres-null-socket-guard-env.ts for
|
||||
// why the check itself lives in a separate leaf module.
|
||||
export { isGuardEnabled };
|
||||
|
||||
/**
|
||||
* Handles one uncaught exception. The known driver fault logs one
|
||||
|
||||
+47
-22
@@ -37,17 +37,26 @@
|
||||
// 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.
|
||||
// removes it — but only while `postgres-null-socket-guard.ts` registers its
|
||||
// own `uncaughtException` listener (the default: see
|
||||
// `postgres-null-socket-guard-env.ts` for the shared opt-out this module
|
||||
// reads). That guard runs before the server does anything else, including
|
||||
// this Sentry gate, and 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 while the guard is enabled, 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.
|
||||
//
|
||||
// When an operator disables that guard
|
||||
// (`POSTGRES_NULL_SOCKET_GUARD_ENABLED=false`), no module registers an
|
||||
// `uncaughtException` listener on its behalf, so this initializer keeps the
|
||||
// default `OnUncaughtException` integration active instead. Without that,
|
||||
// an unrelated uncaught exception would reach no error reporter at all —
|
||||
// Node's default handler exits the process without telling Sentry.
|
||||
//
|
||||
// Before it imports the package, the bootstrap checks the installed
|
||||
// `@sentry/node` version against the exact version this manifest's
|
||||
@@ -59,6 +68,7 @@
|
||||
|
||||
import { checkExactPeerVersions } from "./peer-version-check.js";
|
||||
import { resolveSentryDsns } from "./sentry-dsn.js";
|
||||
import { isGuardEnabled } from "./postgres-null-socket-guard-env.js";
|
||||
|
||||
const { backend: dsn, legacyFallbackUsed } = resolveSentryDsns();
|
||||
|
||||
@@ -151,10 +161,19 @@ export interface SentryInitOptions {
|
||||
* `bootstrapSentry` so a test can call it with a real `@sentry/node` module
|
||||
* and assert the resolved integration list and the captured-event shape
|
||||
* against the true SDK, not a stand-in.
|
||||
*
|
||||
* `guardOwnsUncaughtException` defaults to `true` — the common case, where
|
||||
* `postgres-null-socket-guard.ts` registers its own listener and this
|
||||
* initializer must remove the default `OnUncaughtException` integration so
|
||||
* Node does not call two independent listeners for the same event. Pass
|
||||
* `false` only when that guard is disabled
|
||||
* (`POSTGRES_NULL_SOCKET_GUARD_ENABLED=false`), so the default integration
|
||||
* stays active and an unrelated uncaught exception still reaches Sentry.
|
||||
*/
|
||||
export function buildSentryInitOptions(
|
||||
dsn: string,
|
||||
Sentry: SentryModuleLike,
|
||||
guardOwnsUncaughtException: boolean = true,
|
||||
): SentryInitOptions {
|
||||
return {
|
||||
dsn,
|
||||
@@ -162,16 +181,16 @@ export function buildSentryInitOptions(
|
||||
tracesSampleRate: 0,
|
||||
sendDefaultPii: false,
|
||||
integrations: (defaults: Array<{ name: string }>) => {
|
||||
const kept = defaults.filter(
|
||||
(integration) =>
|
||||
integration.name !== "Console" &&
|
||||
integration.name !== "ContextLines" &&
|
||||
integration.name !== "Http" &&
|
||||
integration.name !== "OnUnhandledRejection" &&
|
||||
// Removed so this default integration never registers its own
|
||||
// `uncaughtException` listener. See the module comment above.
|
||||
integration.name !== "OnUncaughtException",
|
||||
);
|
||||
const kept = defaults.filter((integration) => {
|
||||
if (integration.name === "Console") return false;
|
||||
if (integration.name === "ContextLines") return false;
|
||||
if (integration.name === "Http") return false;
|
||||
if (integration.name === "OnUnhandledRejection") return false;
|
||||
// Removed only while `postgres-null-socket-guard.ts` owns the
|
||||
// `uncaughtException` decision. See the module comment above.
|
||||
if (integration.name === "OnUncaughtException") return !guardOwnsUncaughtException;
|
||||
return true;
|
||||
});
|
||||
return [
|
||||
...kept,
|
||||
// Keep the rest of the Http integration — RequestData and request
|
||||
@@ -210,7 +229,13 @@ async function bootstrapSentry(dsn: string): Promise<void> {
|
||||
// @ts-ignore optional peer dep
|
||||
const Sentry = await import("@sentry/node");
|
||||
|
||||
Sentry.init(buildSentryInitOptions(dsn, Sentry));
|
||||
// Reads the same opt-out `postgres-null-socket-guard.ts` reads, so this
|
||||
// module strips the default `OnUncaughtException` integration only when
|
||||
// that guard actually owns the process's `uncaughtException` decision.
|
||||
// `registerPostgresNullSocketGuard()` runs before this bootstrap
|
||||
// settles and validates the same variable, so a startup with an
|
||||
// unrecognized value never reaches this line.
|
||||
Sentry.init(buildSentryInitOptions(dsn, Sentry, isGuardEnabled(process.env)));
|
||||
|
||||
sentryHandle = {
|
||||
captureException: (error) => Sentry.captureException(error),
|
||||
|
||||
Reference in new issue
Block a user