Skip to content

fix(idempotency): clear the in progress record when the response can't be serialized - #8740

Open
sumedh-aerram wants to merge 3 commits into
aws-powertools:developfrom
sumedh-aerram:fix/idempotency-delete-inprogress-on-save-failure
Open

sumedh-aerram wants to merge 3 commits into
aws-powertools:developfrom
sumedh-aerram:fix/idempotency-delete-inprogress-on-save-failure

Conversation

@sumedh-aerram

Copy link
Copy Markdown

Issue number:

closes #8721

Summary

Changes

When the decorated function returns a value that the output serializer or json.dumps can't convert, delete the in progress record before raising IdempotencyPersistenceLayerError, the same way the record is deleted when the function itself raises. Cleanup runs at both conversion points: the output serializer's to_dict in the handler, and json.dumps in BasePersistenceLayer.save_success. Both run before anything is written. The error type and message are unchanged.

A failure while writing the COMPLETED record still keeps the record, because the write may have landed and deleting it would let a retry run the function twice. New tests guard that boundary.

The "Handling exceptions" docs section and its example now describe the new failure mode. They say cleanup is attempted, retries wait for expiry if the deletion itself fails, and write failures keep the record.

User experience

Before: returning e.g. a datetime, set or bytes failed the invocation, but left the record INPROGRESS. Every retry with the same payload raised IdempotencyAlreadyInProgressError until the in progress expiry, or for expires_after_seconds (one hour by default) without register_lambda_context.

After: the invocation still fails with the same error, but the next call with the same payload runs the function again.

Known limitation, not introduced here: deletion is by key, without an ownership check. If a function outlives expires_after_seconds and another invocation reacquires the key, a cleanup can remove the other invocation's record. Upstream's existing cleanup for function exceptions already has this behaviour. I reproduced it on unchanged develop with a scripted replacement record (fake Redis client, public decorator API). This PR adds a new trigger to that existing cleanup, but no new mechanism. An ownership-aware delete (DynamoDB conditional delete, Redis compare-and-delete) seems worth a separate issue. I'm happy to open one if you agree.

Verification

AI assistants (Claude and Codex) helped implement and review this change. I ran the commands below and read the output.

  • New tests against the unchanged upstream implementation: the 6 serialization-cleanup cases fail. The 4 write-failure retention cases pass on both, since they are guards. With the fix, all 10 pass.
  • Counterfactual: widening cleanup to also cover a failed persistence write makes all 4 retention guards fail.
  • Idempotency unit and functional tests: 212 passed on Python 3.12. The functional idempotency suite also passed on Python 3.10, 3.11, 3.13 and 3.14 (run before the final, behaviour-preserving helper refactor; 3.12 was rerun after it).
  • Full gate on Python 3.12, make -k pr ty exited 0: 4012 passed, 73 skipped, 10 deselected, plus all 10 performance tests. Ruff, mypy (949 source files), ty, docs lint, security and complexity checks passed. An earlier gate flagged the handler module at complexity rank B; the serializer cleanup moved into a small helper and the gate was rerun clean.
  • Tests cover DynamoDB through botocore Stubber and Redis through the existing fake client, using only the public decorator API. The issue reproduces locally without AWS resources, so no live service was called.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

…t be serialized

When the decorated function returned a value the output serializer or
json.dumps could not convert (datetime, set, bytes, ...), the invocation
failed with IdempotencyPersistenceLayerError but the INPROGRESS record
stayed. Every retry with the same payload then raised
IdempotencyAlreadyInProgressError until the in progress expiry, or for
expires_after_seconds (one hour by default) without register_lambda_context.

Serialization fails before anything is written, so the record can be
deleted safely, the same way it is when the function raises. Clear it at
both serialization points: the output serializer's to_dict in the handler,
and json.dumps in BasePersistenceLayer.save_success. The error type and
message are unchanged.

A failure while writing the COMPLETED record still keeps the record: that
write may have landed, and deleting it would let a retry run the function
twice.
Add public decorator tests showing a failed write of the COMPLETED record
keeps the record, both when the write landed (the next call replays the
saved response) and when it did not (the next call stays in progress),
with and without the local cache. This guards the cleanup from widening
to write failures.

Say in the docs that the in progress record is only deleted if cleanup
succeeds, otherwise retries wait for it to expire, and that write failures
keep the record. Mention serialization cleanup in the exceptions example.
Keeps the idempotency handler module within the complexity rank checked
by make complexity-baseline.
@sumedh-aerram
sumedh-aerram requested a review from a team as a code owner November 1, 2026 22:03
@sumedh-aerram
sumedh-aerram requested a review from svozza November 1, 2026 22:03
@boring-cyborg boring-cyborg Bot added documentation Improvements or additions to documentation tests labels Nov 1, 2026
@powertools-for-aws-oss-automation powertools-for-aws-oss-automation Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 1, 2026
@sonarqubecloud

sonarqubecloud Bot commented Nov 1, 2026

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: a result that is not JSON serializable leaves the idempotency record stuck as INPROGRESS

1 participant