fix: preserve empty errors in state and history by zhongkechen · Pull Request #870 · aws/aws-durable-execution-sdk-python

zhongkechen · 2026-10-27T22:45:59Z

Issue #, if available

Fixes #862.

Description of changes

A present empty error was lost when SDK operation details were decoded or
reloaded from storage. Empty callback errors could therefore differ between
live memory, saved state, and operations reconstructed from history.

Preserve Error: {} in callback, step, child-context, and chained-invoke
details through decoding and persistence. The local runner retains that same
empty object in live and reconstructed state. Detailed history keeps
Error.Payload: {}; metadata-only history omits the payload and retains its
truncation flag.

Plugin operation information treats an object with no fields as absent
diagnostics without changing stored state. Typed replay exceptions use the
existing default or caller-supplied fallback for that object. Explicit fields,
including empty strings and empty stack-trace lists, remain intact.

Design

The cause was using truthiness for two different questions: whether an error
object is present in stored state and whether it contains diagnostic fields.
Use presence-based decoding for stored errors. Check diagnostic contents only
in plugin.py::_extract_error() and
CheckpointedResult.raise_operation_error().
Normalizing state to None and recreating an empty history payload would lose
backend state fidelity. This change removes that normalization helper and the
synthetic history payload branch.
Preserve the existing handling of falsey detail containers while extracting
plugin errors; the wire-decoder regression also covers empty invoke details.
This implements the
maintainer-confirmed design.
No public API members or configuration options are added.
OTel status mapping and exception-event timing remain in fix: correct terminal telemetry status and timing #865.

Validation

Regression runs against the preceding PR head 5defff7:

Regression set	Failed before fix	Passed controls
Core decoding and wire/persistence round trips	12	28
Plugin diagnostics and typed exception fallbacks	10	60
Runner history, live callback state, and resume	13	78

Integrated validation:

Full core and OTel suites: 3,039 passed, plus 24 subtests.
Full testing-library suite: 1,839 passed.
After the final detail-container compatibility fix, all changed test modules
together: 760 passed, plus 24 subtests.
Core and testing type checks passed: 96 and 137 source files respectively.
Both packages' hatch fmt --check and git diff --check passed.
Full JS example validation was attempted with two workers against the existing
built checkout at bcb41eba9b41bfec2f7e6938b3355503d69fcdc6 (--no-build).
Execution-timeout failures were logged, and the run did not complete or
produce its final Jest/report artifacts. Full JS compatibility is not yet
qualified for this revision.
Local Python validation used Python 3.13.5 and Hatch 1.17.0.

The public callback test checks memory and filesystem stores, captures the live
operation before persistence, reloads saved state, and reconstructs detailed
history. It also verifies the resumed invocation's plugin snapshot, the default
callback exception message, and a completed step running exactly once.
Parameterized tests retain exact result-wrapper checks and persist reconstructed
errors, including {}, empty string fields, and empty stack-trace lists.

Scope decisions: operation persistence checks cover every present error,
including {}; absent-error history reconstruction is checked directly. The
existing decoding of an empty ChainedInvokeDetails: {} container remains
outside this error-presence fix. The callback integration test reads the existing
invocation-start snapshot so it does not depend on separate callback
UpdatedOperationIds delivery work.

Changed expectations in the PR-added tests:
test_failure_history_reconstruction_preserves_error_presence and
test_live_callback_failure_preserves_error_through_persistence now require a
present empty error instead of None.
test_callback_history_projection_preserves_state_and_data_inclusion no longer
expects an invented payload for an actually absent error.

Assumptions and limits

The backend representation follows the maintainer's confirmation linked above.
No new deployed Lambda run was performed, so this PR does not claim independent
cloud conformance qualification. Shared conformance requirements and handlers
are unchanged.

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

Preserve a present empty ErrorObject in callback, step, context, and chained-invoke details by checking presence instead of truthiness. Keep the matching history payload decoder fix and remove the testing library's completion/reconstruction normalization and synthetic payload. Add core decoding and round-trip regressions, and require live callback errors to survive memory and filesystem persistence with the existing default CallbackExternalError message. The focused regressions fail 25 cases on the previous PR revision and pass all 131 with this fix.

yaythomas · 2026-10-28T22:10:14Z

thanks! preserving Error: {} through decoding and persistence is intent here, and it matches the JavaScript SDK and the service.

