Skip to content

Commit e92bf68

Browse files
myftijaTrigger.dev RepoOps
authored andcommitted
fix(run-engine): apply a completion's metadata only after its snapshot check passes
Retrying an attempt completion the server had already applied re-ran the run metadata changes sent with it (increments, appends), because the metadata was applied before the completion's snapshot check. The metadata update now happens only after that check passes, so a replayed completion changes nothing. The stale-completion refusal keeps its 400 status and now also carries `code: "SNAPSHOT_CONFLICT"` and the run's latest snapshot id, so a worker can treat it as already delivered. Mono-RevId: 5ebfac0ebf95f35760ca6d04fbdf7bd9447d1207
1 parent f294229 commit e92bf68

5 files changed

Lines changed: 351 additions & 12 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Run metadata changes sent with an attempt's completion, such as increments and appends, are no longer applied a second time when a completion that already went through is retried.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { startTestServer, type TestServer } from "@internal/testcontainers/webapp";
2+
import { ApiClient } from "@trigger.dev/core/v3";
3+
import { SupervisorHttpClient } from "@trigger.dev/core/v3/runEngineWorker";
4+
import { setTimeout } from "node:timers/promises";
5+
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
6+
import {
7+
MANAGED_WORKER_SECRET,
8+
seedEngineFixtures,
9+
workerHeaders,
10+
type EngineFixtures,
11+
} from "../../test/bench/lib/engineFixtures";
12+
13+
const INSTANCE_NAME = "attempt-complete-e2e";
14+
15+
let server: TestServer;
16+
let fixtures: EngineFixtures;
17+
let api: ApiClient;
18+
let supervisor: SupervisorHttpClient;
19+
20+
beforeAll(async () => {
21+
server = await startTestServer({ overrideEnv: { RUN_ENGINE_WORKER_ENABLED: "1" } });
22+
fixtures = await seedEngineFixtures(server.prisma, { taskCount: 1 });
23+
api = new ApiClient(server.webapp.baseUrl, fixtures.environmentApiKey);
24+
supervisor = new SupervisorHttpClient({
25+
apiUrl: server.webapp.baseUrl,
26+
workerToken: fixtures.workerGroupToken,
27+
instanceName: INSTANCE_NAME,
28+
managedWorkerSecret: MANAGED_WORKER_SECRET,
29+
});
30+
}, 180_000);
31+
32+
afterAll(async () => {
33+
await server?.stop();
34+
}, 120_000);
35+
36+
describe("attempt completion replayed after it was applied", () => {
37+
it("refuses the replay with the latest snapshot and applies its metadata only once", async () => {
38+
const { runId, snapshotId } = await startAttempt();
39+
const completion = {
40+
ok: false,
41+
id: runId,
42+
error: { type: "BUILT_IN_ERROR", name: "UserError", message: "boom", stackTrace: "" },
43+
retry: { timestamp: Date.now(), delay: 0 },
44+
flushedMetadata: {
45+
data: JSON.stringify({ operations: [{ type: "increment", key: "count", value: 1 }] }),
46+
dataType: "application/json",
47+
},
48+
};
49+
50+
const first = await completeAttempt(runId, snapshotId, completion);
51+
expect(first.status).toBe(200);
52+
expect(first.body.result.attemptStatus).toBe("RETRY_IMMEDIATELY");
53+
await vi.waitFor(async () => expect(await metadataCount(runId)).toBe(1), {
54+
timeout: 5_000,
55+
interval: 50,
56+
});
57+
58+
const replay = await completeAttempt(runId, snapshotId, completion);
59+
await setTimeout(500);
60+
expect(await metadataCount(runId)).toBe(1);
61+
62+
expect(replay.status).toBe(400);
63+
expect(replay.body).toEqual({
64+
error: "Snapshot ID doesn't match the latest snapshot",
65+
code: "SNAPSHOT_CONFLICT",
66+
latestSnapshotId: first.body.result.snapshot.friendlyId,
67+
});
68+
}, 60_000);
69+
});
70+
71+
async function startAttempt() {
72+
const { id: runId } = await api.triggerTask(fixtures.taskIdentifiers[0]!, {
73+
payload: {},
74+
options: { maxAttempts: 3 },
75+
});
76+
77+
const dequeued = await vi.waitFor(
78+
async () => {
79+
const result = await supervisor.dequeue({});
80+
if (!result.success) throw new Error(result.error);
81+
const message = result.data.find((m) => m.run.friendlyId === runId);
82+
if (!message) throw new Error(`${runId} was not dequeued`);
83+
return message;
84+
},
85+
{ timeout: 10_000, interval: 100 }
86+
);
87+
88+
const started = await supervisor.startRunAttempt(runId, dequeued.snapshot.friendlyId, {});
89+
if (!started.success) throw new Error(started.error);
90+
91+
return { runId, snapshotId: started.data.snapshot.friendlyId };
92+
}
93+
94+
async function completeAttempt(runId: string, snapshotId: string, completion: unknown) {
95+
const response = await server.webapp.fetch(
96+
`/engine/v1/worker-actions/runs/${runId}/snapshots/${snapshotId}/attempts/complete`,
97+
{
98+
method: "POST",
99+
headers: workerHeaders(fixtures, INSTANCE_NAME),
100+
body: JSON.stringify({ completion }),
101+
}
102+
);
103+
104+
return { status: response.status, body: await response.json() };
105+
}
106+
107+
async function metadataCount(runId: string) {
108+
const { metadata } = await server.prisma.taskRun.findFirstOrThrow({
109+
where: { friendlyId: runId },
110+
select: { metadata: true },
111+
});
112+
return metadata ? JSON.parse(metadata).count : undefined;
113+
}

