From ea07db3881afb0ce1c88776633292eb480c3b62c Mon Sep 17 00:00:00 2001 From: Saadi Myftija Date: Tue, 1 Sep 2026 10:10:04 +0200 Subject: [PATCH] fix(webapp,rbac): retry branch environment lookups that race replica lag The first deploy of a newly created branch upserts the branch environment and immediately authenticates with it, so the auth-time replica read can miss the just-committed row and the request fails. Branch lookups in API auth (PAT and OAT resolution, the RBAC bearer resolver, and the legacy API key resolver) now retry the replica once with jitter and fall back to the primary before reporting the branch missing. Installs without a dedicated read replica skip the retry entirely. --- .../preview-branch-first-deploy-race.md | 6 ++ .../app/models/runtimeEnvironment.server.ts | 27 +++++- apps/webapp/app/services/apiAuth.server.ts | 63 +++++++------ .../app/services/authTelemetry.server.ts | 11 ++- .../app/services/replicaLagRetry.server.ts | 47 ++++++++++ apps/webapp/test/replicaLagRetry.test.ts | 88 +++++++++++++++++++ .../rbac/src/bearerCredentials.ts | 52 +++++++++-- 7 files changed, 260 insertions(+), 34 deletions(-) create mode 100644 .server-changes/preview-branch-first-deploy-race.md create mode 100644 apps/webapp/app/services/replicaLagRetry.server.ts create mode 100644 apps/webapp/test/replicaLagRetry.test.ts diff --git a/.server-changes/preview-branch-first-deploy-race.md b/.server-changes/preview-branch-first-deploy-race.md new file mode 100644 index 00000000000..d316864bc5d --- /dev/null +++ b/.server-changes/preview-branch-first-deploy-race.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Fixed a race that could make the first deploy of a newly created preview branch fail and eventually time out. Deploys to just-created branches now resolve reliably. diff --git a/apps/webapp/app/models/runtimeEnvironment.server.ts b/apps/webapp/app/models/runtimeEnvironment.server.ts index 790576200ec..09541efb6ba 100644 --- a/apps/webapp/app/models/runtimeEnvironment.server.ts +++ b/apps/webapp/app/models/runtimeEnvironment.server.ts @@ -6,6 +6,9 @@ import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver. import { logger } from "~/services/logger.server"; import { getUsername } from "~/utils/username"; import { hashApiKey } from "~/utils/apiKeys"; +import { findWithReplicaRetry } from "~/services/replicaLagRetry.server"; +import { observeBranchEnvironmentReplicaMiss } from "~/services/authTelemetry.server"; +import { isReadReplicaClient } from "@internal/run-store"; import { BuildRuntime } from "@trigger.dev/core/v3"; import { isAdditionalApiKey } from "@trigger.dev/core/v3/apiKeys"; import { isDefaultDevBranch, sanitizeBranchName } from "@trigger.dev/core/v3/utils/gitBranch"; @@ -109,6 +112,22 @@ export type ApiKeyEnvironmentResolution = * scopes explicitly grant full access; restricted keys fail closed here * (`reason: "restricted"`, so callers can explain the rejection). */ +// A just-created branch env can be missing from the replica when its first deploy authenticates. +function findBranchChildWithReplicaRetry( + tx: PrismaClientOrTransaction, + parentEnvironmentId: string, + branchName: string +) { + const where = { parentEnvironmentId, branchName, archivedAt: null }; + return findWithReplicaRetry({ + replicaFind: () => tx.runtimeEnvironment.findFirst({ where }), + primaryFind: () => prisma.runtimeEnvironment.findFirst({ where }), + hasDedicatedReplica: isReadReplicaClient(tx), + retryDelayMs: { min: 50, max: 200 }, + onOutcome: observeBranchEnvironmentReplicaMiss, + }); +} + async function resolveEnvironmentByApiKey( apiKey: string, branchName: string | undefined, @@ -227,7 +246,9 @@ async function resolveEnvironmentByApiKey( return { ok: false, reason: "not-found" }; } - const childEnvironment = environment.childEnvironments.at(0); + const childEnvironment = + environment.childEnvironments.at(0) ?? + (await findBranchChildWithReplicaRetry(tx, environment.id, branch)); if (childEnvironment) { return { @@ -248,7 +269,9 @@ async function resolveEnvironmentByApiKey( // If there is a named DEV branch (other than default), return it if (environment.type === "DEVELOPMENT" && branch !== undefined && !isDefaultDevBranch(branch)) { - const childEnvironment = environment.childEnvironments.at(0); + const childEnvironment = + environment.childEnvironments.at(0) ?? + (await findBranchChildWithReplicaRetry(tx, environment.id, branch)); if (childEnvironment) { return { diff --git a/apps/webapp/app/services/apiAuth.server.ts b/apps/webapp/app/services/apiAuth.server.ts index 5f7207bb0b7..c2d86d7b60c 100644 --- a/apps/webapp/app/services/apiAuth.server.ts +++ b/apps/webapp/app/services/apiAuth.server.ts @@ -2,7 +2,7 @@ import { json } from "@remix-run/server-runtime"; import { SignJWT } from "jose"; import { z } from "zod"; -import { $replica } from "~/db.server"; +import { $replica, prisma } from "~/db.server"; import { env } from "~/env.server"; import { findProjectByRef } from "~/models/project.server"; import { @@ -33,11 +33,15 @@ import { } from "./organizationAccessToken.server"; import { isPublicJWT, validatePublicJwtKey } from "./realtime/jwtAuth.server"; import { isDefaultDevBranch, sanitizeBranchName } from "@trigger.dev/core/v3/utils/gitBranch"; +import type { Prisma } from "@trigger.dev/database"; import { authenticateAuthorizeBearerWithTelemetry, authenticateBearerWithTelemetry, + observeBranchEnvironmentReplicaMiss, observeLegacyBearerAuthentication, } from "~/services/authTelemetry.server"; +import { findWithReplicaRetry } from "~/services/replicaLagRetry.server"; +import { isReadReplicaClient } from "@internal/run-store"; const ClaimsSchema = z.object({ scopes: z.array(z.string()).optional(), @@ -653,6 +657,21 @@ export async function authenticatedEnvironmentForAuthentication( return environment; } +const BRANCH_ENV_REPLICA_RETRY_DELAY_MS = { min: 50, max: 200 }; + +// A just-created branch env can be missing from the replica when its first deploy authenticates. +function findBranchEnvironment(where: Prisma.RuntimeEnvironmentWhereInput) { + return findWithReplicaRetry({ + replicaFind: () => + $replica.runtimeEnvironment.findFirst({ where, include: authIncludeWithParent }), + primaryFind: () => + prisma.runtimeEnvironment.findFirst({ where, include: authIncludeWithParent }), + hasDedicatedReplica: isReadReplicaClient($replica), + retryDelayMs: BRANCH_ENV_REPLICA_RETRY_DELAY_MS, + onOutcome: observeBranchEnvironmentReplicaMiss, + }); +} + async function resolveEnvironmentForAuthentication( auth: AuthenticationResult, projectRef: string, @@ -742,21 +761,18 @@ async function resolveEnvironmentForAuthentication( return toAuthenticated(environment); } - const environment = await $replica.runtimeEnvironment.findFirst({ - where: { - projectId: project.id, - type: slug === "dev" ? "DEVELOPMENT" : "PREVIEW", - branchName: resolvedBranch, - ...(slug === "dev" - ? { - orgMember: { - userId: user.id, - }, - } - : {}), - archivedAt: null, - }, - include: authIncludeWithParent, + const environment = await findBranchEnvironment({ + projectId: project.id, + type: slug === "dev" ? "DEVELOPMENT" : "PREVIEW", + branchName: resolvedBranch, + ...(slug === "dev" + ? { + orgMember: { + userId: user.id, + }, + } + : {}), + archivedAt: null, }); if (!environment) { @@ -813,15 +829,12 @@ async function resolveEnvironmentForAuthentication( return toAuthenticated(environment); } - const environment = await $replica.runtimeEnvironment.findFirst({ - where: { - projectId: project.id, - // No Development branches for OAT - type: "PREVIEW", - branchName: resolvedBranch, - archivedAt: null, - }, - include: authIncludeWithParent, + const environment = await findBranchEnvironment({ + projectId: project.id, + // No Development branches for OAT + type: "PREVIEW", + branchName: resolvedBranch, + archivedAt: null, }); if (!environment) { diff --git a/apps/webapp/app/services/authTelemetry.server.ts b/apps/webapp/app/services/authTelemetry.server.ts index b19fa6447c2..89b560fcbf9 100644 --- a/apps/webapp/app/services/authTelemetry.server.ts +++ b/apps/webapp/app/services/authTelemetry.server.ts @@ -11,6 +11,7 @@ import type { import { authFeatureControls } from "~/services/authFeatureControls.server"; import { rbac } from "~/services/rbac.server"; import { singleton } from "~/utils/singleton"; +import type { ReplicaRetryOutcome } from "~/services/replicaLagRetry.server"; type ApiAuthResult = "success" | "invalid" | "forbidden" | "disabled" | "error"; @@ -23,6 +24,10 @@ const telemetry = singleton("apiAuthTelemetry", () => { description: "Environment bearer authentication duration", unit: "ms", }); + const branchReplicaMiss = meter.createCounter("api_auth.branch_env_replica_miss", { + description: + "Branch environment lookups that missed the read replica, by recovery outcome (or not_found)", + }); meter .createObservableGauge("api_auth.rollout_mode", { @@ -35,9 +40,13 @@ const telemetry = singleton("apiAuthTelemetry", () => { }); }); - return { attempts, duration }; + return { attempts, duration, branchReplicaMiss }; }); +export function observeBranchEnvironmentReplicaMiss(outcome: ReplicaRetryOutcome) { + telemetry.branchReplicaMiss.add(1, { outcome }); +} + export async function authenticateBearerWithTelemetry( request: Request, options: BearerAuthOptions diff --git a/apps/webapp/app/services/replicaLagRetry.server.ts b/apps/webapp/app/services/replicaLagRetry.server.ts new file mode 100644 index 00000000000..73e35a26fcc --- /dev/null +++ b/apps/webapp/app/services/replicaLagRetry.server.ts @@ -0,0 +1,47 @@ +import { setTimeout as sleep } from "node:timers/promises"; + +export type ReplicaRetryOutcome = "replica_retry" | "primary" | "not_found"; + +// Replica-lag guard: on a miss, retry the replica once with jitter, then let the primary decide. +export async function findWithReplicaRetry({ + replicaFind, + primaryFind, + hasDedicatedReplica, + retryDelayMs, + onOutcome, +}: { + replicaFind: () => Promise; + primaryFind: () => Promise; + hasDedicatedReplica: boolean; + retryDelayMs: { min: number; max: number }; + onOutcome?: (outcome: ReplicaRetryOutcome) => void; +}): Promise { + const report = (outcome: ReplicaRetryOutcome) => { + try { + onOutcome?.(outcome); + } catch {} + }; + + const found = await replicaFind(); + if (found) { + return found; + } + + // Without a dedicated replica both lookups hit the same database, so a retry can't help. + if (!hasDedicatedReplica) { + report("not_found"); + return null; + } + + await sleep(retryDelayMs.min + Math.random() * Math.max(0, retryDelayMs.max - retryDelayMs.min)); + + const retried = await replicaFind(); + if (retried) { + report("replica_retry"); + return retried; + } + + const fromPrimary = await primaryFind(); + report(fromPrimary ? "primary" : "not_found"); + return fromPrimary; +} diff --git a/apps/webapp/test/replicaLagRetry.test.ts b/apps/webapp/test/replicaLagRetry.test.ts new file mode 100644 index 00000000000..850913bd734 --- /dev/null +++ b/apps/webapp/test/replicaLagRetry.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it, vi } from "vitest"; +import { findWithReplicaRetry } from "~/services/replicaLagRetry.server"; + +const base = { hasDedicatedReplica: true, retryDelayMs: { min: 0, max: 0 } }; + +describe("findWithReplicaRetry", () => { + it("returns the first replica hit without retrying or touching the primary", async () => { + const replicaFind = vi.fn().mockResolvedValue({ id: "env_1" }); + const primaryFind = vi.fn(); + const onOutcome = vi.fn(); + + const result = await findWithReplicaRetry({ ...base, replicaFind, primaryFind, onOutcome }); + + expect(result).toEqual({ id: "env_1" }); + expect(replicaFind).toHaveBeenCalledTimes(1); + expect(primaryFind).not.toHaveBeenCalled(); + expect(onOutcome).not.toHaveBeenCalled(); + }); + + it("recovers via a replica retry when the row appears on the second read", async () => { + const replicaFind = vi.fn().mockResolvedValueOnce(null).mockResolvedValueOnce({ id: "env_1" }); + const primaryFind = vi.fn(); + const onOutcome = vi.fn(); + + const result = await findWithReplicaRetry({ ...base, replicaFind, primaryFind, onOutcome }); + + expect(result).toEqual({ id: "env_1" }); + expect(replicaFind).toHaveBeenCalledTimes(2); + expect(primaryFind).not.toHaveBeenCalled(); + expect(onOutcome).toHaveBeenCalledWith("replica_retry"); + }); + + it("falls back to the primary when the replica misses twice", async () => { + const replicaFind = vi.fn().mockResolvedValue(null); + const primaryFind = vi.fn().mockResolvedValue({ id: "env_1" }); + const onOutcome = vi.fn(); + + const result = await findWithReplicaRetry({ ...base, replicaFind, primaryFind, onOutcome }); + + expect(result).toEqual({ id: "env_1" }); + expect(replicaFind).toHaveBeenCalledTimes(2); + expect(primaryFind).toHaveBeenCalledTimes(1); + expect(onOutcome).toHaveBeenCalledWith("primary"); + }); + + it("reports a genuine miss and returns null", async () => { + const replicaFind = vi.fn().mockResolvedValue(null); + const primaryFind = vi.fn().mockResolvedValue(null); + const onOutcome = vi.fn(); + + const result = await findWithReplicaRetry({ ...base, replicaFind, primaryFind, onOutcome }); + + expect(result).toBeNull(); + expect(onOutcome).toHaveBeenCalledWith("not_found"); + expect(onOutcome).toHaveBeenCalledTimes(1); + }); + + it("does a single lookup when there is no dedicated replica", async () => { + const replicaFind = vi.fn().mockResolvedValue(null); + const primaryFind = vi.fn(); + const onOutcome = vi.fn(); + + const result = await findWithReplicaRetry({ + ...base, + hasDedicatedReplica: false, + replicaFind, + primaryFind, + onOutcome, + }); + + expect(result).toBeNull(); + expect(replicaFind).toHaveBeenCalledTimes(1); + expect(primaryFind).not.toHaveBeenCalled(); + expect(onOutcome).toHaveBeenCalledWith("not_found"); + }); + + it("does not fail the lookup when the outcome callback throws", async () => { + const replicaFind = vi.fn().mockResolvedValueOnce(null).mockResolvedValueOnce({ id: "env_1" }); + const primaryFind = vi.fn(); + const onOutcome = vi.fn(() => { + throw new Error("meter unavailable"); + }); + + const result = await findWithReplicaRetry({ ...base, replicaFind, primaryFind, onOutcome }); + + expect(result).toEqual({ id: "env_1" }); + }); +}); diff --git a/internal-packages/rbac/src/bearerCredentials.ts b/internal-packages/rbac/src/bearerCredentials.ts index d1e4685c8ec..c66d93bcbe8 100644 --- a/internal-packages/rbac/src/bearerCredentials.ts +++ b/internal-packages/rbac/src/bearerCredentials.ts @@ -9,6 +9,7 @@ import { type BearerAuthResult, } from "@trigger.dev/plugins"; import { createHash } from "node:crypto"; +import { setTimeout as sleep } from "node:timers/promises"; import { buildJwtAbility, permissiveAbility } from "./ability.js"; export type BearerCredentialClients = { @@ -79,6 +80,27 @@ export class BearerCredentialResolver { this.replica = clients.replica; } + // A just-created branch env can be missing from the replica when its first deploy + // authenticates. Retry the replica once with jitter, then let the primary decide. + private async findBranchChildWithRetry( + parentEnvironmentId: string, + branchName: string + ): Promise { + if (this.replica === this.prisma) { + return null; + } + + const where = { parentEnvironmentId, branchName, archivedAt: null }; + + await sleep(50 + Math.random() * 150); + const retried = await this.replica.runtimeEnvironment.findFirst({ where }); + if (retried) { + return retried; + } + + return this.prisma.runtimeEnvironment.findFirst({ where }); + } + async authenticate( request: Request, options?: BearerAuthOptions @@ -272,7 +294,18 @@ export class BearerCredentialResolver { }; } - const [branchError, resolvedEnvironment] = resolveBranch(env, branchName, allowPreviewParent); + let branchResolution = resolveBranch(env, branchName, allowPreviewParent); + if (branchResolution[0] === "No matching branch env" && branchName !== null) { + const child = await this.findBranchChildWithRetry(env.id, branchName); + if (child) { + branchResolution = resolveBranch( + { ...env, childEnvironments: [child] }, + branchName, + allowPreviewParent + ); + } + } + const [branchError, resolvedEnvironment] = branchResolution; if (branchError !== null) { return { ok: false, @@ -324,11 +357,18 @@ export class BearerCredentialResolver { return { ok: false, status: 401, error: "Invalid API key", resolution }; } - const [branchError, resolvedEnvironment] = resolveBranch( - match.runtimeEnvironment, - branchName, - allowPreviewParent - ); + let branchResolution = resolveBranch(match.runtimeEnvironment, branchName, allowPreviewParent); + if (branchResolution[0] === "No matching branch env" && branchName !== null) { + const child = await this.findBranchChildWithRetry(match.runtimeEnvironment.id, branchName); + if (child) { + branchResolution = resolveBranch( + { ...match.runtimeEnvironment, childEnvironments: [child] }, + branchName, + allowPreviewParent + ); + } + } + const [branchError, resolvedEnvironment] = branchResolution; if (branchError !== null) { return { ok: false,