Skip to content

Commit 984e418

Browse files
authored
fix(sdk): remove 3-second timeout from checkpoint queue completion (#368)
1 parent 50ae0aa commit 984e418

File tree

4 files changed

+8
-108
lines changed

4 files changed

+8
-108
lines changed

packages/aws-durable-execution-sdk-js-examples/src/examples/run-in-child-context/checkpoint-size-limit/run-in-child-context-checkpoint-size-limit.test.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ createTests({
66
name: "run-in-child-context-checkpoint-size-limit boundary test",
77
functionName: "run-in-child-context-checkpoint-size-limit",
88
handler,
9-
// localRunnerConfig: {
10-
// checkpointDelay: 100,
11-
// },
9+
localRunnerConfig: {
10+
checkpointDelay: 100,
11+
},
1212
tests: (runner, { assertEventSignatures }) => {
1313
it("should handle 100 iterations near checkpoint size limit", async () => {
1414
const execution = await runner.run();
@@ -20,15 +20,12 @@ createTests({
2020

2121
// Verify totalIterations matches actual operations created
2222
expect(result.totalIterations).toBe(
23-
// TODO: https://github.com/aws/aws-durable-execution-sdk-js/issues/365
24-
// execution.getOperations({
25-
// status: "SUCCEEDED",
26-
// }).length,
27-
execution.getOperations().length,
23+
execution.getOperations({
24+
status: "SUCCEEDED",
25+
}).length,
2826
);
2927

30-
// TODO: https://github.com/aws/aws-durable-execution-sdk-js/issues/365
31-
// assertEventSignatures(execution.getHistoryEvents(), historyEvents);
28+
assertEventSignatures(execution.getHistoryEvents(), historyEvents);
3229
}, 120000);
3330
},
3431
});

packages/aws-durable-execution-sdk-js/src/utils/checkpoint/checkpoint-manager.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ export class CheckpointManager implements Checkpoint {
4141
reject: (error: Error) => void;
4242
}> = [];
4343
private queueCompletionResolver: (() => void) | null = null;
44-
private queueCompletionTimeout: NodeJS.Timeout | null = null;
4544
private readonly MAX_PAYLOAD_SIZE = 750 * 1024; // 750KB in bytes
4645
private isTerminating = false;
4746
private static textEncoder = new TextEncoder();
@@ -152,17 +151,8 @@ export class CheckpointManager implements Checkpoint {
152151
return;
153152
}
154153

155-
return new Promise<void>((resolve, reject) => {
154+
return new Promise<void>((resolve) => {
156155
this.queueCompletionResolver = resolve;
157-
158-
// Set a timeout to prevent infinite waiting
159-
this.queueCompletionTimeout = setTimeout(() => {
160-
this.queueCompletionResolver = null;
161-
this.queueCompletionTimeout = null;
162-
// Clear the queue since it's taking too long
163-
this.clearQueue();
164-
reject(new Error("Timeout waiting for checkpoint queue completion"));
165-
}, 3000); // 3 second timeout
166156
});
167157
}
168158

@@ -393,10 +383,6 @@ export class CheckpointManager implements Checkpoint {
393383

394384
private notifyQueueCompletion(): void {
395385
if (this.queueCompletionResolver) {
396-
if (this.queueCompletionTimeout) {
397-
clearTimeout(this.queueCompletionTimeout);
398-
this.queueCompletionTimeout = null;
399-
}
400386
this.queueCompletionResolver();
401387
this.queueCompletionResolver = null;
402388
}

packages/aws-durable-execution-sdk-js/src/utils/checkpoint/checkpoint-queue-completion.test.ts

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -67,58 +67,6 @@ describe("CheckpointManager Queue Completion", () => {
6767

6868
await expect(waits).resolves.toEqual([undefined, undefined, undefined]);
6969
});
70-
71-
it("should timeout after 3 seconds if queue doesn't complete", async () => {
72-
jest.useFakeTimers();
73-
74-
const mockCheckpoint = jest.fn().mockImplementation(
75-
() => new Promise(() => {}), // Never resolves
76-
);
77-
(checkpointManager as any).storage = { checkpoint: mockCheckpoint };
78-
79-
// Add item to queue
80-
checkpointManager.checkpoint("test-step", {});
81-
82-
const waitPromise = checkpointManager.waitForQueueCompletion();
83-
84-
// Fast-forward time by 3 seconds
85-
jest.advanceTimersByTime(3000);
86-
87-
await expect(waitPromise).rejects.toThrow(
88-
"Timeout waiting for checkpoint queue completion",
89-
);
90-
91-
jest.useRealTimers();
92-
});
93-
94-
it("should clear queue on timeout", async () => {
95-
jest.useFakeTimers();
96-
97-
const mockCheckpoint = jest.fn().mockImplementation(
98-
() => new Promise(() => {}), // Never resolves
99-
);
100-
(checkpointManager as any).storage = { checkpoint: mockCheckpoint };
101-
102-
// Add items to queue
103-
checkpointManager.checkpoint("test-step-1", {});
104-
checkpointManager.checkpoint("test-step-2", {});
105-
106-
expect((checkpointManager as any).queue.length).toBe(2);
107-
108-
const waitPromise = checkpointManager.waitForQueueCompletion();
109-
110-
// Fast-forward time by 3 seconds
111-
jest.advanceTimersByTime(3000);
112-
113-
await expect(waitPromise).rejects.toThrow(
114-
"Timeout waiting for checkpoint queue completion",
115-
);
116-
117-
// Queue should be cleared
118-
expect((checkpointManager as any).queue.length).toBe(0);
119-
120-
jest.useRealTimers();
121-
});
12270
});
12371

12472
describe("clearQueue", () => {

packages/aws-durable-execution-sdk-js/src/with-durable-execution-queue-completion.test.ts

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -104,37 +104,6 @@ describe("withDurableExecution Queue Completion", () => {
104104
waitSpy.mockRestore();
105105
});
106106

107-
it("should handle waitForQueueCompletion timeout gracefully", async () => {
108-
// Mock waitForQueueCompletion to reject with timeout error after 3 seconds
109-
const waitSpy = jest
110-
.spyOn(CheckpointManager.prototype, "waitForQueueCompletion")
111-
.mockImplementation(
112-
() =>
113-
new Promise((_, reject) =>
114-
setTimeout(
115-
() =>
116-
reject(
117-
new Error("Timeout waiting for checkpoint queue completion"),
118-
),
119-
3000,
120-
),
121-
),
122-
);
123-
124-
const mockHandler = jest.fn().mockResolvedValue("success");
125-
const wrappedHandler = withDurableExecution(mockHandler);
126-
127-
const startTime = Date.now();
128-
await wrappedHandler(mockEvent, mockContext);
129-
const endTime = Date.now();
130-
131-
// Should complete within timeout period (3 seconds + buffer for test overhead)
132-
expect(endTime - startTime).toBeLessThan(7000);
133-
expect(waitSpy).toHaveBeenCalled();
134-
135-
waitSpy.mockRestore();
136-
}, 10000);
137-
138107
it("should handle waitForQueueCompletion errors gracefully", async () => {
139108
const waitSpy = jest
140109
.spyOn(CheckpointManager.prototype, "waitForQueueCompletion")

0 commit comments

Comments
 (0)