‎apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.attempts.complete.ts‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import { EngineServiceValidationError } from "@internal/run-engine";
12
import type { TypedResponse } from "@remix-run/server-runtime";
23
import { json } from "@remix-run/server-runtime";
4+
import { SnapshotId } from "@trigger.dev/core/v3/isomorphic";
35
import type { WorkerApiRunAttemptCompleteResponseBody } from "@trigger.dev/core/v3/workers";
46
import { WorkerApiRunAttemptCompleteRequestBody } from "@trigger.dev/core/v3/workers";
57
import { z } from "zod";
@@ -23,14 +25,33 @@ export const action = createActionWorkerApiRoute(
2325
const { completion, forceRequeue } = body;
2426
const { runFriendlyId, snapshotFriendlyId } = params;
2527

26-
const completeResult = await authenticatedWorker.completeRunAttempt({
27-
runFriendlyId,
28-
snapshotFriendlyId,
29-
completion,
30-
forceRequeue,
31-
runnerId,
32-
environmentId,
33-
});
28+
const completeResult = await authenticatedWorker
29+
.completeRunAttempt({
30+
runFriendlyId,
31+
snapshotFriendlyId,
32+
completion,
33+
forceRequeue,
34+
runnerId,
35+
environmentId,
36+
})
37+
.catch((error) => {
38+
if (
39+
error instanceof EngineServiceValidationError &&
40+
error.metadata?.code === "SNAPSHOT_CONFLICT" &&
41+
typeof error.metadata.latestSnapshotId === "string"
42+
) {
43+
// Stays a 400: the worker's client retries a 409, and a stale completion never applies.
44+
throw json(
45+
{
46+
error: error.message,
47+
code: error.metadata.code,
48+
latestSnapshotId: SnapshotId.toFriendlyId(error.metadata.latestSnapshotId),
49+
},
50+
{ status: 400 }
51+
);
52+
}
53+
throw error;
54+
});
3455

3556
return json({ result: completeResult });
3657
}

‎internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -733,8 +733,6 @@ export class RunAttemptSystem {
733733
runnerId?: string;
734734
environmentId?: string;
735735
}): Promise<CompleteRunAttemptResult> {
736-
await this.#notifyMetadataUpdated(runId, completion);
737-
738736
switch (completion.ok) {
739737
case true: {
740738
return this.attemptSucceeded({
@@ -794,13 +792,18 @@ export class RunAttemptSystem {
794792
);
795793

796794
if (latestSnapshot.id !== snapshotId) {
797-
throw new ServiceValidationError("Snapshot ID doesn't match the latest snapshot", 400);
795+
throw new ServiceValidationError("Snapshot ID doesn't match the latest snapshot", 400, {
796+
code: "SNAPSHOT_CONFLICT",
797+
latestSnapshotId: latestSnapshot.id,
798+
});
798799
}
799800

800801
if (latestSnapshot.executionStatus === "FINISHED") {
801802
throw new ServiceValidationError("Run is already finished", 400);
802803
}
803804

805+
await this.#notifyMetadataUpdated(runId, completion);
806+
804807
span.setAttribute("completionStatus", completion.ok);
805808
span.setAttribute("runId", runId);
806809

@@ -1001,13 +1004,18 @@ export class RunAttemptSystem {
10011004
);
10021005

10031006
if (latestSnapshot.id !== snapshotId) {
1004-
throw new ServiceValidationError("Snapshot ID doesn't match the latest snapshot", 400);
1007+
throw new ServiceValidationError("Snapshot ID doesn't match the latest snapshot", 400, {
1008+
code: "SNAPSHOT_CONFLICT",
1009+
latestSnapshotId: latestSnapshot.id,
1010+
});
10051011
}
10061012

10071013
if (latestSnapshot.executionStatus === "FINISHED") {
10081014
throw new ServiceValidationError("Run is already finished", 400);
10091015
}
10101016

1017+
await this.#notifyMetadataUpdated(runId, completion);
1018+
10111019
span.setAttribute("completionStatus", completion.ok);
10121020

10131021
//remove waitpoints blocking the run

0 commit comments

Comments
 (0)