aws / aws/aws-durable-execution-sdk-js

Checkpoint promises never settle when a batch fails; large-result path can hang

Open
#871 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.