diff --git a/.server-changes/remove-unused-trace-sync-route.md b/.server-changes/remove-unused-trace-sync-route.md new file mode 100644 index 00000000000..b2e857405a0 --- /dev/null +++ b/.server-changes/remove-unused-trace-sync-route.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Removed an unused internal dashboard sync route that is no longer part of any product surface. diff --git a/apps/webapp/app/routes/sync.traces.$traceId.ts b/apps/webapp/app/routes/sync.traces.$traceId.ts deleted file mode 100644 index d7695014b56..00000000000 --- a/apps/webapp/app/routes/sync.traces.$traceId.ts +++ /dev/null @@ -1,95 +0,0 @@ -import type { LoaderFunctionArgs } from "@remix-run/node"; -import { z } from "zod"; -import { $replica } from "~/db.server"; -import { env } from "~/env.server"; -import { logger } from "~/services/logger.server"; -import { getUserId } from "~/services/session.server"; -import { longPollingFetch } from "~/utils/longPollingFetch"; -import { - OtelTraceIdSchema, - RESERVED_ELECTRIC_SHAPE_PARAMS, - buildElectricTraceWhereClause, -} from "~/v3/electricShape.server"; - -const Params = z.object({ - traceId: OtelTraceIdSchema, -}); - -export async function loader({ params, request }: LoaderFunctionArgs) { - try { - const userId = await getUserId(request); - - const parsedParams = Params.safeParse(params); - if (!parsedParams.success) { - // Treat a malformed traceId as not-found rather than 400 to avoid - // signalling the validator. - return new Response("Not found", { status: 404 }); - } - const { traceId } = parsedParams.data; - - logger.log(`/sync/traces/${traceId}`, { userId }); - - if (!userId) { - return new Response("No user found in cookie", { status: 401 }); - } - - const trace = await $replica.taskEvent.findFirst({ - select: { - organizationId: true, - }, - where: { - traceId, - }, - }); - - if (!trace) { - return new Response("No trace found", { status: 404 }); - } - - const member = await $replica.orgMember.findFirst({ - where: { - organizationId: trace.organizationId, - userId, - }, - }); - - if (!member) { - return new Response("Not a member of this org", { status: 401 }); - } - - const url = new URL(request.url); - const originUrl = new URL(`${env.ELECTRIC_ORIGIN}/v1/shape/public."TaskEvent"`); - // Strip params we set ourselves so the caller can't override them. - url.searchParams.forEach((value, key) => { - if (RESERVED_ELECTRIC_SHAPE_PARAMS.has(key)) return; - originUrl.searchParams.set(key, value); - }); - - originUrl.searchParams.set( - "where", - buildElectricTraceWhereClause({ - traceId, - scope: { column: "organizationId", id: trace.organizationId }, - }) - ); - - const finalUrl = originUrl.toString(); - - logger.log("Fetching trace data", { url: finalUrl }); - - return longPollingFetch(finalUrl); - } catch (error) { - if (error instanceof Response) { - // Error responses from longPollingFetch - return error; - } else if (error instanceof TypeError) { - // Unexpected errors - logger.error("Unexpected error in loader:", { error: error.message }); - return new Response("An unexpected error occurred", { status: 500 }); - } else { - // Unknown errors - logger.error("Unknown error occurred in loader, not Error", { error: JSON.stringify(error) }); - return new Response("An unknown error occurred", { status: 500 }); - } - } -} diff --git a/apps/webapp/app/routes/sync.traces.runs.$traceId.ts b/apps/webapp/app/routes/sync.traces.runs.$traceId.ts deleted file mode 100644 index 24870cf2f49..00000000000 --- a/apps/webapp/app/routes/sync.traces.runs.$traceId.ts +++ /dev/null @@ -1,117 +0,0 @@ -import type { LoaderFunctionArgs } from "@remix-run/node"; -import { z } from "zod"; -import { $replica } from "~/db.server"; -import { env } from "~/env.server"; -import { logger } from "~/services/logger.server"; -import { getUserId } from "~/services/session.server"; -import { longPollingFetch } from "~/utils/longPollingFetch"; -import { - OtelTraceIdSchema, - RESERVED_ELECTRIC_SHAPE_PARAMS, - buildElectricTraceWhereClause, -} from "~/v3/electricShape.server"; -import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server"; -import { runStore } from "~/v3/runStore.server"; - -const Params = z.object({ - traceId: OtelTraceIdSchema, -}); - -export async function loader({ params, request }: LoaderFunctionArgs) { - try { - const userId = await getUserId(request); - - const parsedParams = Params.safeParse(params); - if (!parsedParams.success) { - return new Response("Not found", { status: 404 }); - } - const { traceId } = parsedParams.data; - - logger.log(`/sync/runs/${traceId}`, { userId }); - - if (!userId) { - return new Response("No user found in cookie", { status: 401 }); - } - - let run = await runStore.findRun( - { - traceId, - }, - { - select: { - projectId: true, - runtimeEnvironmentId: true, - }, - }, - $replica - ); - - if (!run) { - // Read-your-writes: a just-created run may not have replicated yet. Re-read the owning - // primary before 404ing so a live run's realtime trace feed isn't spuriously not-found. - run = await runStore.findRunOnPrimary( - { traceId }, - { select: { projectId: true, runtimeEnvironmentId: true } } - ); - } - - if (!run) { - return new Response("No run found", { status: 404 }); - } - - const resolvedEnv = await controlPlaneResolver.resolveEnv(run.runtimeEnvironmentId); - - if (!resolvedEnv) { - return new Response("No run found", { status: 404 }); - } - - const member = await $replica.orgMember.findFirst({ - where: { - organizationId: resolvedEnv.organizationId, - userId, - }, - }); - - if (!member) { - return new Response("Not a member of this org", { status: 401 }); - } - - const url = new URL(request.url); - const originUrl = new URL(`${env.ELECTRIC_ORIGIN}/v1/shape/public."TaskRun"`); - // Strip params we set ourselves so the caller can't override them. - url.searchParams.forEach((value, key) => { - if (RESERVED_ELECTRIC_SHAPE_PARAMS.has(key)) return; - originUrl.searchParams.set(key, value); - }); - - originUrl.searchParams.set( - "where", - // Scope by non-null projectId, not the nullable organizationId (legacy - // rows would vanish). Tenant-safe: membership was verified against this - // project's org and a trace's runs all live in one project. - buildElectricTraceWhereClause({ - traceId, - scope: { column: "projectId", id: run.projectId }, - }) - ); - - const finalUrl = originUrl.toString(); - - logger.log("Fetching trace runs data", { url: finalUrl }); - - return longPollingFetch(finalUrl); - } catch (error) { - if (error instanceof Response) { - // Error responses from longPollingFetch - return error; - } else if (error instanceof TypeError) { - // Unexpected errors - logger.error("Unexpected error in loader:", { error: error.message }); - return new Response("An unexpected error occurred", { status: 500 }); - } else { - // Unknown errors - logger.error("Unknown error occurred in loader, not Error", { error: JSON.stringify(error) }); - return new Response("An unknown error occurred", { status: 500 }); - } - } -} diff --git a/apps/webapp/app/v3/electricShape.server.ts b/apps/webapp/app/v3/electricShape.server.ts index c430cc0273c..65d52032afb 100644 --- a/apps/webapp/app/v3/electricShape.server.ts +++ b/apps/webapp/app/v3/electricShape.server.ts @@ -1,47 +1,3 @@ -import { z } from "zod"; - -/** - * OTel trace IDs are 32 lowercase hex chars. The traceparent parser only - * checks the dash-delimited format, so crafted ids can be persisted and later - * interpolated into shape `where` clauses. Validate here to close the SQLi vector. - */ -export const OtelTraceIdSchema = z - .string() - .regex(/^[0-9a-f]{32}$/, "traceId must be 32 lowercase hex characters"); - -/** Params the sync routes set themselves; stripped from incoming requests. */ -export const RESERVED_ELECTRIC_SHAPE_PARAMS = new Set(["where", "table", "columns"]); - -const CUID_LIKE = /^[a-z][a-z0-9_]*$/i; - -/** - * Tenant column a trace shape is scoped by. TaskEvent scopes by non-null - * organizationId; TaskRun scopes by non-null projectId (its organizationId is - * nullable). The column is from this fixed union, never user input, so it's - * safe to interpolate. - */ -export type TraceScope = - | { column: "organizationId"; id: string } - | { column: "projectId"; id: string }; - -/** - * Build the Electric Shape `where` clause for the trace sync routes. Both ids - * are re-validated as defense-in-depth so a missed call site can't bypass scope. - */ -export function buildElectricTraceWhereClause(args: { - traceId: string; - scope: TraceScope; -}): string { - const { traceId, scope } = args; - if (!OtelTraceIdSchema.safeParse(traceId).success) { - throw new Error("buildElectricTraceWhereClause: unsafe traceId"); - } - if (!CUID_LIKE.test(scope.id)) { - throw new Error("buildElectricTraceWhereClause: unsafe scope id"); - } - return `"traceId"='${traceId}' AND "${scope.column}"='${scope.id}'`; -} - /** * Characters rejected in realtime tag values — the single source of truth * shared by the apiBuilder Zod refine (`realtime.v1.runs.ts`) and the runtime diff --git a/apps/webapp/test/spanTraceRoutes.replicaLag.test.ts b/apps/webapp/test/spanTraceRoutes.replicaLag.test.ts index 6b8dac27932..3c81ab3c5e4 100644 --- a/apps/webapp/test/spanTraceRoutes.replicaLag.test.ts +++ b/apps/webapp/test/spanTraceRoutes.replicaLag.test.ts @@ -2,12 +2,11 @@ // route caller (never a reimplemented read) against a real split Postgres with the owning LEGACY replica // FROZEN via laggingReplica. The routes pass a branded $replica, so reads stay on the owning replica. // Only orthogonal webapp singletons are mocked (bearer auth, mollifier buffer, ClickHouse repository, -// formatters, session cookie, control-plane resolver, longPollingFetch). +// formatters). // // Properties per read: the spans/trace findResource reads emit a RETRYABLE 404 (x-should-retry:true) on a // replica+buffer miss and return 200 once the replica catches up (proven with a caught-up store); the -// triggeredRuns list simply omits a just-triggered child under lag (200, eventually consistent); the sync -// trace-runs loader recovers a live run via a primary re-read on a replica miss and still 404s a truly-absent run. +// triggeredRuns list simply omits a just-triggered child under lag (200, eventually consistent). import { describe, expect, vi } from "vitest"; import { heteroRunOpsPostgresTest, laggingReplica } from "@internal/testcontainers"; @@ -20,20 +19,15 @@ vi.setConfig({ testTimeout: 60_000, hookTimeout: 60_000 }); // ---- Hoisted holders wired into the mocked module singletons before each loader call. ------------- // The branded `$replica` marker uses the global-registry symbol the run-store brands replicas with -// (readReplicaClient.ts) so the routing store keeps the read on the owning REPLICA. It also carries a -// live `orgMember` delegate (pointed at the real prisma14) because the sync route reads -// `$replica.orgMember.findFirst` directly — orthogonal to the run read under test. +// (readReplicaClient.ts) so the routing store keeps the read on the owning REPLICA. const { holder } = vi.hoisted(() => { const REPLICA_BRAND = Symbol.for("trigger.dev/run-store/read-replica"); return { holder: { REPLICA_BRAND, store: undefined as unknown, - replicaMarker: undefined as unknown, environment: undefined as unknown, bufferResult: null as unknown, - userId: undefined as unknown, - resolvedEnv: undefined as unknown, span: undefined as unknown, traceSummary: undefined as unknown, }, @@ -57,7 +51,7 @@ vi.mock("~/v3/runStore.server", () => ({ ), })); -// `$replica` brand marker (routes it to the owning replica) + a live orgMember delegate for the sync route. +// `$replica` brand marker: routes the read to the owning replica. vi.mock("~/db.server", () => ({ prisma: {}, $replica: new Proxy( @@ -65,8 +59,7 @@ vi.mock("~/db.server", () => ({ { get(_t, prop) { if (prop === holder.REPLICA_BRAND) return true; - const marker = holder.replicaMarker as Record | undefined; - return marker ? marker[prop] : undefined; + return undefined; }, } ), @@ -108,27 +101,8 @@ vi.mock("~/v3/mollifier/syntheticApiResponses.server", () => ({ buildSyntheticTraceBody: (r: unknown) => ({ synthetic: true, run: r }), })); -// Sync-route peripherals (orthogonal to the run read under test). -vi.mock("~/services/session.server", () => ({ - getUserId: async () => holder.userId, -})); -vi.mock("~/v3/runOpsMigration/controlPlaneResolver.server", () => ({ - controlPlaneResolver: { resolveEnv: async () => holder.resolvedEnv }, -})); -vi.mock("~/utils/longPollingFetch", () => ({ - longPollingFetch: async () => - new Response("shape-stream-ok", { status: 200, headers: { "x-longpoll": "hit" } }), -})); -// Ensure ELECTRIC_ORIGIN is defined for the sync route's happy path; keep every other env value real -// so the heavy apiBuilder import graph still loads. -vi.mock("~/env.server", async (importOriginal) => { - const actual = (await importOriginal()) as { env: Record }; - return { env: { ...actual.env, ELECTRIC_ORIGIN: "http://electric.test" } }; -}); - import { loader as spansLoader } from "~/routes/api.v1.runs.$runId.spans.$spanId"; import { loader as traceLoader } from "~/routes/api.v1.runs.$runId.trace"; -import { loader as syncTraceRunsLoader } from "~/routes/sync.traces.runs.$traceId"; // A cuid (25 chars after `run_`) classifies LEGACY, so both the create and the friendlyId/traceId // reads route to the legacy (control-plane) store — the store that owns these runs. @@ -266,13 +240,6 @@ function traceRequest(runId: string) { context: {} as never, }; } -function syncRequest(traceId: string) { - return { - request: new Request(`https://app.trigger.dev/sync/traces/runs/${traceId}?live=true`), - params: { traceId }, - context: {} as never, - } as never; -} describe("run-trace/span-detail route loaders under a lagging replica", () => { // spans loader — findResource findRun ($replica) @@ -466,81 +433,4 @@ describe("run-trace/span-detail route loaders under a lagging replica", () => { expect(body2.trace).toEqual({ rootSpanId: `span_${suffix}`, spans: [] }); } ); - - // sync trace-runs loader — findRun by traceId ($replica), with a primary fallback - heteroRunOpsPostgresTest( - "sync trace-runs loader: a live run lagging the replica is recovered via the primary fallback", - async ({ prisma14, prisma17 }) => { - const suffix = `sync_${seq++}`; - const seed = await seedTenant(prisma14, suffix); - const runId = `run_${CUID_25}`; - const friendlyId = `run_${suffix}`; - const traceId = "a".repeat(32); - const userId = `user_${suffix}`; - - // The dashboard user, joined to the org so the route's real orgMember check passes. - await prisma14.user.create({ - data: { id: userId, email: `u-${suffix}@example.com`, authenticationMethod: "MAGIC_LINK" }, - }); - await prisma14.orgMember.create({ - data: { userId, organizationId: seed.organization.id, role: "ADMIN" }, - }); - - const lagged = buildRouter(prisma14, prisma17, [{ model: "taskRun", mode: "missing" }]); - await lagged.legacyStore.createRun( - buildCreateRunInput({ - runId, - friendlyId, - organizationId: seed.organization.id, - projectId: seed.project.id, - runtimeEnvironmentId: seed.environment.id, - traceId, - spanId: `span_${suffix}`, - }) - ); - - holder.store = lagged.router; - holder.environment = authEnvironment(seed); - holder.userId = userId; - holder.resolvedEnv = { organizationId: seed.organization.id }; - holder.replicaMarker = { orgMember: prisma14.orgMember }; - - const res = (await syncTraceRunsLoader(syncRequest(traceId))) as Response; - - // The frozen replica WAS consulted (the lag was really exercised). - expect(lagged.legacyReplica.wasHit("taskRun")).toBe(true); - - // The primary fallback recovers the live run and the loader proceeds to the shape stream (200). - expect(res.status).toBe(200); - expect(res.headers.get("x-longpoll")).toBe("hit"); - } - ); - - // Negative control for the sync route: a truly-absent run must still 404 (the primary fallback - // recovers a LIVE run, it does not turn every miss into a 200). - heteroRunOpsPostgresTest( - "sync trace-runs loader: 404s when the run is absent on the primary too", - async ({ prisma14, prisma17 }) => { - const suffix = `sync_absent_${seq++}`; - const seed = await seedTenant(prisma14, suffix); - const userId = `user_${suffix}`; - await prisma14.user.create({ - data: { id: userId, email: `u-${suffix}@example.com`, authenticationMethod: "MAGIC_LINK" }, - }); - await prisma14.orgMember.create({ - data: { userId, organizationId: seed.organization.id, role: "ADMIN" }, - }); - - const lagged = buildRouter(prisma14, prisma17, [{ model: "taskRun", mode: "missing" }]); - holder.store = lagged.router; - holder.environment = authEnvironment(seed); - holder.userId = userId; - holder.resolvedEnv = { organizationId: seed.organization.id }; - holder.replicaMarker = { orgMember: prisma14.orgMember }; - - const res = (await syncTraceRunsLoader(syncRequest("b".repeat(32)))) as Response; - expect(lagged.legacyReplica.wasHit("taskRun")).toBe(true); - expect(res.status).toBe(404); - } - ); }); diff --git a/internal-packages/run-store/src/runOpsStore.routesSpanTraceReadView.replicaLag.test.ts b/internal-packages/run-store/src/runOpsStore.routesSpanTraceReadView.replicaLag.test.ts index 15754d55724..5a5ecb4a790 100644 --- a/internal-packages/run-store/src/runOpsStore.routesSpanTraceReadView.replicaLag.test.ts +++ b/internal-packages/run-store/src/runOpsStore.routesSpanTraceReadView.replicaLag.test.ts @@ -1,5 +1,5 @@ -// Lagging-replica coverage for the SPAN-DETAIL / TRACE / TRACE-SYNC route read views on the run-ops -// split. Four run lookups back three GET routes; NONE is a read-your-writes mutation gate — every one +// Lagging-replica coverage for the SPAN-DETAIL / TRACE route read views on the run-ops +// split. Three run lookups back two GET routes; NONE is a read-your-writes mutation gate — every one // resolves a run for a DISPLAY/GET response and tolerates a stale/missing read by a named caller // mechanism. This builds the store as each route holds it (`runStore` via a RoutingRunStore), freezes // the OWNING store's replica with the shared laggingReplica primitive, invokes each read EXACTLY as @@ -22,8 +22,6 @@ // 2. spans.$spanId findRuns({where:{runtimeEnvironmentId, parentSpanId}, take:50, select}) — open // predicate → both stores' replicas, merged. // 3. trace findRun({friendlyId, runtimeEnvironmentId}) — same shape as read 1. -// 4. sync.traces findRun({traceId}, {select:{runtimeEnvironmentId}}) — unclassifiable where → -// unrouted lookup (NEW-first then LEGACY, both replicas). // // Real split topology via heteroRunOpsPostgresTest — NEVER mocked. The laggingReplica primitive // freezes findFirst/findMany/findUnique on a chosen Prisma model so a replica-routed read misses the @@ -136,7 +134,7 @@ function makeRouters(prisma14: PrismaClient, prisma17: RunOpsPrismaClient) { return { laggingRouter, healthyRouter, legacyReplica }; } -describe("run-ops split — span-detail / trace / trace-sync route read views vs. a lagging replica", () => { +describe("run-ops split — span-detail / trace route read views vs. a lagging replica", () => { // --- spans.$spanId findPgRun -------------------------------------------------------------------- // findResource resolves the run for the span-detail GET. Under lag the owning (legacy) replica // returns null; the route falls through to the mollifier buffer fallback and, failing that, returns @@ -273,50 +271,4 @@ describe("run-ops split — span-detail / trace / trace-sync route read views vs expect(recovered!.friendlyId).toBe(friendlyId); } ); - - // --- sync.traces findRun ------------------------------------------------------------------------ - // The dashboard trace-sync loader resolves a run by traceId to authorize the user + resolve the - // env, then long-polls Electric for the "TaskRun" shape. The where carries ONLY traceId - // (unclassifiable) → unrouted lookup (NEW-first then LEGACY, both on their replicas). Under lag the - // owning replica returns null → the loader returns a 404 "No run found". This is a live-sync loader - // the dashboard re-establishes (long-poll); a run reaching it is user-navigation sourced and has - // existed for seconds, so the stale-null window is transient and re-polled. Tolerated (display/sync - // loader, re-polled; no mutation). Store fact under lag: null. - heteroRunOpsPostgresTest( - "sync.traces findRun(traceId, select, branded $replica) is NULL under owning-replica lag; caught-up replica recovers the run (re-polled sync loader tolerates the stale 404)", - async ({ prisma14, prisma17 }) => { - const { laggingRouter, healthyRouter, legacyReplica } = makeRouters(prisma14, prisma17); - const seed = await seedEnvironment(prisma14, "trace_sync"); - const runId = `run_${CUID_25}`; - const traceId = "trace_sync_unique"; - await prisma14.taskRun.create({ - data: taskRunData({ - id: runId, - friendlyId: `run_tracesync00000000000000`, - organizationId: seed.organization.id, - projectId: seed.project.id, - runtimeEnvironmentId: seed.environment.id, - traceId, - spanId: "span_trace_sync", - }), - }); - - // Exactly the call site: findRun({traceId}, {select:{runtimeEnvironmentId}}, $replica). - const stale = await laggingRouter.findRun( - { traceId }, - { select: { runtimeEnvironmentId: true } }, - BRANDED_REPLICA - ); - expect(stale).toBeNull(); - expect(legacyReplica.wasHit()).toBe(true); - - const recovered = (await healthyRouter.findRun( - { traceId }, - { select: { runtimeEnvironmentId: true } }, - BRANDED_REPLICA - )) as { runtimeEnvironmentId: string } | null; - expect(recovered).not.toBeNull(); - expect(recovered!.runtimeEnvironmentId).toBe(seed.environment.id); - } - ); });