aws / aws/aws-durable-execution-sdk-js
Checkpoint promises never settle when a batch fails; large-result path can hang
- Dominant language
- TypeScript
- Stars
- 84
- Forks
- 28
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 43
Description
### Summary
When a checkpoint batch fails, the `checkpoint()` promises for every item in that batch never settle — they are neither resolved nor rejected. Every fire-and-forget call site is unaffected, and every `await checkpoint(...)` inside the customer handler is unaffected in practice because `Promise.race` against the termination promise abandons the handler. The one place where it is not survivable is the large-result path in `runHandler`, which runs *after* the race has settled and so has nothing left to abandon it: the `await` there can hang until the Lambda times out.
Split out of #870, which fixed the adjacent case (a batch failure during the final queue drain being reported as SUCCEEDED) and documented this one as out of scope.
### Why the promises never settle
`processQueue` removes the batch from the queue *before* sending it (`checkpoint-manager.ts:408`):
```ts
if (consumed > 0) {
this.queue = this.queue.slice(consumed);
}
```
On success each item is resolved (`checkpoint-manager.ts:424`):
```ts
batch.forEach((item) => {
item.resolve();
});
```
The failure path (`checkpoint-manager.ts:437`) never touches `batch`. It classifies the error, clears the queue and terminates:
```ts
} catch (error) {
const checkpointError = this.classifyCheckpointError(error);
this.clearQueue();
this.terminationManager.terminate({
reason: TerminationReason.CHECKPOINT_FAILED,
...
});
}
```
`clearQueue()` is explicit that it does not settle anything, and by then the failing batch is not in `this.queue` anyway (`checkpoint-manager.ts:217`):
```ts
public clearQueue(): void {
// Silently clear queue - we're terminating so no need to reject promises
this.queue = [];
```
So both the in-flight batch and anything still queued are dropped with their promises pending forever. Nothing else settles them: `terminate()` resolves the termination promise and sets the terminating flag, neither of which touches a queued item.
### Where that becomes a hang
`runHandler` checkpoints a result that exceeds the response size limit and awaits it (`with-durable-execution.ts:307`, `:317`):
```ts
if (serializedResult && serializedSize > LAMBDA_RESPONSE_SIZE_LIMIT) {
...
try {
await durableExecution.checkpointManager.checkpoint(stepId, {
Id: stepId,
Action: "SUCCEED",
Type: OperationType.EXECUTION,
Payload: serializedResult,
});
```
This is past `await Promise.race([handlerPromise, terminationPromise])` at `:214`. Earlier awaited checkpoints are reachable only from handler code, where a hang is harmless because the termination promise wins the race and the handler is abandoned. Here the race has already settled, so a failed batch leaves this `await` with no observer and no timeout — the invocation stalls rather than failing fast, and the result is lost when the platform eventually times it out.
The drain immediately below it carries the same reasoning #870 corrected elsewhere (`with-durable-execution.ts:335`):
```ts
// Continue anyway - the checkpoint will be retried on next invocation
```
There is no next invocation on this path: a result is being returned, so `SUCCEEDED` at `:349` is the end of the execution.
### Reproduction sketch
Composed-test shape, no mocking of `CheckpointManager` needed:
1. Real `withDurableExecution` with a transport that succeeds until the handler returns, then fails with a 5xx (see `checkpoint-failure.composed.test.ts` for the client and `invoke()` helpers).
2. Handler returns a value larger than `LAMBDA_RESPONSE_SIZE_LIMIT`.
3. The invocation neither resolves nor rejects.
### Possible directions
Two that seem plausible, both larger than they first look:
- **Settle the batch on failure.** Reject each item in the failing batch with the classified error. This is the honest fix but it changes an established idiom: several call sites deliberately rely on a checkpoint promise never resolving during termination (`checkpoint()` itself returns `new Promise(() => {})` when `isTerminating` or when an ancestor has finished). Rejecting would surface unhandled rejections at call sites that currently expect to be abandoned, so it likely needs to be conditional on not already terminating.
- **Have the large-result path stop relying on the await.** Race the checkpoint against `waitForQueueCompletion()` and then consult the batch-failure accessor added in #870, rather than assuming the await will settle. Narrower, and it leaves the general contract as-is.
Happy to send a PR for either once there's a preference.
Contributor guide
Research direction
Start with processQueue and clearQueue in checkpoint-manager.ts, then read the large-result checkpoint branch in with-durable-execution.ts. Run checkpoint-failure.composed.test.ts using its client and invoke() helpers. Done means the large-result invocation no longer hangs when the checkpoint transport fails, with the chosen failure behavior covered by a regression test.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100