From bc01f6ea3ab6f2a9bf2bdef401016f17be901d62 Mon Sep 17 00:00:00 2001 From: Daniel Sutton <45313566+d-cs@users.noreply.github.com> Date: Wed, 10 Jun 2026 08:53:57 +0100 Subject: [PATCH] fix(webapp): stop writer DB connectivity errors leaking to trigger() API clients (#3874) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary During `trigger()` worker-queue resolution, `getWorkerQueue` wrapped any error from `getDefaultWorkerGroupForProject` into a client-facing `ServiceValidationError` (HTTP 422) carrying `error.message`. That method runs `project.findFirst` on the **writer**; when the writer is unreachable Prisma throws a connection error (P1001) whose message includes the database host, and that raw message was returned to the API client and surfaced in the run view via the SDK's `TriggerApiError`. It also mis-classifies a transient outage: a 422 is not retried by the SDK, so triggers failed permanently instead of riding out a brief writer blip. ## Design This is the only place on the trigger path that folds a *caught* error's message into a client-facing error — every other DB failure on the path propagates to the route's generic 500 handler (scrubbed, and retried by the SDK). So the fix is local: - Add `isInfrastructureError()` — true for Prisma connection-level failures (the DB-unreachable family: P1001/P1002/P1008/P1017, plus the init/panic/unknown client error classes), false for query/validation errors (e.g. P2002). - At the wrap site, rethrow infrastructure errors so they reach the generic 500 handler (no raw message, and retryable). Genuine domain failures (e.g. "Project not found.") still become a 422. Only P1001 ("can't reach database server") has been observed in practice; the rest of the connection family is included as same-class forward-proofing. ## Test plan - [x] Unit: `isInfrastructureError` classifies a P1001 (incl. the Prisma 6.x `PrismaClientKnownRequestError` shape) and init errors as infrastructure; P2002 and a plain `Error` as not - [x] `getWorkerQueue` rethrows a P1001 unchanged instead of wrapping it in a `ServiceValidationError`; still wraps a domain failure as a `ServiceValidationError` — RED on current code, GREEN after - [ ] (optional) toxiproxy e2e: trigger with the writer cut → HTTP 500 generic body, no DB host in the response --------- Co-authored-by: Claude Opus 4.8 --- .../trigger-worker-queue-db-error-leak.md | 6 +++ .../app/runEngine/concerns/queues.server.ts | 12 ++++++ apps/webapp/app/utils/prismaErrors.ts | 39 +++++++++++++++++++ apps/webapp/test/prismaErrors.test.ts | 32 +++++++++++++++ 4 files changed, 89 insertions(+) create mode 100644 .server-changes/trigger-worker-queue-db-error-leak.md create mode 100644 apps/webapp/app/utils/prismaErrors.ts create mode 100644 apps/webapp/test/prismaErrors.test.ts diff --git a/.server-changes/trigger-worker-queue-db-error-leak.md b/.server-changes/trigger-worker-queue-db-error-leak.md new file mode 100644 index 000000000..9725ef9f2 --- /dev/null +++ b/.server-changes/trigger-worker-queue-db-error-leak.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Stop `trigger()` from leaking raw database connection errors to API clients during a database outage; infrastructure errors now return a generic, retryable 500. diff --git a/apps/webapp/app/runEngine/concerns/queues.server.ts b/apps/webapp/app/runEngine/concerns/queues.server.ts index 2fc35fc84..dce74d7d1 100644 --- a/apps/webapp/app/runEngine/concerns/queues.server.ts +++ b/apps/webapp/app/runEngine/concerns/queues.server.ts @@ -15,6 +15,7 @@ import type { RunEngine } from "~/v3/runEngine.server"; import { env } from "~/env.server"; import { tryCatch } from "@trigger.dev/core/v3"; import { ServiceValidationError } from "~/v3/services/common.server"; +import { isInfrastructureError } from "~/utils/prismaErrors"; import { createCache, createLRUMemoryStore, DefaultStatefulContext, Namespace } from "@internal/cache"; import { singleton } from "~/utils/singleton"; import type { TaskMetadataCache, TaskMetadataEntry } from "~/services/taskMetadataCache.server"; @@ -394,6 +395,17 @@ export class DefaultQueueManager implements QueueManager { ); if (error) { + // getDefaultWorkerGroupForProject queries the writer DB. A Prisma + // infrastructure error (e.g. P1001 "Can't reach database server", whose + // message carries the DB hostname) must NOT be promoted into a + // client-facing ServiceValidationError: that leaks internal infra detail + // to the API client (the SDK echoes it into the run view) and + // mis-classifies a transient outage as a non-retryable 422. Let it + // propagate to the route's generic 500 handler (scrubbed + retryable); + // only wrap genuine domain failures. + if (isInfrastructureError(error)) { + throw error; + } throw new ServiceValidationError(error.message); } diff --git a/apps/webapp/app/utils/prismaErrors.ts b/apps/webapp/app/utils/prismaErrors.ts new file mode 100644 index 000000000..b8262ed3d --- /dev/null +++ b/apps/webapp/app/utils/prismaErrors.ts @@ -0,0 +1,39 @@ +import { Prisma } from "@trigger.dev/database"; + +// Prisma connectivity / infrastructure error codes — engine- and +// connection-level failures, not query- or validation-level ones. When the +// database is unreachable, Prisma 6.x throws a PrismaClientKnownRequestError +// carrying one of these codes (e.g. P1001 "Can't reach database server"). +const INFRASTRUCTURE_PRISMA_CODES = new Set([ + "P1001", // Can't reach database server + "P1002", // Database server reached but timed out + "P1008", // Operations timed out + "P1017", // Server has closed the connection +]); + +/** + * True when `error` is a Prisma infrastructure/connectivity failure (DB + * unreachable, timed out, connection dropped) rather than a query- or + * validation-level error. + * + * These errors carry internal infrastructure detail (e.g. the database + * hostname) in their `.message`, so they must never be surfaced to API + * clients — callers should let them propagate to the generic 5xx handler + * (which both scrubs the message and is retryable by the SDK) instead of + * folding `.message` into a client-facing error. + */ +export function isInfrastructureError(error: unknown): boolean { + if ( + error instanceof Prisma.PrismaClientInitializationError || + error instanceof Prisma.PrismaClientRustPanicError || + error instanceof Prisma.PrismaClientUnknownRequestError + ) { + return true; + } + + if (error instanceof Prisma.PrismaClientKnownRequestError) { + return INFRASTRUCTURE_PRISMA_CODES.has(error.code); + } + + return false; +} diff --git a/apps/webapp/test/prismaErrors.test.ts b/apps/webapp/test/prismaErrors.test.ts new file mode 100644 index 000000000..48af6c829 --- /dev/null +++ b/apps/webapp/test/prismaErrors.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { Prisma } from "@trigger.dev/database"; +import { isInfrastructureError } from "../app/utils/prismaErrors.js"; + +describe("isInfrastructureError", () => { + it("treats a P1001 'can't reach database server' (KnownRequestError) as infrastructure", () => { + // Prisma 6.x reports P1001 as a PrismaClientKnownRequestError with code P1001 — + // this is the exact production shape that leaked the RDS hostname to a customer. + const err = new Prisma.PrismaClientKnownRequestError( + "Invalid `prisma.project.findFirst()` invocation: Can't reach database server at host:5432", + { code: "P1001", clientVersion: "6.14.0" } + ); + expect(isInfrastructureError(err)).toBe(true); + }); + + it("treats a PrismaClientInitializationError as infrastructure", () => { + const err = new Prisma.PrismaClientInitializationError("init failed", "6.14.0"); + expect(isInfrastructureError(err)).toBe(true); + }); + + it("does NOT treat a query/validation error (P2002 unique constraint) as infrastructure", () => { + const err = new Prisma.PrismaClientKnownRequestError("Unique constraint failed", { + code: "P2002", + clientVersion: "6.14.0", + }); + expect(isInfrastructureError(err)).toBe(false); + }); + + it("does NOT treat a plain domain Error as infrastructure", () => { + expect(isInfrastructureError(new Error("Project not found."))).toBe(false); + }); +});