The preserved empty object is currently treated as if it contains error details. As a result, plugins receive an empty ErrorObject, and step, context, and invoke exceptions have message=None. Passing that empty object to plugins does not match JavaScript. It causes OTel to record an exception with no message or type. Using that empty object to create an exception also gives the exception message=None.

Can we keep the current decoder changes, but treat an ErrorObject with no fields like a missing error when building plugin information and exception messages? Concretely, this should only require plugin.py::_extract_error() and state.py::CheckpointedResult.raise_operation_error() to treat not error.to_dict() the same as error is None; the decoder changes can stay unchanged?

Revert 58567dc to restore the original testing-library solution. Normalize empty callback and reconstructed history errors to absent SDK diagnostics while preserving empty payloads in detailed callback history. Restore the original regression expectations and README. Keep the later main-branch merges and unrelated fixes intact.

zhongkechen · 2026-10-28T22:56:15Z

thanks! preserving Error: {} through decoding and persistence is intent here, and it matches the JavaScript SDK and the service.

The preserved empty object is currently treated as if it contains error details. As a result, plugins receive an empty ErrorObject, and step, context, and invoke exceptions have message=None. Passing that empty object to plugins does not match JavaScript. It causes OTel to record an exception with no message or type. Using that empty object to create an exception also gives the exception message=None.

Can we keep the current decoder changes, but treat an ErrorObject with no fields like a missing error when building plugin information and exception messages? Concretely, this should only require plugin.py::_extract_error() and state.py::CheckpointedResult.raise_operation_error() to treat not error.to_dict() the same as error is None; the decoder changes can stay unchanged?

Thanks for the clarification. This PR will focus on the OTel behavior fixes: marking failed operations as ERROR even when diagnostic details are absent, and avoiding empty exception events.

I’ll retain the existing core SDK error-decoding behavior here and leave changes to those semantics for future PRs.

SilanHe · 2026-10-28T23:24:08Z

+            assert reconstructed.callback_details.error == (
+                expected_error if include_data else None
+            )
+        assert history.to_dict() == wire


wire was built from history.to_dict() a few lines up, so this can't fail. Drop it, or snapshot wire before decoding/reconstructing if the goal is to show those steps don't mutate history.

SilanHe · 2026-10-28T23:24:08Z

+        assert history.to_dict() == wire
+
+    assert live.callback_details is not None
+    assert live.callback_details.error == expected_error


Nit: this repeats the loop assertion above (live is already in that tuple).

SilanHe · 2026-10-28T23:24:08Z

+    expected_wire = history.to_dict()
+    if decode_wire:
+        history = GetDurableExecutionHistoryResponse.from_dict(expected_wire)
+    assert history.to_dict() == expected_wire


With decode_wire=False, this compares history.to_dict() with itself, so it can't fail. Same for line 146.

SilanHe · 2026-10-28T23:24:08Z

+    if decode_wire:
+        history = GetDurableExecutionHistoryResponse.from_dict(expected_wire)
+    assert history.to_dict() == expected_wire
+    expected_error = error if payload else None


The previous revision also round-tripped the reconstructed operation through Operation.from_json_dict. That was the only check that a reconstructed op with e.g. {"ErrorMessage": ""} or {"StackTrace": []} survives persistence unchanged. Worth keeping, at least for the non-empty payloads?

SilanHe · 2026-10-28T23:24:08Z

+    )
+    result = DurableFunctionTestResult.from_execution_history(execution, history)
+    result_operation = result.get_operation_by_name("failed-operation")
+    assert isinstance(result_operation, (ContextOperation, InvokeOperation))


StepOperation and CallbackOperation both subclass ContextOperation, so this passes even if an event maps to the wrong wrapper. Could we add the expected wrapper class to the parametrize tuple and assert the exact type?

SilanHe · 2026-10-29T00:28:38Z

thanks! preserving Error: {} through decoding and persistence is intent here, and it matches the JavaScript SDK and the service.

The preserved empty object is currently treated as if it contains error details. As a result, plugins receive an empty ErrorObject, and step, context, and invoke exceptions have message=None. Passing that empty object to plugins does not match JavaScript. It causes OTel to record an exception with no message or type. Using that empty object to create an exception also gives the exception message=None.

Can we keep the current decoder changes, but treat an ErrorObject with no fields like a missing error when building plugin information and exception messages? Concretely, this should only require plugin.py::_extract_error() and state.py::CheckpointedResult.raise_operation_error() to treat not error.to_dict() the same as error is None; the decoder changes can stay unchanged?

