diff --git a/.server-changes/prisma-infrastructure-error-capture.md b/.server-changes/prisma-infrastructure-error-capture.md new file mode 100644 index 0000000000..400cc9ddb6 --- /dev/null +++ b/.server-changes/prisma-infrastructure-error-capture.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Log Prisma infrastructure errors (P1xxx) centrally and obfuscate their messages (which carry the DB hostname) on API responses that previously returned the raw message, without changing status codes or headers. diff --git a/apps/webapp/app/db.server.ts b/apps/webapp/app/db.server.ts index 96f6307f57..89f826b436 100644 --- a/apps/webapp/app/db.server.ts +++ b/apps/webapp/app/db.server.ts @@ -12,6 +12,11 @@ import { z } from "zod"; import { env } from "./env.server"; import { logger } from "./services/logger.server"; import { isValidDatabaseUrl } from "./utils/db"; +import { + captureInfrastructureErrors, + infraErrorAlreadyLogged, + logTransactionInfrastructureError, +} from "./utils/prismaErrors"; import { singleton } from "./utils/singleton"; import { DATASOURCE_CONTEXT_KEY, startActiveSpan } from "./v3/tracer.server"; import { context, Span, trace } from "@opentelemetry/api"; @@ -24,6 +29,22 @@ export type { PrismaReplicaClient, }; +// Boundary logger for transac(): skips an error the client extension already +// logged (and tagged) at the statement level, so a single failure is logged +// once. Shared by both $transaction overloads so the guard can't drift. +function logTransactionPrismaError(error: Prisma.PrismaClientKnownRequestError) { + if (infraErrorAlreadyLogged(error)) { + return; + } + logger.error("prisma.$transaction error", { + code: error.code, + meta: error.meta, + stack: error.stack, + message: error.message, + name: error.name, + }); +} + export async function $transaction( prisma: PrismaClientOrTransaction, name: string, @@ -40,6 +61,22 @@ export async function $transaction( fnOrName: ((prisma: PrismaTransactionClient) => Promise) | string, fnOrOptions?: ((prisma: PrismaTransactionClient) => Promise) | PrismaTransactionOptions, options?: PrismaTransactionOptions +): Promise { + try { + return await $transactionInner(prisma, fnOrName, fnOrOptions, options); + } catch (error) { + // transac()'s callback only logs coded Prisma errors; infra errors such as + // PrismaClientInitializationError reach the boundary without a `.code`. + logTransactionInfrastructureError(error); + throw error; + } +} + +async function $transactionInner( + prisma: PrismaClientOrTransaction, + fnOrName: ((prisma: PrismaTransactionClient) => Promise) | string, + fnOrOptions?: ((prisma: PrismaTransactionClient) => Promise) | PrismaTransactionOptions, + options?: PrismaTransactionOptions ): Promise { if (typeof fnOrName === "string") { return await startActiveSpan(fnOrName, async (span) => { @@ -63,34 +100,13 @@ export async function $transaction( const fn = fnOrOptions as (prisma: PrismaTransactionClient, span: Span) => Promise; - return transac( - prisma, - (client) => fn(client, span), - (error) => { - logger.error("prisma.$transaction error", { - code: error.code, - meta: error.meta, - stack: error.stack, - message: error.message, - name: error.name, - }); - }, - options - ); + return transac(prisma, (client) => fn(client, span), logTransactionPrismaError, options); }); } else { return transac( prisma, fnOrName, - (error) => { - logger.error("prisma.$transaction error", { - code: error.code, - meta: error.meta, - stack: error.stack, - message: error.message, - name: error.name, - }); - }, + logTransactionPrismaError, typeof fnOrOptions === "function" ? undefined : fnOrOptions ); } @@ -116,11 +132,13 @@ function tagDatasource( }) as unknown as T; } -export const prisma = singleton("prisma", () => tagDatasource("writer", getClient())); +export const prisma = singleton("prisma", () => + captureInfrastructureErrors(tagDatasource("writer", getClient())) +); export const $replica: PrismaReplicaClient = singleton("replica", () => { const replica = getReplicaClient(); - return replica ? tagDatasource("replica", replica) : prisma; + return replica ? captureInfrastructureErrors(tagDatasource("replica", replica)) : prisma; }); function getClient() { diff --git a/apps/webapp/app/routes/api.v1.runs.$runParam.replay.ts b/apps/webapp/app/routes/api.v1.runs.$runParam.replay.ts index 4bb5922997..130f6ff163 100644 --- a/apps/webapp/app/routes/api.v1.runs.$runParam.replay.ts +++ b/apps/webapp/app/routes/api.v1.runs.$runParam.replay.ts @@ -8,6 +8,7 @@ import { logger } from "~/services/logger.server"; import { ReplayTaskRunService } from "~/v3/services/replayTaskRun.server"; import { findRunByIdWithMollifierFallback } from "~/v3/mollifier/readFallback.server"; import { sanitizeTriggerSource } from "~/utils/triggerSource"; +import { clientSafeErrorMessage } from "~/utils/prismaErrors"; const ParamsSchema = z.object({ /* This is the run friendly ID */ @@ -145,7 +146,7 @@ export async function action({ request, params }: ActionFunctionArgs) { }, run: runParam, }); - return json({ error: error.message }, { status: 400 }); + return json({ error: clientSafeErrorMessage(error) }, { status: 400 }); } else { logger.error("Failed to replay run", { error: JSON.stringify(error), run: runParam }); return json({ error: JSON.stringify(error) }, { status: 400 }); diff --git a/apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts b/apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts index e76f65e6e8..8f19194a5d 100644 --- a/apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts +++ b/apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts @@ -3,6 +3,7 @@ import { json } from "@remix-run/server-runtime"; import { ScheduleObject, UpdateScheduleOptions } from "@trigger.dev/core/v3"; import { z } from "zod"; import { Prisma, prisma } from "~/db.server"; +import { clientSafeErrorMessage } from "~/utils/prismaErrors"; import { scheduleUniqWhereClause } from "~/models/schedules.server"; import { ViewSchedulePresenter } from "~/presenters/v3/ViewSchedulePresenter.server"; import { authenticateApiRequest } from "~/services/apiAuth.server"; @@ -54,7 +55,7 @@ export async function action({ request, params }: ActionFunctionArgs) { // Check if it's a Prisma error if (error instanceof Prisma.PrismaClientKnownRequestError) { return json( - { error: error.code === "P2025" ? "Schedule not found" : error.message }, + { error: error.code === "P2025" ? "Schedule not found" : clientSafeErrorMessage(error) }, { status: error.code === "P2025" ? 404 : 422 } ); } else { diff --git a/apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts b/apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts index 2e8c5e9749..2ecf29f7c8 100644 --- a/apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts +++ b/apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts @@ -4,6 +4,7 @@ import { BatchTriggerTaskRequestBody, BatchTriggerTaskV2RequestBody } from "@tri import { z } from "zod"; import { fromZodError } from "zod-validation-error"; import { MAX_BATCH_TRIGGER_ITEMS } from "~/consts"; +import { clientSafeErrorMessage } from "~/utils/prismaErrors"; import { env } from "~/env.server"; import { authenticateApiRequest } from "~/services/apiAuth.server"; import { logger } from "~/services/logger.server"; @@ -125,7 +126,7 @@ export async function action({ request, params }: ActionFunctionArgs) { ); } catch (error) { if (error instanceof Error) { - return json({ error: error.message }, { status: 400 }); + return json({ error: clientSafeErrorMessage(error) }, { status: 400 }); } return json({ error: "Something went wrong" }, { status: 500 }); diff --git a/apps/webapp/app/routes/api.v1.token.ts b/apps/webapp/app/routes/api.v1.token.ts index 1122c88cbe..a210b52bfc 100644 --- a/apps/webapp/app/routes/api.v1.token.ts +++ b/apps/webapp/app/routes/api.v1.token.ts @@ -7,6 +7,7 @@ import { import { generateErrorMessage } from "zod-error"; import { logger } from "~/services/logger.server"; import { getPersonalAccessTokenFromAuthorizationCode } from "~/services/personalAccessToken.server"; +import { clientSafeErrorMessage } from "~/utils/prismaErrors"; export async function action({ request }: ActionFunctionArgs) { logger.info("Getting PersonalAccessToken from AuthorizationCode", { url: request.url }); @@ -45,7 +46,7 @@ export async function action({ request }: ActionFunctionArgs) { logger.error("Error getting PersonalAccessToken from AuthorizationCode", fields); } - return json({ error: error.message }, { status: 400 }); + return json({ error: clientSafeErrorMessage(error) }, { status: 400 }); } return json({ error: "Something went wrong" }, { status: 400 }); diff --git a/apps/webapp/app/routes/api.v2.tasks.batch.ts b/apps/webapp/app/routes/api.v2.tasks.batch.ts index 8b2be6e3ca..974b1ed296 100644 --- a/apps/webapp/app/routes/api.v2.tasks.batch.ts +++ b/apps/webapp/app/routes/api.v2.tasks.batch.ts @@ -21,6 +21,7 @@ import { ServiceValidationError } from "~/v3/services/baseService.server"; import { BatchProcessingStrategy } from "~/v3/services/batchTriggerV3.server"; import { OutOfEntitlementError } from "~/v3/services/triggerTask.server"; import { sanitizeTriggerSource } from "~/utils/triggerSource"; +import { clientSafeErrorMessage } from "~/utils/prismaErrors"; import { HeadersSchema } from "./api.v1.tasks.$taskId.trigger"; import { determineRealtimeStreamsVersion } from "~/services/realtime/v1StreamsGlobal.server"; import { extractJwtSigningSecretKey } from "~/services/realtime/jwtAuth.server"; @@ -175,7 +176,7 @@ const { action, loader } = createActionApiRoute( if (error instanceof Error) { return json( - { error: error.message }, + { error: clientSafeErrorMessage(error) }, { status: 500, headers: { "x-should-retry": "false" } } ); } diff --git a/apps/webapp/app/routes/api.v3.batches.ts b/apps/webapp/app/routes/api.v3.batches.ts index f422710676..8787ae4701 100644 --- a/apps/webapp/app/routes/api.v3.batches.ts +++ b/apps/webapp/app/routes/api.v3.batches.ts @@ -14,6 +14,7 @@ import { import { ServiceValidationError } from "~/v3/services/baseService.server"; import { OutOfEntitlementError } from "~/v3/services/triggerTask.server"; import { sanitizeTriggerSource } from "~/utils/triggerSource"; +import { clientSafeErrorMessage } from "~/utils/prismaErrors"; import { HeadersSchema } from "./api.v1.tasks.$taskId.trigger"; import { determineRealtimeStreamsVersion } from "~/services/realtime/v1StreamsGlobal.server"; import { extractJwtSigningSecretKey } from "~/services/realtime/jwtAuth.server"; @@ -190,7 +191,7 @@ const { action, loader } = createActionApiRoute( if (error instanceof Error) { return json( - { error: error.message }, + { error: clientSafeErrorMessage(error) }, { status: 500, headers: { "x-should-retry": "false" } } ); } diff --git a/apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.continue.ts b/apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.continue.ts index bdeb76ca8a..2feb5bf263 100644 --- a/apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.continue.ts +++ b/apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.continue.ts @@ -3,6 +3,7 @@ import { WorkerApiContinueRunExecutionRequestBody } from "@trigger.dev/core/v3/w import { z } from "zod"; import { logger } from "~/services/logger.server"; import { createLoaderWorkerApiRoute } from "~/services/routeBuilders/apiBuilder.server"; +import { clientSafeErrorMessage } from "~/utils/prismaErrors"; export const loader = createLoaderWorkerApiRoute( { @@ -31,7 +32,7 @@ export const loader = createLoaderWorkerApiRoute( } catch (error) { logger.warn("Failed to suspend run", { runFriendlyId, snapshotFriendlyId, error }); if (error instanceof Error) { - throw json({ error: error.message }, { status: 422 }); + throw json({ error: clientSafeErrorMessage(error) }, { status: 422 }); } throw json({ error: "Failed to continue run execution" }, { status: 422 }); diff --git a/apps/webapp/app/utils/prismaErrors.ts b/apps/webapp/app/utils/prismaErrors.ts index b8262ed3d1..ddbdb3cef6 100644 --- a/apps/webapp/app/utils/prismaErrors.ts +++ b/apps/webapp/app/utils/prismaErrors.ts @@ -1,4 +1,9 @@ -import { Prisma } from "@trigger.dev/database"; +import { Prisma, type PrismaClient, isPrismaKnownError } from "@trigger.dev/database"; +import { logger } from "~/services/logger.server"; + +// Minimal structural logger so this stays decoupled from the concrete Logger +// (and lets tests pass a capturing logger). +type ErrorLogger = { error: (message: string, fields?: Record) => void }; // Prisma connectivity / infrastructure error codes — engine- and // connection-level failures, not query- or validation-level ones. When the @@ -37,3 +42,97 @@ export function isInfrastructureError(error: unknown): boolean { return false; } + +// One-shot marker so a single infra error is logged exactly once: the client +// extension (statement level) tags it, and the $transaction-boundary loggers +// skip a tagged error rather than logging the same failure a second time. +const INFRA_ERROR_LOGGED: unique symbol = Symbol("prismaInfraErrorLogged"); + +function markInfraErrorLogged(error: unknown): void { + if (typeof error !== "object" || error === null) { + return; + } + try { + // Non-enumerable so error-spreads/serializers can't copy the marker onto a + // different error; try/catch so a frozen error object can't make this throw + // and mask the original error as it propagates out of the catch. + Object.defineProperty(error, INFRA_ERROR_LOGGED, { + value: true, + enumerable: false, + configurable: true, + writable: true, + }); + } catch { + // best-effort: a sealed/frozen error simply won't be deduped. + } +} + +export function infraErrorAlreadyLogged(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + (error as Record)[INFRA_ERROR_LOGGED] === true + ); +} + +// Logs infrastructure failures (P1xxx-class, see isInfrastructureError) and +// rethrows the ORIGINAL error: callers branch on error.code, and this fires +// per-statement inside transactions, so converting it would break that. +export function captureInfrastructureErrors( + client: T, + log: ErrorLogger = logger +): T { + return client.$extends({ + name: "infrastructure-error-capture", + query: { + $allOperations: async ({ model, operation, args, query }) => { + try { + return await query(args); + } catch (error) { + if (isInfrastructureError(error)) { + log.error("prisma infrastructure error", { + model, + operation, + code: error instanceof Prisma.PrismaClientKnownRequestError ? error.code : undefined, + meta: error instanceof Prisma.PrismaClientKnownRequestError ? error.meta : undefined, + message: error instanceof Error ? error.message : String(error), + stack: error instanceof Error ? error.stack : undefined, + }); + markInfraErrorLogged(error); + } + + throw error; + } + }, + }, + }) as unknown as T; +} + +// Logs infrastructure errors that reach the $transaction boundary WITHOUT a +// Prisma error code (e.g. PrismaClientInitializationError). Coded errors there +// are already logged by transac()'s callback, and errors that bubbled up from a +// statement were already logged (and tagged) by the client extension — both are +// skipped here to avoid double-logging. Returns whether it logged. +export function logTransactionInfrastructureError( + error: unknown, + log: ErrorLogger = logger +): boolean { + if (!isInfrastructureError(error) || isPrismaKnownError(error) || infraErrorAlreadyLogged(error)) { + return false; + } + + log.error("prisma.$transaction infrastructure error", { + message: error instanceof Error ? error.message : String(error), + name: error instanceof Error ? error.name : undefined, + stack: error instanceof Error ? error.stack : undefined, + }); + + return true; +} + +// Replaces a Prisma infrastructure error's message (which carries the DB +// hostname) with a generic one before it reaches an API client. Any other +// error's message is returned unchanged. Status codes/headers are unaffected. +export function clientSafeErrorMessage(error: Error): string { + return isInfrastructureError(error) ? "Internal Server Error" : error.message; +} diff --git a/apps/webapp/test/prismaInfrastructureErrorCapture.test.ts b/apps/webapp/test/prismaInfrastructureErrorCapture.test.ts new file mode 100644 index 0000000000..63170c9e28 --- /dev/null +++ b/apps/webapp/test/prismaInfrastructureErrorCapture.test.ts @@ -0,0 +1,213 @@ +import { describe, expect, it, vi } from "vitest"; +import { postgresTest } from "@internal/testcontainers"; +import { Prisma, PrismaClient } from "@trigger.dev/database"; +import { + captureInfrastructureErrors, + clientSafeErrorMessage, + infraErrorAlreadyLogged, + logTransactionInfrastructureError, +} from "~/utils/prismaErrors"; + +vi.setConfig({ testTimeout: 60_000 }); + +function capturingLogger() { + const captured: Array<{ message: string; fields?: Record }> = []; + return { + captured, + error: (message: string, fields?: Record) => { + captured.push({ message, fields }); + }, + }; +} + +describe("captureInfrastructureErrors", () => { + postgresTest("P2025 (not found) passes through with code intact and unlogged", async ({ + prisma, + }) => { + const log = capturingLogger(); + const client = captureInfrastructureErrors(prisma, log); + + const error = await client.secretStore + .update({ where: { key: "does-not-exist" }, data: { version: "2" } }) + .then(() => undefined) + .catch((e) => e); + + expect(error).toBeInstanceOf(Prisma.PrismaClientKnownRequestError); + expect((error as Prisma.PrismaClientKnownRequestError).code).toBe("P2025"); + expect(log.captured).toHaveLength(0); + }); + + postgresTest("P2002 (unique violation) passes through with code intact and unlogged", async ({ + prisma, + }) => { + const log = capturingLogger(); + const client = captureInfrastructureErrors(prisma, log); + + await client.secretStore.create({ data: { key: "dup-key", value: { a: 1 } } }); + + const error = await client.secretStore + .create({ data: { key: "dup-key", value: { a: 2 } } }) + .then(() => undefined) + .catch((e) => e); + + expect(error).toBeInstanceOf(Prisma.PrismaClientKnownRequestError); + expect((error as Prisma.PrismaClientKnownRequestError).code).toBe("P2002"); + expect(log.captured).toHaveLength(0); + }); + + postgresTest("errors raised inside an interactive $transaction keep their code", async ({ + prisma, + }) => { + const log = capturingLogger(); + const client = captureInfrastructureErrors(prisma, log); + + // Proves $allOperations fires per-statement inside a transaction — the + // basis for transaction retry logic (which branches on error.code) staying + // intact. + const error = await client + .$transaction(async (tx) => { + await tx.secretStore.update({ where: { key: "missing-in-tx" }, data: { version: "2" } }); + }) + .then(() => undefined) + .catch((e) => e); + + expect(error).toBeInstanceOf(Prisma.PrismaClientKnownRequestError); + expect((error as Prisma.PrismaClientKnownRequestError).code).toBe("P2025"); + expect(log.captured).toHaveLength(0); + }); + + postgresTest("raw queries (model undefined) are wrapped without crashing", async ({ + prisma, + }) => { + const log = capturingLogger(); + const client = captureInfrastructureErrors(prisma, log); + + const rows = await client.$queryRaw>(Prisma.sql`SELECT 1 as one`); + expect(rows[0].one).toBe(1); + + // A failing raw query (non-infra) must still rethrow rather than throw on + // the undefined `model`. + const error = await client + .$queryRaw(Prisma.sql`SELECT 1 / 0`) + .then(() => undefined) + .catch((e) => e); + + expect(error).toBeInstanceOf(Error); + expect(log.captured).toHaveLength(0); + }); + + postgresTest("a genuine connectivity failure is logged with model/operation/code", async () => { + const log = capturingLogger(); + // Point at a closed port to provoke a real P1001 / initialization error — + // no mocking. + const unreachable = new PrismaClient({ + datasources: { + db: { url: "postgresql://postgres:postgres@127.0.0.1:1/postgres?connect_timeout=2" }, + }, + }); + const client = captureInfrastructureErrors(unreachable, log); + + try { + const error = await client.secretStore + .findFirst({ where: { key: "anything" } }) + .then(() => undefined) + .catch((e) => e); + + expect(error).toBeInstanceOf(Error); + expect(log.captured).toHaveLength(1); + expect(log.captured[0].message).toBe("prisma infrastructure error"); + expect(log.captured[0].fields?.operation).toBe("findFirst"); + expect(log.captured[0].fields?.model).toBe("SecretStore"); + + // Dedupe: the extension tagged it, so a $transaction-boundary logger + // seeing the same error must NOT log it a second time. + expect(infraErrorAlreadyLogged(error)).toBe(true); + const boundaryLog = capturingLogger(); + expect(logTransactionInfrastructureError(error, boundaryLog)).toBe(false); + expect(boundaryLog.captured).toHaveLength(0); + } finally { + await unreachable.$disconnect(); + } + }); +}); + +describe("logTransactionInfrastructureError", () => { + // Covers the transaction boundary, which $allOperations cannot reach. + it("logs an uncoded infra error (PrismaClientInitializationError)", () => { + const log = capturingLogger(); + const error = new Prisma.PrismaClientInitializationError( + "Can't reach database server", + "6.14.0", + "P1001" + ); + + expect(logTransactionInfrastructureError(error, log)).toBe(true); + expect(log.captured).toHaveLength(1); + expect(log.captured[0].message).toBe("prisma.$transaction infrastructure error"); + expect(log.captured[0].fields?.name).toBe("PrismaClientInitializationError"); + }); + + it("skips a coded infra error (transac's callback already logs those)", () => { + const log = capturingLogger(); + const error = new Prisma.PrismaClientKnownRequestError("Can't reach database server", { + code: "P1001", + clientVersion: "6.14.0", + }); + + expect(logTransactionInfrastructureError(error, log)).toBe(false); + expect(log.captured).toHaveLength(0); + }); + + it("skips a non-infra coded error (P2002)", () => { + const log = capturingLogger(); + const error = new Prisma.PrismaClientKnownRequestError("Unique constraint failed", { + code: "P2002", + clientVersion: "6.14.0", + }); + + expect(logTransactionInfrastructureError(error, log)).toBe(false); + expect(log.captured).toHaveLength(0); + }); + + it("skips a plain non-Prisma error", () => { + const log = capturingLogger(); + + expect(logTransactionInfrastructureError(new Error("boom"), log)).toBe(false); + expect(log.captured).toHaveLength(0); + }); +}); + +describe("clientSafeErrorMessage", () => { + // Guards the API-route leak: an infra error's message carries the DB hostname. + it("obfuscates a coded infra error (P1001) message", () => { + const error = new Prisma.PrismaClientKnownRequestError( + "Can't reach database server at `db-internal.example.com:5432`", + { code: "P1001", clientVersion: "6.14.0" } + ); + + expect(clientSafeErrorMessage(error)).toBe("Internal Server Error"); + }); + + it("obfuscates an uncoded infra error (PrismaClientInitializationError) message", () => { + const error = new Prisma.PrismaClientInitializationError( + "Can't reach database server at `db-internal.example.com:5432`", + "6.14.0", + "P1001" + ); + + expect(clientSafeErrorMessage(error)).toBe("Internal Server Error"); + }); + + it("leaves non-infra Prisma error (P2002) messages unchanged", () => { + const error = new Prisma.PrismaClientKnownRequestError("Unique constraint failed on email", { + code: "P2002", + clientVersion: "6.14.0", + }); + + expect(clientSafeErrorMessage(error)).toBe("Unique constraint failed on email"); + }); + + it("leaves plain domain/validation error messages unchanged", () => { + expect(clientSafeErrorMessage(new Error("Invalid delay value"))).toBe("Invalid delay value"); + }); +});