Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .server-changes/prisma-infrastructure-error-capture.md
Original file line number Diff line number Diff line change
@@ -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.
68 changes: 43 additions & 25 deletions apps/webapp/app/db.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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<R>(
prisma: PrismaClientOrTransaction,
name: string,
Expand All @@ -40,6 +61,22 @@ export async function $transaction<R>(
fnOrName: ((prisma: PrismaTransactionClient) => Promise<R>) | string,
fnOrOptions?: ((prisma: PrismaTransactionClient) => Promise<R>) | PrismaTransactionOptions,
options?: PrismaTransactionOptions
): Promise<R | undefined> {
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<R>(
prisma: PrismaClientOrTransaction,
fnOrName: ((prisma: PrismaTransactionClient) => Promise<R>) | string,
fnOrOptions?: ((prisma: PrismaTransactionClient) => Promise<R>) | PrismaTransactionOptions,
options?: PrismaTransactionOptions
): Promise<R | undefined> {
if (typeof fnOrName === "string") {
return await startActiveSpan(fnOrName, async (span) => {
Expand All @@ -63,34 +100,13 @@ export async function $transaction<R>(

const fn = fnOrOptions as (prisma: PrismaTransactionClient, span: Span) => Promise<R>;

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
);
}
Expand All @@ -116,11 +132,13 @@ function tagDatasource<T extends PrismaClient>(
}) 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() {
Expand Down
3 changes: 2 additions & 1 deletion apps/webapp/app/routes/api.v1.runs.$runParam.replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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 });
Expand Down
3 changes: 2 additions & 1 deletion apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Status code mismatch: 'Internal Server Error' message returned with 400/422 status in several routes

Several routes now return clientSafeErrorMessage(error) which yields "Internal Server Error" for infrastructure errors, but the HTTP status code remains 400 or 422 (client error codes). For example:

  • api.v1.tasks.$taskId.batch.ts:129 returns status 400 with message "Internal Server Error"
  • api.v1.schedules.$scheduleId.ts:58-59 returns status 422 for non-P2025 known Prisma errors including P1001
  • engine.v1.worker-actions...continue.ts:35 returns status 422

The PR description explicitly states "without changing status codes", so this is intentional. However, for transient infrastructure errors (P1001 — DB unreachable), a 4xx status tells SDK clients not to retry, which is suboptimal for what is fundamentally a server-side transient failure. The routes at api.v2.tasks.batch.ts:179 and api.v3.batches.ts:194 correctly use status 500. The pre-existing status code issue in the v1 routes may warrant a follow-up.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}

return json({ error: "Something went wrong" }, { status: 500 });
Expand Down
3 changes: 2 additions & 1 deletion apps/webapp/app/routes/api.v2.tasks.batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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" } }
);
}
Expand Down
3 changes: 2 additions & 1 deletion apps/webapp/app/routes/api.v3.batches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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" } }
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{
Expand Down Expand Up @@ -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 });
Expand Down
101 changes: 100 additions & 1 deletion apps/webapp/app/utils/prismaErrors.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>) => void };

// Prisma connectivity / infrastructure error codes — engine- and
// connection-level failures, not query- or validation-level ones. When the
Expand Down Expand Up @@ -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<symbol, unknown>)[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<T extends PrismaClient>(
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;
}
Comment on lines +136 to +138

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Several other API routes still return raw error.message to clients — not covered by this PR

This PR applies clientSafeErrorMessage to 6 route files but there are other routes that still return raw error.message to API clients in catch blocks, e.g. api.v3.batches.$batchId.items.ts:105, api.v1.token.ts:48, api.v1.deployments.ts:57. These are pre-existing and not part of this PR's scope, but any of them could similarly leak the DB hostname if a Prisma infrastructure error propagates to their catch blocks. A follow-up pass may be warranted to apply clientSafeErrorMessage consistently across all public API routes.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Loading
Loading