Thanks for the clarification. This PR will focus on the OTel behavior fixes: marking failed operations as ERROR even when diagnostic details are absent, and avoiding empty exception events.

I’ll retain the existing core SDK error-decoding behavior here and leave changes to those semantics for future PRs.

Do we not need the recommended plugin.py::_extract_error() and state.py::CheckpointedResult.raise_operation_error() changes at all?

zhongkechen · 2026-10-29T02:21:50Z

Do we not need the recommended plugin.py::_extract_error() and state.py::CheckpointedResult.raise_operation_error() changes at all?

Created a separate issue to track that #880

Require exact callback, step, context, and invoke result wrappers. Restore operation persistence checks for populated error payloads, including explicit empty strings and stack-trace lists. Remove redundant history comparisons and duplicate live callback assertions.

yaythomas · 2026-10-29T03:00:07Z

@zhongkechen Oh, sorry, I think we misunderstood each other. I see that the latest revision now changes an empty ErrorObject to None when the callback completes. It stores None in operation state, creates {} only for detailed history, and changes that {} back to None when rebuilding operation state.

I meant to keep the previous changes that preserved {}. The backend keeps Error: {} in operation state and detailed history. So the local runner should keep {} in live, saved, and reconstructed operation state too. Only metadata-only history should omit the payload.

My plugin and exception comments described how those consumers should handle the preserved {}. I did not mean that the local runner should replace {} with None.

I was also not asking to move OTel status work into this PR. Can we restore the previous {}-preservation changes and keep this PR focused on matching the backend?

yaythomas · 2026-10-29T03:00:08Z

@SilanHe Yes, we still need both changes.

The latest revision only makes them unnecessary because it changes {} to None before those methods run. I meant to keep {} in operation state so the local runner matches the backend.

Once we restore that behavior, _extract_error() should return no plugin error when {} has no fields. raise_operation_error() should use the default exception message when {} has no message.

yaythomas

Please address the backend-parity issue discussed above.

Decode present empty errors in callback, step, context, and chained-invoke operation details without collapsing them to None. Retain omitted/null errors and explicit empty diagnostic fields. Add presence and operation round-trip regressions. Before the fix, the new cases report 12 failures and 28 passes; after it, all 247 tests in lambda_service_test.py pass.

Keep checkpointed empty errors intact while omitting diagnostics from plugin snapshots and terminal hooks. Use the existing typed operation error fallback for empty checkpoint errors, preserving supplied empty fields and custom fallback messages.

Keep the existing truthiness check for operation detail containers while normalizing empty diagnostic objects. Verify real wire-decoded empty invoke details keep direct, invocation, and terminal plugin information.

Keep callback errors and reconstructed history errors intact instead of normalizing them to None and synthesizing detailed history payloads. Retain metadata-only omission and explicit empty diagnostic fields. Require empty errors to survive live, persisted, and reconstructed state. Verify resumed plugin snapshots omit empty diagnostics while the handler keeps its callback fallback message and completed steps stay skipped. The updated regression suite fails 13 cases on the previous PR head.

github-actions · 2026-10-29T04:06:08Z

Codex AI review

No actionable findings in the PR changes or against CONTRIBUTING.md rules 1–9. Tests were not executed during this read-only review. Residual risk: the PR reports incomplete JS compatibility validation and no deployed Lambda validation.

Reviewed commit 1709ab7e97fd21679566b2137346161c039502d7. Workflow run

zhongkechen · 2026-10-29T04:08:47Z

Thanks for clarifying, and sorry for the confusion. I’ve updated the PR accordingly.

zhongkechen added 2 commits October 27, 2026 15:42

test: cover empty callback failure diagnostics

51ac65f

fix: preserve empty callback error history

d69e398

zhongkechen mentioned this pull request Oct 27, 2026

fix: export visible replay completions #869

Draft

Merge branch 'main' into codex/fix-862-empty-callback-errors

b43593b

zhongkechen marked this pull request as ready for review October 28, 2026 19:08

Merge branch 'main' into codex/fix-862-empty-callback-errors

18dbfda

zhongkechen deployed to ai-pr-review-runtime October 28, 2026 19:08 — with GitHub Actions Active

zhongkechen deployed to ai-pr-review-runtime October 28, 2026 19:09 — with GitHub Actions Active