feat(webapp,database): opt-in per-client Prisma driver adapters - #4539
feat(webapp,database): opt-in per-client Prisma driver adapters#4539ericallam wants to merge 2 commits into
Conversation
Add per-client env vars to route each Prisma client through @prisma/adapter-pg (node-postgres) instead of the built-in engine driver. All default off, so behavior is unchanged unless a flag is set: - CONTROL_PLANE_DATABASE_WRITER_DRIVER_ADAPTER - CONTROL_PLANE_DATABASE_REPLICA_DRIVER_ADAPTER - RUN_OPS_DATABASE_WRITER_DRIVER_ADAPTER - RUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTER - RUN_OPS_LEGACY_DATABASE_WRITER_DRIVER_ADAPTER - RUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTER Enables the driverAdapters preview feature on both schemas (keeps the Rust query engine; does not add queryCompiler). Each adapter pool is built with a bounded connectionTimeoutMillis and an onPoolError handler. Handle the connect-failure differences the adapter introduces: - isInfrastructureError now recognizes the adapter's connect-failure shapes (P2010 'not reachable' and raw ECONNREFUSED/ENOTFOUND-class errors) so the DB host is still scrubbed from API-client errors and infra failures are logged. - isPrismaRetriableError treats the adapter pool-acquire timeout as retriable, preserving the P2024 retry behavior. refs TRI-13039 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
WalkthroughThe web application adds optional PostgreSQL driver-adapter support for control-plane, run-ops, and legacy run-ops Prisma writers and replicas. Environment flags independently enable each adapter. Adapter-backed clients use configured PostgreSQL pools. Default behavior continues to use datasource URLs. Prisma generators and package dependencies are updated. Connectivity detection now handles adapter timeout messages, network errors, and connectivity-related 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Pass the per-client resolved connection limit into the adapter pool instead of always using DATABASE_CONNECTION_LIMIT, so per-client overrides (e.g. RUN_OPS_DATABASE_READ_REPLICA_CONNECTION_LIMIT) are honored on the adapter path. - Build the adapter pool from the base DSN (drops prisma-only URL params the pg driver ignores and the duplicate application_name). - Scope the connectivity message match to 'database not reachable' so a generic 'not reachable' error is no longer misclassified as infrastructure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const pool = new Pool({ | ||
| connectionString, | ||
| max: connectionLimit, | ||
| connectionTimeoutMillis: poolTimeoutSeconds * 1000, | ||
| application_name: env.SERVICE_NAME, | ||
| }); | ||
| pool.on("error", (error) => { | ||
| logger.error("prisma driver adapter pool error", { | ||
| clientType, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| ignoreError: true, | ||
| }); | ||
| }); | ||
| return new PrismaPg(pool); |
There was a problem hiding this comment.
🔴 Turning on the new database connection driver makes the app read and write the wrong database schema for installs that use a custom schema
The database schema named in the connection URL is dropped when the new connection driver is built (new Pool({ connectionString }) at apps/webapp/app/db.server.ts:446-451), so an installation configured with a non-default schema silently talks to the default one instead.
Impact: Operators who enable the new driver on a deployment whose connection string names a custom schema get "table does not exist" failures or, worse, reads/writes against an unrelated schema.
Mechanism: `?schema=` is a Prisma-only DSN parameter that node-postgres ignores, and the adapter's `schema` option is never set
With the Rust engine path the DSN is handed to Prisma (datasources: { db: { url } }), and Prisma uses the schema query parameter to set the connection's search_path. The repo relies on this: getDatabaseSchema() (apps/webapp/app/db.server.ts:973-987) reads ?schema= and defaults to public, and .env.example / docs/self-hosting/kubernetes.mdx:262 ship URLs with ?schema=....
On the adapter path the URL is passed straight to pg.Pool. node-postgres only forwards a fixed set of startup parameters (user, database, application_name, options, ...) — schema is not among them, so it is silently discarded. @prisma/adapter-pg@6.14.0 surfaces the schema to the query engine only via its second constructor argument (getConnectionInfo() returns { schemaName: this.options?.schema }), which this code never passes: return new PrismaPg(pool).
Result: with useDriverAdapter on and a DSN such as ...?schema=trigger, all model queries are emitted unqualified and resolve against the session default (public) rather than trigger. Note the raw-SQL helper sqlDatabaseSchema still uses the configured schema, so raw and generated queries would diverge.
Prompt for agents
buildDriverAdapterPool in apps/webapp/app/db.server.ts builds a pg.Pool from the raw DSN and wraps it with `new PrismaPg(pool)`. The DSN's `?schema=` query parameter is meaningful only to Prisma's Rust engine; node-postgres discards it, and @prisma/adapter-pg only learns the schema from its second constructor argument (`new PrismaPg(pool, { schema })`, surfaced through getConnectionInfo().schemaName). Consequently any deployment whose connection string names a non-public schema will run all Prisma model queries against the default search_path once a driver-adapter flag is turned on. Fix by parsing the `schema` search param out of the connection URL for each client (the same way getDatabaseSchema() does for DATABASE_URL, but per-URL since run-ops/legacy DSNs can differ) and passing it as the adapter option; consider also setting the pool's `options: '-c search_path=...'` so raw queries behave identically. Also worth deciding whether an absent schema param should default to 'public' explicitly.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ignoreError: true, | ||
| }); | ||
| }); | ||
| return new PrismaPg(pool); |
There was a problem hiding this comment.
🔍 External pg pool is never closed when the Prisma client disconnects
new PrismaPg(pool) is constructed without { disposeExternalPool: true }. In @prisma/adapter-pg@6.14.0 the factory only calls pool.end() when it created the pool itself; for an externally supplied pool it merely removes its error listener on release. So client.$disconnect() (or engine reconnects) will leave the node-postgres pool and its sockets open. Only matters for shutdown/reconnect paths and only when a driver-adapter flag is on, but it is a real difference from the Rust-engine path, which releases its connections on disconnect.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const client = useDriverAdapter | ||
| ? new PrismaClient({ | ||
| adapter: buildDriverAdapterPool( | ||
| url, | ||
| clientType, | ||
| poolTimeout ?? env.DATABASE_POOL_TIMEOUT, | ||
| env.DATABASE_CONNECTION_LIMIT | ||
| ), | ||
| log: logConfig, | ||
| }) |
There was a problem hiding this comment.
🔍 Prisma $metrics consumers are not guarded for the adapter path
The PR notes that $metrics-based pool observability goes away under the adapter, but the two existing consumers are unguarded: apps/webapp/app/routes/metrics.ts:17 awaits prisma.$metrics.prometheus() with no try/catch (a throw makes the whole /metrics scrape 500, taking core metrics with it), and the OTEL batch observable callback in apps/webapp/app/v3/tracer.server.ts:552-554 awaits prisma.$metrics.json() inside an async callback whose rejection isn't handled. Worth confirming whether $metrics throws or simply returns pool metrics as zero when the client is constructed with a driver adapter; if it throws, these two call sites should be defensively wrapped in this PR rather than the follow-up.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (isPrismaKnownError(error)) { | ||
| return retryCodes.includes(error.code); | ||
| } | ||
|
|
||
| return retryCodes.includes(error.code); | ||
| const message = (error as { message?: unknown })?.message; | ||
| return typeof message === "string" && ADAPTER_ACQUIRE_TIMEOUT.test(message); |
There was a problem hiding this comment.
🔍 Retriable-error fallback only fires when the acquire timeout is not a coded Prisma error
The new message check is placed after an early return for isPrismaKnownError, which duck-types on any string .code. If Prisma wraps the adapter's "timeout exceeded when trying to connect" into a coded error (e.g. P2010 "Raw query failed", which the companion change in prismaErrors.ts explicitly anticipates for adapter connect failures), the first branch returns false and the pool-acquire retry behaviour the PR is trying to preserve never engages. Checking the message before/independently of the coded branch would make the intent hold in both shapes.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
What
Adds an opt-in path to run each Prisma client through
@prisma/adapter-pg(the node-postgres driver) instead of the built-in engine driver, controlled by a per-client env var, all off by default:CONTROL_PLANE_DATABASE_WRITER_DRIVER_ADAPTERCONTROL_PLANE_DATABASE_REPLICA_DRIVER_ADAPTERRUN_OPS_DATABASE_WRITER_DRIVER_ADAPTERRUN_OPS_DATABASE_REPLICA_DRIVER_ADAPTERRUN_OPS_LEGACY_DATABASE_WRITER_DRIVER_ADAPTERRUN_OPS_LEGACY_DATABASE_REPLICA_DRIVER_ADAPTERWith every flag unset the construction path is byte-identical to today (
datasourcesURL + Rust engine), so this is inert until a flag is turned on. Per-client granularity allows enabling the adapter only where it's wanted.How
driverAdapterspreview feature on both schemas (@trigger.dev/databaseand@internal/run-ops-database). This keeps the Rust query engine — it does NOT addqueryCompiler— so query behavior, result types, and engine tracing spans are unchanged.buildDriverAdapterPoolbuilds each client'spg.Poolwith an explicitmax, a boundedconnectionTimeoutMillis(the node-postgres pool otherwise waits unbounded on acquire), and anonPoolErrorhandler (an unhandled idle-connection error would otherwise crash the process). Threaded through all four client builders via auseDriverAdapterflag.@prisma/adapter-pg+@types/pgto the webapp;pgis already pinned at8.15.6(adapter-pg 6.x requirespg < 8.17).Connect-failure handling (the important correctness/security bit)
Under the adapter an unreachable DB no longer surfaces as
PrismaClientInitializationError/P1001; it becomes aP2010"Database not reachable: " (or a rawECONNREFUSED/ENOTFOUND-class error). Two handlers are updated so a client on the adapter behaves like today:isInfrastructureErrornow recognizes those shapes (P2010 with a connectivity message, and raw connectivity errno codes). Without this, the DB hostname would leak into API-client-facing errors and the failure would go unlogged. Security-relevant.isPrismaRetriableErrortreats the adapter's pool-acquire timeout ("timeout exceeded when trying to connect") as retriable, preserving theP2024retry behavior the adapter otherwise drops.Evidence
Validated on an isolated stack that mirrors the production DB topology (chained PgBouncers in front of writer + reader):
metaare byte-identical between the engine driver and the adapter across the queried shapes (unique-constraintmeta.target, record-not-found, transaction-timeout, serialization-failure, etc.).Rollout / rollback
All flags default off; enable per client via env var, roll back by unsetting and redeploying (no data migration). Recommended first target is a single writer; enable one client at a time.
Follow-ups (not in this PR)
$metrics-based pool observability is removed under the adapter (the Prometheus route +db.pool.connections.*instruments); the metrics replacement (viapg.Poolcounters) lands in a separate PR.maxWaitdoes not bound pool acquisition —connectionTimeoutMillisdoes.refs TRI-13039