Skip to content

Commit 77f3ce1

Browse files
committed
fix(webapp): drop an unconfigured gen-2 batch member loudly, not into the gen-1 read
Raised in review. When the shard map was non-empty but lacked the id's shard key, the member fell into the gen-1 group. The new replica is a different database so it missed, and a dedicated-family id never reaches the legacy probe, so the member vanished from the results with no error — the exact silent drop this change set out to remove. Report it and drop it, matching hydrateRunsAcrossSeam. The gen-2 tests now use the N-shard fixture that arrived with main, so the shard is a real separate database and a member seeded there is genuinely absent from the gen-1 store. That removes the handle-shadowing proxy the earlier version of the test needed to discriminate.
1 parent 8455b57 commit 77f3ce1

2 files changed

Lines changed: 108 additions & 37 deletions

File tree

apps/webapp/app/presenters/v3/ApiBatchResultsPresenter.server.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { BasePresenter } from "./basePresenter.server";
1414

1515
import { boundedIn } from "@trigger.dev/database";
1616
import { runOpsShardReplicas } from "~/v3/runOpsMigration/shardHandles.server";
17+
import { logger } from "~/services/logger.server";
1718
/**
1819
* Run-ops read-through wiring. All optional; absent (or `splitEnabled` falsy) collapses `call` to
1920
* passthrough. `legacyReplica` is a READ REPLICA handle only — there is NO legacy-primary field.
@@ -191,11 +192,20 @@ export class ApiBatchResultsPresenter extends BasePresenter {
191192
const idsByShard = new Map<ShardKey, string[]>();
192193
for (const id of taskRunIds) {
193194
const shardKey = resolveShard(id);
194-
if (shardKey !== "new" && shardKey !== "legacy" && shardReplicas.has(shardKey)) {
195+
if (shardKey === "new" || shardKey === "legacy") {
196+
genOneIds.push(id);
197+
} else if (shardReplicas.has(shardKey)) {
195198
const group = idsByShard.get(shardKey);
196199
group ? group.push(id) : idsByShard.set(shardKey, [id]);
197200
} else {
198-
genOneIds.push(id);
201+
// Not routable and not a gen-1 shape. A gen-1 store is the wrong database, and a
202+
// dedicated-family id never reaches the legacy probe, so falling back there would
203+
// drop the member silently. Drop it loudly instead.
204+
logger.error("ApiBatchResultsPresenter: gen-2 member on an unconfigured shard key", {
205+
runId: id,
206+
shardKey,
207+
configured: [...shardReplicas.keys()],
208+
});
199209
}
200210
}
201211

apps/webapp/test/apiBatchResultsPresenter.dedicatedSeam.test.ts

Lines changed: 96 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// RESULTS READ assembles correctly when one batch's members are genuinely split across the real
44
// dedicated run-ops subset schema (prisma17 / RunOpsPrismaClient) and the full control-plane
55
// schema (prisma14) — not a mirrored full schema on both sides. No mocks.
6-
import { heteroRunOpsPostgresTest } from "@internal/testcontainers";
6+
import { heteroRunOpsPostgresTest, makeNShardRunOpsPostgresTest } from "@internal/testcontainers";
77
import type { RunOpsPrismaClient } from "@internal/run-ops-database";
88
import type { PrismaClient } from "@trigger.dev/database";
99
import { generateRunOpsId, generateRunOpsIdV2 } from "@trigger.dev/core/v3/isomorphic";
@@ -209,6 +209,10 @@ async function seedBatchOnNew(
209209
return batch;
210210
}
211211

212+
// One real gen-2 shard on its OWN database, so a member seeded there is genuinely absent
213+
// from the gen-1 `new` store rather than merely routed away from it.
214+
const oneShardTest = makeNShardRunOpsPostgresTest(1);
215+
212216
const env = (ctx: SeedCtx) =>
213217
({
214218
id: ctx.environment.id,
@@ -338,19 +342,21 @@ describe("ApiBatchResultsPresenter split mode — real run-ops dedicated schema
338342
// A gen-2 member is directly routable to its own shard. Before the shard arm existed it
339343
// joined the gen-1 `new` read, missed there, and — classifying dedicated-family — never
340344
// reached the legacy probe either, so it vanished from the batch results with no error.
341-
heteroRunOpsPostgresTest(
342-
"a gen-2 member is hydrated from its own shard alongside a legacy-resident member",
343-
async ({ prisma14, prisma17 }: { prisma14: PrismaClient; prisma17: RunOpsPrismaClient }) => {
344-
const ctx = await seedLegacyEnv(prisma14, "gen2-shard");
345-
await relaxLegacyAttemptFk(prisma14);
346-
await relaxNewBatchItemFk(prisma17);
345+
oneShardTest(
346+
"a gen-2 member is hydrated from its own shard database alongside a legacy-resident member",
347+
async ({ legacyPrisma, newPrisma, shardPrismas }) => {
348+
const shardPrisma = shardPrismas[0]!;
349+
const ctx = await seedLegacyEnv(legacyPrisma, "gen2-shard");
350+
await relaxLegacyAttemptFk(legacyPrisma);
351+
await relaxNewBatchItemFk(newPrisma);
347352

348353
const shardMemberId = generateRunOpsIdV2("a");
349-
expect(shardMemberId.length).toBe(26);
350354
const legacyMemberId = generateLegacyCuid();
351355

356+
// The gen-2 member exists ONLY on the shard database. The gen-1 `new` store below is a
357+
// different database, so routing this id there would genuinely miss.
352358
await seedNewMember(
353-
prisma17,
359+
shardPrisma,
354360
{ envId: ctx.environment.id, orgId: ctx.organization.id, projectId: ctx.project.id },
355361
{
356362
id: shardMemberId,
@@ -359,44 +365,24 @@ describe("ApiBatchResultsPresenter split mode — real run-ops dedicated schema
359365
output: JSON.stringify({ from: "shard-a" }),
360366
}
361367
);
362-
await seedLegacyMember(prisma14, ctx, {
368+
await seedLegacyMember(legacyPrisma, ctx, {
363369
id: legacyMemberId,
364370
friendlyId: "run_legacy_member",
365371
status: "COMPLETED_WITH_ERRORS",
366372
error: { type: "BUILT_IN_ERROR", name: "Err", message: "boom", stackTrace: "" },
367373
});
368374

369375
const batchFriendlyId = "batch_gen2_shard";
370-
await seedBatchOnNew(prisma17, ctx.environment.id, batchFriendlyId, [
376+
await seedBatchOnNew(newPrisma, ctx.environment.id, batchFriendlyId, [
371377
shardMemberId,
372378
legacyMemberId,
373379
]);
374380

375-
// Both handles point at prisma17 (there is no third container), so the discriminator
376-
// cannot be WHICH database answers — it has to be WHICH HANDLE is asked. The gen-1 new
377-
// handle serves the batch row but REFUSES a taskRun read that includes the gen-2 id;
378-
// only routing that id to the shard handle avoids the refusal.
379-
const genOneNew = new Proxy(prisma17 as unknown as PrismaReplicaClient, {
380-
get(target, prop, receiver) {
381-
if (prop === "taskRun") {
382-
return {
383-
findMany: (args: { where?: { id?: { in?: string[] } } }) => {
384-
if (args.where?.id?.in?.includes(shardMemberId)) {
385-
throw new Error("a gen-2 id must not be read on the gen-1 new handle");
386-
}
387-
return (target as unknown as RunOpsPrismaClient).taskRun.findMany(args as never);
388-
},
389-
};
390-
}
391-
return Reflect.get(target, prop, receiver);
392-
},
393-
});
394-
395381
const presenter = new ApiBatchResultsPresenter(throwingPrisma, throwingPrisma, {
396382
splitEnabled: true,
397-
newClient: genOneNew,
398-
legacyReplica: prisma14 as unknown as PrismaReplicaClient,
399-
shardReplicas: new Map([["a", prisma17 as unknown as PrismaReplicaClient]]),
383+
newClient: newPrisma as unknown as PrismaReplicaClient,
384+
legacyReplica: legacyPrisma as unknown as PrismaReplicaClient,
385+
shardReplicas: new Map([["a", shardPrisma as unknown as PrismaReplicaClient]]),
400386
});
401387

402388
const result = await presenter.call(batchFriendlyId, env(ctx));
@@ -412,6 +398,81 @@ describe("ApiBatchResultsPresenter split mode — real run-ops dedicated schema
412398
outputType: "application/json",
413399
});
414400
expect(second).toMatchObject({ ok: false, id: "run_legacy_member" });
415-
}
401+
},
402+
180_000
403+
);
404+
405+
// A gen-2 id naming a shard that is NOT configured must not fall back onto a gen-1 store:
406+
// that reads the wrong database, misses, and (being dedicated-family) never reaches the
407+
// legacy probe, so the member disappears with no error. Drop it, but loudly.
408+
oneShardTest(
409+
"a gen-2 member on an unconfigured shard is dropped without being read from a gen-1 store",
410+
async ({ legacyPrisma, newPrisma, shardPrismas }) => {
411+
const shardPrisma = shardPrismas[0]!;
412+
const ctx = await seedLegacyEnv(legacyPrisma, "gen2-unconfigured");
413+
await relaxLegacyAttemptFk(legacyPrisma);
414+
await relaxNewBatchItemFk(newPrisma);
415+
416+
// Shard "z" is not in the configured map; shard "a" is.
417+
const unconfiguredId = generateRunOpsIdV2("z");
418+
const legacyMemberId = generateLegacyCuid();
419+
420+
await seedNewMember(
421+
shardPrisma,
422+
{ envId: ctx.environment.id, orgId: ctx.organization.id, projectId: ctx.project.id },
423+
{ id: unconfiguredId, friendlyId: "run_unconfigured", status: "COMPLETED_SUCCESSFULLY" }
424+
);
425+
await seedLegacyMember(legacyPrisma, ctx, {
426+
id: legacyMemberId,
427+
friendlyId: "run_legacy_member",
428+
status: "COMPLETED_SUCCESSFULLY",
429+
});
430+
431+
const batchFriendlyId = "batch_gen2_unconfigured";
432+
await seedBatchOnNew(newPrisma, ctx.environment.id, batchFriendlyId, [
433+
unconfiguredId,
434+
legacyMemberId,
435+
]);
436+
437+
// A closure-based recorder, not a mock: it records the id sets each store is asked for,
438+
// so the assertion is about real reads rather than about a test double's behaviour.
439+
const askedOf = (label: string, target: RunOpsPrismaClient | PrismaClient) => {
440+
const asked: string[][] = [];
441+
const handle = {
442+
...target,
443+
taskRun: {
444+
findMany: (args: { where?: { id?: { in?: string[] } } }) => {
445+
asked.push(args.where?.id?.in ?? []);
446+
return (target as unknown as PrismaReplicaClient).taskRun.findMany(args as never);
447+
},
448+
},
449+
} as unknown as PrismaReplicaClient;
450+
return { label, asked, handle };
451+
};
452+
const genOneNew = askedOf("new", newPrisma);
453+
const legacy = askedOf("legacy", legacyPrisma);
454+
455+
const presenter = new ApiBatchResultsPresenter(throwingPrisma, throwingPrisma, {
456+
splitEnabled: true,
457+
newClient: genOneNew.handle,
458+
legacyReplica: legacy.handle,
459+
shardReplicas: new Map([["a", shardPrisma as unknown as PrismaReplicaClient]]),
460+
});
461+
462+
const result = await presenter.call(batchFriendlyId, env(ctx));
463+
464+
// The legacy member still resolves; the unconfigured gen-2 member is dropped.
465+
expect(result).toBeDefined();
466+
expect(result!.items).toHaveLength(1);
467+
expect(result!.items[0]).toMatchObject({ ok: true, id: "run_legacy_member" });
468+
469+
// The unconfigured id was never asked of a gen-1 store.
470+
for (const store of [genOneNew, legacy]) {
471+
for (const ids of store.asked) {
472+
expect(ids).not.toContain(unconfiguredId);
473+
}
474+
}
475+
},
476+
180_000
416477
);
417478
});

0 commit comments

Comments
 (0)