chore: adopt Prisma 7 beta (tsgo)#3813
Conversation
…t build server failures (triggerdotdev#2913)
- Include reproduction scripts for Sentry (triggerdotdev#2900) and engine strictness (triggerdotdev#2913) - Include PR body drafts for consolidated tracking
- Include reproduction scripts for Sentry (triggerdotdev#2900) and engine strictness (triggerdotdev#2913) - Include PR body drafts for consolidated tracking
When the underlying logical-replication client errored (e.g. after a Postgres failover), the runs and sessions replication services logged the error and left the stream stopped. The host process kept running, the WAL backed up, and ClickHouse silently fell behind. Both services now run a configurable recovery strategy on stream errors, defaulting to in-process reconnect with exponential backoff so a fresh self-hosted setup heals on its own: - "reconnect" (default) re-subscribes via the existing subscribe(lastLsn) path with exponential backoff (1s -> 60s cap, unlimited attempts), which re-validates the publication, re-acquires the leader lock, and resumes from the last acknowledged LSN. - "exit" calls process.exit after a short flush window so a host's supervisor (Docker restart=always, systemd, k8s, etc.) can replace the process. - "log" preserves the historical behaviour. Per-service strategy + exit knobs are env-driven via RUN_REPLICATION_ERROR_STRATEGY / SESSION_REPLICATION_ERROR_STRATEGY plus matching *_EXIT_DELAY_MS / *_EXIT_CODE. Reconnect tuning is shared across both services via REPLICATION_RECONNECT_INITIAL_DELAY_MS / _MAX_DELAY_MS / _MAX_ATTEMPTS (0 = unlimited).
Addresses PR review feedback:
- LogicalReplicationClient.subscribe() can throw before its internal
"error" listener is wired up (notably when pg client.connect() fails
mid-failover). The reconnect strategy's catch block only logged, so
recovery silently stopped. Now also calls scheduleReconnect(err) — the
pendingReconnect guard makes it idempotent if an error event was also
emitted.
- Reject negative values for the new replication-recovery env vars and
cap exit codes at 255.
- Convert the new ReplicationErrorRecovery{Deps,} interfaces to type
aliases to match the repo's TypeScript style.
- Tighten the reconnect dep comment to drop a stale "lastAcknowledgedLsn"
reference (the wrapper-tracked resume LSN is what callers actually pass).
- Restore process.exit after service.shutdown() in the exit-strategy
test so a delayed exit timer can't terminate the test worker.
LogicalReplicationClient.subscribe() can resolve without throwing or emitting an "error" event when leader-lock acquisition fails — it just calls this.stop() and returns. The reconnect callback now checks isStopped after subscribe() and throws so the recovery handler can schedule the next attempt instead of silently giving up.
…rough handle() The previous post-subscribe() isStopped check was always true on the happy path: subscribe() calls stop() up front (setting _isStopped=true) and only resets the flag inside the replicationStart event, which fires asynchronously after subscribe() returns. So the check threw on every successful reconnect, the catch rescheduled, the next attempt tore down the just-built client, and the cycle continued — replication briefly worked between teardowns, which is why the integration test passed. Replace it with the correct nudge: subscribe to leaderElection and call the recovery handler on isLeader=false. That's the only subscribe() exit path that doesn't either throw or emit an "error" event (the other silent-return paths emit "error" first via createPublication/createSlot failures).
The previous commit routed leaderElection(false) through handle(), which under the exit strategy schedules process.exit. In a multi-instance deployment that turns lost leader election — a normal operational state — into a restart loop: exit, supervisor restarts, election fails again, exit, and so on. Add a dedicated notifyLeaderElectionLost() on ReplicationErrorRecovery that the reconnect strategy treats as another retry trigger, while exit and log strategies no-op. Wire the wrapper services through the new method.
fix(webapp): auto-recover replication services after stream errors
…iggerdotdev#3391) - Update schema.prisma for Prisma 7 compatibility - Add prisma.config.ts with driver adapter setup - Update transaction.ts for new Prisma client API - Update docker/entrypoint.sh and Dockerfile for pnpm 10.23.0 - Update package.json across packages for Prisma 7 deps - Update testcontainers, run-engine tests for new Prisma version - Add references/prisma-7/package.json placeholder Closes triggerdotdev#3391
🦋 Changeset detectedLatest commit: 5f4c41a The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi @deepshekhardas, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (47)
WalkthroughThis PR upgrades Prisma from version 6.14.0 to 7.7.0 with adapter-based PostgreSQL connectivity, introduces configurable replication error recovery for critical services, adds flexible source-map support configuration, fixes console interceptor log-chain preservation, and implements CLI stability improvements. The changes span database connectivity patterns across all Prisma clients, new replication error recovery service with exponential-backoff reconnection, environment-variable-driven source maps utility, console method delegation, Docker Hub authentication for builds, engine-check bypassing during deployment, and proper SIGINT handling for dev process cleanup. Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 Prisma metrics endpoint and OTEL metrics removed — observability gap
The PR removes Prisma's built-in $metrics.prometheus() from the /metrics endpoint (apps/webapp/app/routes/metrics.ts) and the entire configurePrismaMetrics function from apps/webapp/app/v3/tracer.server.ts. With PrismaPg, Prisma no longer exposes internal pool metrics (connections open/busy/idle, query wait times, etc.). The pg pool itself exposes pool.totalCount, pool.idleCount, pool.waitingCount via the Pool instance, but these are not wired up anywhere. Operators using Grafana dashboards that monitor db.pool.connections.* or prisma_* metrics will see all those series go to zero after this deploys.
Was this helpful? React with 👍 or 👎 to provide feedback.
| @@ -260,8 +261,7 @@ export async function updateTriggerPackages( | |||
| await installDependencies({ cwd: projectPath, silent: true }); | |||
There was a problem hiding this comment.
🔴 ignoreEngines option declared but never passed to installDependencies
The ignoreEngines option was added to UpdateCommandOptions and is set to true in the deploy path (deploy.ts:262), but the actual installDependencies call at update.ts:261 never reads options.ignoreEngines and never passes engine-related args to installDependencies. The tests verify that args like ["--no-engine-strict"] are passed, but the implementation just calls installDependencies({ cwd: projectPath, silent: true }) with no args field. This means the stated fix — "Ignore engine checks during deployment install phase to prevent failure on build server when Node version mismatch exists" — has no effect at runtime.
Prompt for agents
The ignoreEngines option is added to CommonCommandOptions and passed from deploy.ts, but updateTriggerPackages in update.ts never uses it. The installDependencies call at line 261 needs to compute engine-related CLI args based on options.ignoreEngines and the detected package manager, then pass them via the args field.
The logic should be roughly:
1. After detecting the package manager (line 254), compute engine args based on options.ignoreEngines and packageManager.name
2. For npm: args = ["--no-engine-strict"]
3. For pnpm: args = ["--config.engine-strict=false"]
4. For yarn: args = ["--ignore-engines"]
5. If ignoreEngines is false/undefined, args = []
6. Pass args to installDependencies: await installDependencies({ cwd: projectPath, silent: true, args })
The tests in update.test.ts already verify the expected behavior — they just don't pass because the implementation is missing.
Was this helpful? React with 👍 or 👎 to provide feedback.
| switch (severityNumber) { | ||
| case SeverityNumber.INFO: | ||
| this.originalConsole.log(...args); | ||
| break; |
There was a problem hiding this comment.
🟡 console.info() delegated to originalConsole.log() instead of originalConsole.info()
Both log() and info() methods use SeverityNumber.INFO when calling #handleLog. In the switch at line 98, SeverityNumber.INFO always maps to this.originalConsole.log(...). This means console.info() calls are routed through the original console.log instead of the original console.info. The changeset explicitly states this fix "delegates to original console methods to preserve log chain when other interceptors (like Sentry) are present", but Sentry patches console.info separately — so info calls still bypass Sentry's console.info hook.
The switch treats log and info identically
log() at line 72 and info() at line 76 both pass SeverityNumber.INFO to #handleLog. The switch at line 98 has a single case SeverityNumber.INFO: that calls this.originalConsole.log(...), never this.originalConsole.info(...).
Prompt for agents
The root issue is that both log() and info() use SeverityNumber.INFO, so the switch statement cannot distinguish between them. To fix this properly, the #handleLog method needs an additional signal — for example, passing the original method name or using the severityText parameter (which is already "Log" vs "Info") to decide which original console method to call.
One approach: use severityText to decide:
case SeverityNumber.INFO:
if (severityText === "Info") {
this.originalConsole.info(...args);
} else {
this.originalConsole.log(...args);
}
break;
This ensures console.info() calls flow through the original console.info, preserving interceptor chains from Sentry and similar tools.
Was this helpful? React with 👍 or 👎 to provide feedback.
| idleTimeoutMillis: env.DATABASE_CONNECTION_TIMEOUT * 1000, | ||
| connectionTimeoutMillis: env.DATABASE_CONNECTION_TIMEOUT * 1000, |
There was a problem hiding this comment.
🚩 DATABASE_POOL_TIMEOUT env var is now unused after PrismaPg migration
The old Prisma connection string accepted pool_timeout (mapped from env.DATABASE_POOL_TIMEOUT, default 60s) and connection_timeout (mapped from env.DATABASE_CONNECTION_TIMEOUT, default 20s). The new PrismaPg adapter at apps/webapp/app/db.server.ts:143-144 uses env.DATABASE_CONNECTION_TIMEOUT * 1000 for both idleTimeoutMillis and connectionTimeoutMillis. The DATABASE_POOL_TIMEOUT env var defined at apps/webapp/app/env.server.ts:103 is now dead — any existing deployments that tuned DATABASE_POOL_TIMEOUT separately from DATABASE_CONNECTION_TIMEOUT will silently lose that customization. Additionally, idleTimeoutMillis (how long idle connections stay in the pool) is semantically different from the old Prisma pool_timeout (how long to wait for a free connection). The 20s idle timeout may cause more connection churn than the previous 60s pool_timeout in high-concurrency scenarios.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Migrate Prisma from 6.14.0 to 7.7.0 with driver adapters. Closes #3391