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,