Fix production deployments showing preview URLs#45
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe changes introduce a production URL management system that dynamically computes and validates production URLs across multiple components. New functions Sequence Diagram(s)sequenceDiagram
participant Action as Rollback/Queue Handler
participant URLMgr as URL Manager<br/>(productionUrl.ts)
participant DB as Database<br/>(updateProductionStatus)
participant StateMgr as State Manager<br/>(live/manager.ts)
Action->>URLMgr: getCanonicalProductionUrl(project)
URLMgr->>URLMgr: Resolve Tailscale URL or<br/>fallback to localhost
URLMgr-->>Action: Return canonical URL
Action->>DB: updateProductionStatus<br/>(id, status, {productionUrl})
DB-->>Action: Updated
Note over StateMgr: During buildState()
StateMgr->>URLMgr: repairStaleProductionUrl(project)
URLMgr->>DB: getProductionStatus(project)
DB-->>URLMgr: Current status & URL
alt Status is "running"
URLMgr->>URLMgr: Compute canonical URL
alt URLs mismatch
URLMgr->>DB: updateProductionStatus<br/>(id, status, {productionUrl: canonical})
DB-->>URLMgr: Updated
end
end
URLMgr-->>StateMgr: Validated/repaired URL
StateMgr->>StateMgr: Populate ProductionLiveState<br/>with correct URL
StateMgr-->>StateMgr: Broadcast state changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
❌ Preview deployment failed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/actions/projects.ts`:
- Around line 541-548: getCanonicalProductionUrl(project) can throw after the
rollback container is started, causing updateProductionStatus(input.projectId,
...) to be skipped; wrap the await getCanonicalProductionUrl(project) call in a
try/catch and on error fall back to the same localhost pattern used by
productionWaitReady (i.e. compute a localhost URL instead of throwing), then
call updateProductionStatus with productionHash: input.toHash,
productionStartedAt: new Date(), and productionUrl set to either the resolved
URL or the localhost fallback so the final updateProductionStatus always runs.
In `@src/server/productions/productionUrl.ts`:
- Around line 27-34: The repair currently calls
updateProductionStatus(project.id, project.productionStatus, { productionUrl:
canonicalUrl }) which overwrites the stored production_status with a stale
snapshot; replace this with a status-guarded URL-only update so you only set
productionUrl when the DB row still has the same status. Concretely, either
extend updateProductionStatus or add a new helper (e.g.,
updateProductionUrlIfStatusMatches) and have it perform an UPDATE that sets
production_url = canonicalUrl WHERE id = project.id AND production_status =
project.productionStatus (or accept an expectedStatus parameter), so you don't
write the stale productionStatus back into the row; call
getCanonicalProductionUrl and then this status-guarded URL-only updater instead
of the current updateProductionStatus invocation.
- Around line 5-15: The getCanonicalProductionUrl function should guard against
null/invalid productionPort and resolver exceptions: wrap the
getTailscaleProjectUrl(project.slug, "production", project.id) call in try/catch
and, on any error or if it returns null/undefined, fallback to a localhost URL
built from a validated port; treat project.productionPort as nullable and only
use it if Number.isInteger(+project.productionPort) && +project.productionPort >
0, otherwise use a safe default (e.g. 3000). Update getCanonicalProductionUrl to
perform this validation and error handling so it never returns
"http://localhost:null" and centralizes resolver-failure fallback behavior.
In `@src/server/queue/handlers/productionWaitReady.ts`:
- Around line 112-115: The fallback for computing productionUrl uses
getCanonicalProductionUrl(project) which can fall back to
project.productionPort; update the catch path in the Effect.tryPromise around
productionUrl to build the localhost fallback using the ready job's port
(payload.productionPort) instead of relying on project.productionPort so the
handler always uses the validated port; locate the Effect.tryPromise call that
assigns productionUrl in productionWaitReady.ts and change the catch to return
`http://localhost:${payload.productionPort}` (or equivalent string construction)
so the persisted URL won’t point at a stale DB port.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6284b8a7-d3ca-47e7-b85c-26554f482d3a
📒 Files selected for processing (5)
src/actions/projects.tssrc/server/live/manager.tssrc/server/productions/productionUrl.tssrc/server/queue/handlers/productionStop.tssrc/server/queue/handlers/productionWaitReady.ts
Comment on lines
+541
to
548
| const { getCanonicalProductionUrl } = await import( | ||
| "@/server/productions/productionUrl" | ||
| ); | ||
| await updateProductionStatus(input.projectId, "running", { | ||
| productionHash: input.toHash, | ||
| productionStartedAt: new Date(), | ||
| productionUrl: await getCanonicalProductionUrl(project), | ||
| }); |
There was a problem hiding this comment.
Keep rollback state updates resilient to URL resolution failures.
getCanonicalProductionUrl(project) can reject, and this happens after the rollback container has already been started. Use the same localhost fallback pattern as productionWaitReady so a transient URL-resolution failure does not skip the final updateProductionStatus.
🛠️ Proposed fix
const { getCanonicalProductionUrl } = await import(
"@/server/productions/productionUrl"
);
+ const productionUrl = await getCanonicalProductionUrl(project).catch(
+ () => `http://localhost:${productionPort}`,
+ );
await updateProductionStatus(input.projectId, "running", {
productionHash: input.toHash,
productionStartedAt: new Date(),
- productionUrl: await getCanonicalProductionUrl(project),
+ productionUrl,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/projects.ts` around lines 541 - 548,
getCanonicalProductionUrl(project) can throw after the rollback container is
started, causing updateProductionStatus(input.projectId, ...) to be skipped;
wrap the await getCanonicalProductionUrl(project) call in a try/catch and on
error fall back to the same localhost pattern used by productionWaitReady (i.e.
compute a localhost URL instead of throwing), then call updateProductionStatus
with productionHash: input.toHash, productionStartedAt: new Date(), and
productionUrl set to either the resolved URL or the localhost fallback so the
final updateProductionStatus always runs.
Comment on lines
+5
to
+15
| export async function getCanonicalProductionUrl( | ||
| project: Pick<Project, "id" | "slug" | "productionPort">, | ||
| ): Promise<string> { | ||
| const tailscaleUrl = await getTailscaleProjectUrl( | ||
| project.slug, | ||
| "production", | ||
| project.id, | ||
| ); | ||
|
|
||
| return tailscaleUrl ?? `http://localhost:${project.productionPort}`; | ||
| } |
There was a problem hiding this comment.
Harden canonical URL fallback for missing ports and resolver failures.
productionPort is nullable, so the current fallback can produce http://localhost:null. Also, callers now need to catch Tailscale resolver failures individually; centralizing that fallback here keeps rollback, wait-ready, and live repair behavior consistent.
🛠️ Proposed fix
export async function getCanonicalProductionUrl(
project: Pick<Project, "id" | "slug" | "productionPort">,
): Promise<string> {
- const tailscaleUrl = await getTailscaleProjectUrl(
- project.slug,
- "production",
- project.id,
- );
+ const tailscaleUrl = await getTailscaleProjectUrl(
+ project.slug,
+ "production",
+ project.id,
+ ).catch(() => null);
- return tailscaleUrl ?? `http://localhost:${project.productionPort}`;
+ if (tailscaleUrl) return tailscaleUrl;
+
+ if (project.productionPort == null) {
+ throw new Error(`Cannot resolve production URL without productionPort for project ${project.id}`);
+ }
+
+ return `http://localhost:${project.productionPort}`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/productions/productionUrl.ts` around lines 5 - 15, The
getCanonicalProductionUrl function should guard against null/invalid
productionPort and resolver exceptions: wrap the
getTailscaleProjectUrl(project.slug, "production", project.id) call in try/catch
and, on any error or if it returns null/undefined, fallback to a localhost URL
built from a validated port; treat project.productionPort as nullable and only
use it if Number.isInteger(+project.productionPort) && +project.productionPort >
0, otherwise use a safe default (e.g. 3000). Update getCanonicalProductionUrl to
perform this validation and error handling so it never returns
"http://localhost:null" and centralizes resolver-failure fallback behavior.
Comment on lines
+27
to
+34
| const canonicalUrl = await getCanonicalProductionUrl(project); | ||
| if (project.productionUrl === canonicalUrl) { | ||
| return canonicalUrl; | ||
| } | ||
|
|
||
| await updateProductionStatus(project.id, project.productionStatus, { | ||
| productionUrl: canonicalUrl, | ||
| }); |
There was a problem hiding this comment.
Guard repair writes against stale status snapshots.
updateProductionStatus always writes productionStatus. If a live poll fetched this project as "running" and production is stopped before line 32 executes, this repair can set the row back to "running" and restore the URL. Persist the repair with a status-guarded URL-only update instead of writing the stale status snapshot.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/productions/productionUrl.ts` around lines 27 - 34, The repair
currently calls updateProductionStatus(project.id, project.productionStatus, {
productionUrl: canonicalUrl }) which overwrites the stored production_status
with a stale snapshot; replace this with a status-guarded URL-only update so you
only set productionUrl when the DB row still has the same status. Concretely,
either extend updateProductionStatus or add a new helper (e.g.,
updateProductionUrlIfStatusMatches) and have it perform an UPDATE that sets
production_url = canonicalUrl WHERE id = project.id AND production_status =
project.productionStatus (or accept an expectedStatus parameter), so you don't
write the stale productionStatus back into the row; call
getCanonicalProductionUrl and then this status-guarded URL-only updater instead
of the current updateProductionStatus invocation.
Comment on lines
+112
to
+115
| const productionUrl = yield* Effect.tryPromise({ | ||
| try: () => getCanonicalProductionUrl(project), | ||
| catch: () => `http://localhost:${payload.productionPort}`, | ||
| }); |
There was a problem hiding this comment.
Use the ready job’s port when computing the fallback URL.
This handler has already validated readiness on payload.productionPort, but getCanonicalProductionUrl(project) falls back internally to project.productionPort. If the DB value is stale/null, the persisted URL can point at the wrong localhost port without hitting this catch block.
🛠️ Proposed fix
const productionUrl = yield* Effect.tryPromise({
- try: () => getCanonicalProductionUrl(project),
+ try: () =>
+ getCanonicalProductionUrl({
+ id: project.id,
+ slug: project.slug,
+ productionPort: payload.productionPort,
+ }),
catch: () => `http://localhost:${payload.productionPort}`,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/queue/handlers/productionWaitReady.ts` around lines 112 - 115, The
fallback for computing productionUrl uses getCanonicalProductionUrl(project)
which can fall back to project.productionPort; update the catch path in the
Effect.tryPromise around productionUrl to build the localhost fallback using the
ready job's port (payload.productionPort) instead of relying on
project.productionPort so the handler always uses the validated port; locate the
Effect.tryPromise call that assigns productionUrl in productionWaitReady.ts and
change the catch to return `http://localhost:${payload.productionPort}` (or
equivalent string construction) so the persisted URL won’t point at a stale DB
port.
|
❌ Preview deployment failed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
node_modulesmissing, sopnpm format/ typecheck could not be executed)Summary by CodeRabbit
Release Notes