Skip to content

Commit 4498b5f

Browse files
committed
fix(testing-sdk): fix race condition where concurrent updates can cause duplicate invocations
1 parent c4af3ff commit 4498b5f

File tree

21 files changed

+419
-199
lines changed

21 files changed

+419
-199
lines changed

packages/aws-durable-execution-sdk-js-examples/src/examples/force-checkpointing/multiple-wait/force-checkpointing-multiple-wait.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export const handler = withDurableExecution(
1515
const results = await ctx.parallel([
1616
// Branch 1: Long-running operation that blocks termination
1717
async (branchCtx: DurableContext) => {
18+
await branchCtx.wait("wait-1-1", { seconds: 1 });
1819
return await branchCtx.step("long-running-step", async () => {
1920
await new Promise((resolve) => setTimeout(resolve, 10000));
2021
return "long-complete";

packages/aws-durable-execution-sdk-js-testing/src/checkpoint-server/handlers/__tests__/callbacks.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { CheckpointManager } from "../../storage/checkpoint-manager";
1414
import {
1515
createExecutionId,
1616
createCallbackId,
17+
createInvocationId,
1718
} from "../../utils/tagged-strings";
1819

1920
describe("callbacks handlers", () => {
@@ -28,6 +29,7 @@ describe("callbacks handlers", () => {
2829
executionManager.startExecution({
2930
executionId: mockExecutionId,
3031
payload: '{"test": "data"}',
32+
invocationId: createInvocationId(),
3133
});
3234

3335
const storage = executionManager.getCheckpointsByExecution(mockExecutionId);

packages/aws-durable-execution-sdk-js-testing/src/checkpoint-server/handlers/__tests__/checkpoint-handlers.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ import {
1212
processCheckpointDurableExecution,
1313
} from "../checkpoint-handlers";
1414
import { ExecutionManager } from "../../storage/execution-manager";
15-
import { createExecutionId } from "../../utils/tagged-strings";
15+
import {
16+
createExecutionId,
17+
createInvocationId,
18+
} from "../../utils/tagged-strings";
1619
import { encodeCheckpointToken } from "../../utils/checkpoint-token";
1720

1821
// Mock only external dependencies we can't control
@@ -28,6 +31,7 @@ describe("checkpoint handlers", () => {
2831
const invocationResult = executionManager.startExecution({
2932
executionId,
3033
payload: '{"test": "data"}',
34+
invocationId: createInvocationId(),
3135
});
3236

3337
const storage = executionManager.getCheckpointsByExecution(executionId);

packages/aws-durable-execution-sdk-js-testing/src/checkpoint-server/handlers/__tests__/execution-handlers.test.ts

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,25 @@ describe("execution handlers", () => {
2626
const startExecutionSpy = jest.spyOn(executionManager, "startExecution");
2727

2828
const payload = '{"test": "execution data"}';
29-
const result = processStartDurableExecution(payload, executionManager);
29+
const invocationId = "mock-invocation-id";
30+
const result = processStartDurableExecution(
31+
{
32+
payload,
33+
invocationId: createInvocationId(invocationId),
34+
},
35+
executionManager,
36+
);
3037

3138
expect(startExecutionSpy).toHaveBeenCalledWith({
3239
payload,
40+
invocationId: invocationId,
3341
executionId: expect.any(String),
3442
});
3543

3644
expect(result).toEqual({
3745
checkpointToken: expect.any(String),
3846
executionId: expect.any(String),
39-
invocationId: expect.any(String),
47+
invocationId: invocationId,
4048
operationEvents: expect.any(Array),
4149
});
4250
});
@@ -47,6 +55,7 @@ describe("execution handlers", () => {
4755
// First create an execution
4856
const payload = '{"test": "data"}';
4957
executionManager.startExecution({
58+
invocationId: createInvocationId(),
5059
executionId: createExecutionId("test-execution-id"),
5160
payload,
5261
});
@@ -56,14 +65,20 @@ describe("execution handlers", () => {
5665
"startInvocation",
5766
);
5867

68+
const executionId = createExecutionId("test-execution-id");
69+
const invocationId = createInvocationId("test-invocation-id");
5970
const result = processStartInvocation(
60-
"test-execution-id",
71+
{
72+
executionId,
73+
invocationId,
74+
},
6175
executionManager,
6276
);
6377

64-
expect(startInvocationSpy).toHaveBeenCalledWith(
65-
createExecutionId("test-execution-id"),
66-
);
78+
expect(startInvocationSpy).toHaveBeenCalledWith({
79+
executionId,
80+
invocationId,
81+
});
6782
expect(result).toEqual({
6883
checkpointToken: expect.any(String),
6984
executionId: createExecutionId("test-execution-id"),
@@ -79,13 +94,23 @@ describe("execution handlers", () => {
7994
throw new Error("Execution not found");
8095
});
8196

97+
const executionId = createExecutionId("non-existent-execution");
98+
const invocationId = createInvocationId("test-invocation-id");
99+
82100
expect(() =>
83-
processStartInvocation("non-existent-execution", executionManager),
101+
processStartInvocation(
102+
{
103+
executionId,
104+
invocationId,
105+
},
106+
executionManager,
107+
),
84108
).toThrow("Execution not found");
85109

86-
expect(startInvocationSpy).toHaveBeenCalledWith(
87-
createExecutionId("non-existent-execution"),
88-
);
110+
expect(startInvocationSpy).toHaveBeenCalledWith({
111+
executionId,
112+
invocationId,
113+
});
89114
});
90115
});
91116

@@ -94,7 +119,11 @@ describe("execution handlers", () => {
94119
// First create an execution and invocation
95120
const executionId = createExecutionId("test-execution-id");
96121
const invocationId = createInvocationId("test-invocation-id");
97-
executionManager.startExecution({ executionId, payload: "{}" });
122+
executionManager.startExecution({
123+
executionId,
124+
payload: "{}",
125+
invocationId: createInvocationId(),
126+
});
98127

99128
const completeInvocationSpy = jest.spyOn(
100129
executionManager,

packages/aws-durable-execution-sdk-js-testing/src/checkpoint-server/handlers/__tests__/state-handlers.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { OperationType, OperationAction } from "@aws-sdk/client-lambda";
22
import { processGetDurableExecutionState } from "../state-handlers";
33
import { ExecutionManager } from "../../storage/execution-manager";
4-
import { createExecutionId } from "../../utils/tagged-strings";
4+
import {
5+
createExecutionId,
6+
createInvocationId,
7+
} from "../../utils/tagged-strings";
58

69
describe("state handlers", () => {
710
let executionManager: ExecutionManager;
@@ -21,6 +24,7 @@ describe("state handlers", () => {
2124
executionManager.startExecution({
2225
executionId,
2326
payload: '{"test": "data"}',
27+
invocationId: createInvocationId(),
2428
});
2529

2630
const storage = executionManager.getCheckpointsByExecution(executionId);

packages/aws-durable-execution-sdk-js-testing/src/checkpoint-server/handlers/execution-handlers.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,21 @@ import {
88
ExecutionManager,
99
InvocationResult,
1010
} from "../storage/execution-manager";
11+
import {
12+
StartDurableExecutionRequest,
13+
StartInvocationRequest,
14+
} from "../worker-api/worker-api-request";
1115

1216
/**
1317
* Starts a durable execution. Returns the data needed for the handler invocation event.
1418
*/
1519
export function processStartDurableExecution(
16-
payload: string | undefined,
20+
params: StartDurableExecutionRequest,
1721
executionManager: ExecutionManager,
1822
): InvocationResult {
1923
return executionManager.startExecution({
20-
payload,
2124
executionId: createExecutionId(),
25+
...params,
2226
});
2327
}
2428

@@ -27,10 +31,10 @@ export function processStartDurableExecution(
2731
* in-progress execution.
2832
*/
2933
export function processStartInvocation(
30-
executionIdParam: string,
34+
params: StartInvocationRequest,
3135
executionManager: ExecutionManager,
3236
): InvocationResult {
33-
return executionManager.startInvocation(createExecutionId(executionIdParam));
37+
return executionManager.startInvocation(params);
3438
}
3539

3640
export function processCompleteInvocation(

packages/aws-durable-execution-sdk-js-testing/src/checkpoint-server/storage/__tests__/execution-manager.test.ts

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,11 @@ describe("execution-manager", () => {
115115
it("should create a new execution with the provided parameters", () => {
116116
// Test data
117117
const executionId = createExecutionId("test-execution-id");
118+
const invocationId = createInvocationId("test-invocation-id");
118119
const params: StartExecutionParams = {
119120
executionId,
120121
payload: '{"key":"value"}',
122+
invocationId,
121123
};
122124

123125
// Expected mock operation with properly typed id
@@ -144,9 +146,9 @@ describe("execution-manager", () => {
144146

145147
// Verify results
146148
expect(result).toEqual({
147-
checkpointToken: `encoded-{"executionId":"test-execution-id","token":"mocked-uuid","invocationId":"mocked-uuid"}`,
149+
checkpointToken: `encoded-{"executionId":"${executionId}","token":"mocked-uuid","invocationId":"${invocationId}"}`,
148150
executionId,
149-
invocationId: "mocked-uuid",
151+
invocationId,
150152
operationEvents: [
151153
{
152154
operation: mockInitialOperation,
@@ -167,6 +169,7 @@ describe("execution-manager", () => {
167169
const executionId = createExecutionId("test-execution-id");
168170
const params: StartExecutionParams = {
169171
executionId,
172+
invocationId: createInvocationId("test-invocation-id"),
170173
};
171174

172175
const initializeSpy = jest.spyOn(
@@ -185,38 +188,48 @@ describe("execution-manager", () => {
185188
it("should throw error if execution ID doesn't exist", () => {
186189
const nonExistentId = createExecutionId("non-existent");
187190

188-
expect(() => executionManager.startInvocation(nonExistentId)).toThrow(
191+
expect(() =>
192+
executionManager.startInvocation({
193+
executionId: nonExistentId,
194+
invocationId: createInvocationId("test-invocation-id"),
195+
}),
196+
).toThrow(
189197
"Could not start invocation for invalid execution non-existent",
190198
);
191199
});
192200

193201
it("should start a new invocation for an existing execution", () => {
194202
// First create an execution
195203
const executionId = createExecutionId("test-execution-id");
196-
executionManager.startExecution({ executionId });
204+
const invocationId = createInvocationId("test-invocation-id");
205+
executionManager.startExecution({ executionId, invocationId });
197206

198207
// Reset mocks to track new calls
199208
jest.clearAllMocks();
200-
(randomUUID as jest.Mock).mockReturnValue("new-invocation-uuid");
201209

210+
const newInvocationId = createInvocationId("new-invocation-id");
202211
// Start a new invocation
203-
const result = executionManager.startInvocation(executionId);
212+
const result = executionManager.startInvocation({
213+
executionId,
214+
invocationId: newInvocationId,
215+
});
204216

205217
expect(result).toBeDefined();
206218
expect(result.executionId).toBe(executionId);
207-
expect(result.invocationId).toBe("new-invocation-uuid");
219+
expect(result.invocationId).toBe(newInvocationId);
208220
expect(result.operationEvents).toBeInstanceOf(Array);
209221
expect(encodeCheckpointToken).toHaveBeenCalledWith({
210222
executionId,
211-
token: "new-invocation-uuid",
212-
invocationId: "new-invocation-uuid",
223+
token: expect.any(String),
224+
invocationId: newInvocationId,
213225
});
214226
});
215227

216228
it("should include all operations from the checkpoint storage", () => {
217229
// Create an execution with a mock operation
218230
const executionId = createExecutionId("test-execution-id");
219-
executionManager.startExecution({ executionId });
231+
const invocationId = createInvocationId("test-invocation-id");
232+
executionManager.startExecution({ executionId, invocationId });
220233

221234
// Get the storage and add some operations to it
222235
const storage = executionManager.getCheckpointsByExecution(executionId);
@@ -247,7 +260,11 @@ describe("execution-manager", () => {
247260
});
248261

249262
// Start a new invocation
250-
const result = executionManager.startInvocation(executionId);
263+
const newInvocationId = createInvocationId("new-invocation-id");
264+
const result = executionManager.startInvocation({
265+
executionId,
266+
invocationId: newInvocationId,
267+
});
251268

252269
// Check that we got all operations
253270
expect(result.operationEvents).toHaveLength(2);
@@ -264,13 +281,20 @@ describe("execution-manager", () => {
264281
it("should throw error if execution is completed already", () => {
265282
// First create an execution
266283
const executionId = createExecutionId("test-execution-id");
267-
executionManager.startExecution({ executionId });
284+
const invocationId = createInvocationId("test-invocation-id");
285+
executionManager.startExecution({ executionId, invocationId });
268286

269287
jest
270288
.spyOn(CheckpointManager.prototype, "isExecutionCompleted")
271289
.mockReturnValue(true);
272290

273-
expect(() => executionManager.startInvocation(executionId)).toThrow(
291+
const newInvocationId = createInvocationId("new-invocation-id");
292+
expect(() =>
293+
executionManager.startInvocation({
294+
executionId,
295+
invocationId: newInvocationId,
296+
}),
297+
).toThrow(
274298
`Could not start invocation for completed execution ${executionId}`,
275299
);
276300
});
@@ -286,7 +310,8 @@ describe("execution-manager", () => {
286310

287311
it("should return the checkpoint storage for an existing execution ID", () => {
288312
const executionId = createExecutionId("test-execution-id");
289-
executionManager.startExecution({ executionId });
313+
const invocationId = createInvocationId("test-invocation-id");
314+
executionManager.startExecution({ executionId, invocationId });
290315

291316
const storage = executionManager.getCheckpointsByExecution(executionId);
292317

@@ -321,7 +346,8 @@ describe("execution-manager", () => {
321346
it("should return storage and token data for a valid token", () => {
322347
// Create an execution
323348
const executionId = createExecutionId("test-execution-id");
324-
executionManager.startExecution({ executionId });
349+
const invocationId = createInvocationId("test-invocation-id");
350+
executionManager.startExecution({ executionId, invocationId });
325351

326352
// Create token data that references the execution
327353
const tokenData: CheckpointTokenData = {
@@ -378,7 +404,8 @@ describe("execution-manager", () => {
378404
it("should return CheckpointManager for valid callback ID with existing execution", () => {
379405
// Create an execution first
380406
const executionId = createExecutionId("test-execution-id");
381-
executionManager.startExecution({ executionId });
407+
const invocationId = createInvocationId("test-invocation-id");
408+
executionManager.startExecution({ executionId, invocationId });
382409

383410
const callbackId = createCallbackId("valid-callback-id");
384411

@@ -398,7 +425,8 @@ describe("execution-manager", () => {
398425
it("should return the same CheckpointManager instance as getCheckpointsByExecution", () => {
399426
// Create an execution first
400427
const executionId = createExecutionId("test-execution-id");
401-
executionManager.startExecution({ executionId });
428+
const invocationId = createInvocationId("test-invocation-id");
429+
executionManager.startExecution({ executionId, invocationId });
402430

403431
const callbackId = createCallbackId("valid-callback-id");
404432

@@ -451,7 +479,7 @@ describe("execution-manager", () => {
451479
// Create an execution first
452480
const executionId = createExecutionId("test-execution-id");
453481
const invocationId = createInvocationId("test-invocation-id");
454-
executionManager.startExecution({ executionId });
482+
executionManager.startExecution({ executionId, invocationId });
455483

456484
const storage = executionManager.getCheckpointsByExecution(executionId);
457485
expect(storage).toBeDefined();
@@ -517,7 +545,7 @@ describe("execution-manager", () => {
517545
// Create an execution first
518546
const executionId = createExecutionId("test-execution-id");
519547
const invocationId = createInvocationId("test-invocation-id");
520-
executionManager.startExecution({ executionId });
548+
executionManager.startExecution({ executionId, invocationId });
521549

522550
const storage = executionManager.getCheckpointsByExecution(executionId);
523551
expect(storage).toBeDefined();
@@ -590,7 +618,7 @@ describe("execution-manager", () => {
590618
// Create an execution first
591619
const executionId = createExecutionId("test-execution-id");
592620
const invocationId = createInvocationId("test-invocation-id");
593-
executionManager.startExecution({ executionId });
621+
executionManager.startExecution({ executionId, invocationId });
594622

595623
const storage = executionManager.getCheckpointsByExecution(executionId);
596624
expect(storage).toBeDefined();

0 commit comments

Comments
 (0)