Repository navigation
Idempotency bug audit: 13 reproduced bugs, issues to be opened after the next release #8603
leandrodamascena
started this conversation in
Maintainers discussions
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Same exercise as #8576, now for the Idempotency utility. I read the whole package, wrote a reproduction for every suspect and ran all of them (in-memory persistence layer, a fake Redis client, and
botocore.stub.Stubberfor DynamoDB). Everything below reproduces ondevelop. None has an open issue yet; the two Redis bugs I found earlier are already tracked in #8574 and #8575.I'm holding these here on purpose: I'll open the individual issues after the next release, so we don't flood the queue while #8577-#8584 and #8586-#8601 are still open. All of them are candidates for
good first issueexcept number 3.Severity is my call: High gives wrong results or duplicate execution in normal use, Medium breaks a documented feature or leaves bad state behind, Low is an edge case.
High
1. Payload validation does not work with the Redis/Cache persistence layer
persistence/redis.py:489(_update_record)_update_recordrewrites the whole JSON value with onlydata,statusandexpiration, dropping thevalidationhash that_put_in_progress_recordstored. On replay_get_recordyieldspayload_hash="", and_validate_payloadraises for the same payload.The existing test only asserts the tampered-payload path. Fix: write
validation_key_attrin_update_recordwhen validation is enabled, likedynamodb.py:312does. GFI: yes.2. Falsy results (
0,False,"",[],{}) are replayed asNonebase.py:274serialized_response = self.output_serializer.from_dict(response_dict) if response_dict else Noneuses truthiness. The record is stored correctly (data="0","[]","{}"...), only the replay is wrong. #8176 fixed the write side and promised falsy values are returned on replay; the read side still has the old check from #5251.Fix:
if response_dict is not None. Add a parametrised replay test for{},0,False,"",[]. GFI: yes.3. One persistence layer instance shared by two decorated functions corrupts records and can execute twice
persistence/base.py:95-116(configure) andbase.py:118(called on every invocation)configure()overwritesfunction_nameon every call but ignores everything else after the first (if self.configured: return). The docs define one module-levelDynamoDBPersistenceLayerand show both decorators, so this pattern is natural:After the first call the store has
INPROGRESS handler#...andCOMPLETED process#...twice: the handler's result was saved under the inner function's name. The handler stays INPROGRESS until the in-progress expiry and then runs again. If the inner function raises,delete_recordalso targets the wrong key. With two unrelated functions and two differentIdempotencyConfigs on one layer, the second config (event_key_jmespath,payload_validation_jmespath,expires_after_seconds,use_local_cache) is silently ignored.Fix: stop keeping per-function state on the layer. Pass
function_name/key_prefixfromIdempotencyHandlerinto the key computation, or snapshot the config per handler. At minimum raise whenconfigure()is called with a different config and document "one layer instance per decorated function". This one needs a maintainer. GFI: no.Medium
4. A result that is not JSON serializable leaves the INPROGRESS record behind
base.py:298-305The function already ran,
save_successfails ondatetime/set/bytes, the error is wrapped asIdempotencyPersistenceLayerErrorand the INPROGRESS record is never deleted. Withoutregister_lambda_contextthere is no in-progress expiry, so the key is locked forexpires_after_seconds(1 h default).Fix: call
delete_recordbefore re-raising whensave_successfails, mirroring the exception path. GFI: yes.5.
output_serializer=PydanticSerializer(class form) crashes underfrom __future__ import annotationsidempotency.py:211andserialization/pydantic.py:44function.__annotations__["return"]is a string under PEP 563, andissubclass(str, BaseModel)raisesTypeError: issubclass() arg 1 must be a classat decoration time. Same for-> list[Model]. The repo itself uses future annotations everywhere.Fix:
typing.get_type_hints(function).get("return")inidempotent_function, and aninspect.isclassguard raisingIdempotencyModelTypeError. GFI: yes.6.
DataclassSerializercannot savedatetime/UUIDfields and does not rebuild nested dataclassesserialization/dataclass.py:33-37to_dictreturnsasdict()unchanged andsave_successdumps it with the plainEncoder, which has no datetime/UUID support (same class of bug as #8569, fixed for Pydantic). Andfrom_dictdoesself.__model(**data), so nested dataclasses come back as dicts on replay.Fix: make
to_dictproduce JSON-safe values and rebuild nested dataclasses fromdataclasses.fields()type hints infrom_dict, or document the limitation. GFI: yes.7. DynamoDB composite key:
DataRecord.idempotency_keyis the static partition key valuepersistence/dynamodb.py:165With
sort_key_attr,_item_to_data_recordsetsidempotency_key=data[self.key_attr], which isstatic_pk_valuefor every record. Consequences: the local cache stores every record under the same key souse_local_cachenever hits for records written by other containers, theresponse_hookreceivesidempotency_key='idempotency#myfn'(the docs example reads it), and theIdempotencyAlreadyInProgressErrormessage shows the static value.Fix:
idempotency_key=data[self.sort_key_attr] if self.sort_key_attr else data[self.key_attr]. GFI: yes.Low
8. A dataclass payload with
set/frozenset/bytesfields cannot be hashedpersistence/base.py:41-55_IdempotencyKeyEncoderhandles Enum, date, UUID and Path only.f(p=P(1, {"a"}))fails withIdempotencyPersistenceLayerError: Failed to save in progress record ... Object of type set is not JSON serializable, which reads like a store outage. Fix: encodeset/frozensetas a sorted list andbytesas base64. GFI: yes.9. Redis
_orphan_lock_timeout = min(10, expires_after_seconds)is always 10persistence/redis.py:321Computed in
__init__, beforeconfigure()setsexpires_after_seconds, so it always sees the 3600 default. Withexpires_after_seconds=2the lock outlives the record by 8 s and a request in that window that finds an orphan fails withIdempotencyInconsistentStateError. Fix: compute it in_acquire_lock. GFI: yes.10. Redis: a record expiring between
SET NXandGETfails the invocation instead of retryingpersistence/redis.py:415_get_recordraisesIdempotencyItemNotFoundError, which escapes_put_recordand is wrapped asIdempotencyPersistenceLayerError. DynamoDB handles the same race throughIdempotencyInconsistentStateErrorand the retry loop. Fix: catch it there and raiseIdempotencyInconsistentStateError. GFI: yes.11.
POWERTOOLS_IDEMPOTENCY_DISABLED=""or any non-boolean value crashes every invocationidempotency.py:109, 219strtobool("")raisesValueError: invalid truth value ''."0","no","False"work. Fix: treat unparsable values as not disabled. GFI: yes.12.
IdempotencyConfigdocstring sayslocal_cache_max_itemsdefaults to 1024; it is 256config.py. Docs table already says 256. GFI: yes.Not reported as bugs, noting for the record
idempotent_function:Decimal("2.5")→"2.5",(1, 2)→[1, 2]. Same as Lambda's own JSON behaviour, but the docs don't warn for function results."",[],{}fromevent_key_jmespathare treated as "missing key" and skip idempotency with a warning, while the docs only mentionNone.boto3_supports_condition_check_failureraisesValueErroron versions like1.40.0rc1. Source installs only.IdempotencyConfig(expires_after_seconds=0)makes Redis rejectex=0; DynamoDB tolerates it. No validation on either side.All reactions