Repository navigation
test: cover OTel replay and status mappings - #758
zhongkechen wants to merge 34 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
zhongkechen
changed the title
This comment has been minimized.
This comment has been minimized.
zhongkechen
changed the title
This comment has been minimized.
This comment has been minimized.
zhongkechen
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@hln33 Agreed. I’ve clarified the description in response to your coverage comment: case 24 verifies only the invocation That second mapping remains unit-covered, with the conformance gap tracked in aws/aws-durable-execution-conformance-tests#131. A real callback-failure path is being validated for the follow-up requirement; no additional case 25 coverage is claimed here. The description clarification does not close that coverage gap. |
|
Note on aws/aws-durable-execution-sdk-js#954 (merged 2026-10-07, fixes aws/aws-durable-execution-sdk-js#942). aws/aws-durable-execution-sdk-js#954 fixed a defect in the JS local test runner only. A deployed Lambda receives the event directly from the service. That path never builds the wrapper. So the conformance cases here don't depend on aws/aws-durable-execution-sdk-js#954, and they can't catch a regression of it. The regression test for aws/aws-durable-execution-sdk-js#954 lives in JS: Python's local tester doesn't have this defect. Its in-process invoker also wraps the input in |
Record local callback success, failure, and timeout updates for the next invocation. Deduplicate actual terminal update notifications within an invocation while preserving first delivery and clearing state between invocations. Keep checkpoint versions and error payload handling intact. Exercise public callback completion through both memory and file stores, including timeout, stored results, and two subsequent replays. Cover duplicate checkpoints, first notification, reset, and concurrent updates.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| end_timestamp=now if now is not None else real_now(), | ||
| callback_details=updated_callback_details, | ||
| ) | ||
| self._record_updated_operation(operation.operation_id) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Confirmed and fixed in 246cad8. Checkpoint delivery now consumes only callback update IDs covered by the existing delivery watermark, while retaining newer updates. Token/watermark values and error payloads are unchanged.
Six previously failing public-runner cases cover success, failure and actual timeout during a running submission step, then two later replays, on both memory and file stores. Rejected checkpoints, pure reads, cached/older deliveries and concurrent updates around delivery are covered as well. The affected testing/OTel/handler suite passed 2,149 tests; the additional boundary suite passed 98 tests.
This comment has been minimized.
This comment has been minimized.
Add the root callback completion/replay handler and both view mappings, plus aliases of the existing callback-failure handler for case 25. Exercise four real invocations, stored results, first-completion timing, raw terminal export counts, parentage, and two subsequent replays. Document the tester/core completion fixes and the remaining cloud no-error-details precondition. Keep the workflow and test references unchanged until the shared 26-case definitions are published together.
| end_timestamp=now if now is not None else real_now(), | ||
| callback_details=updated_callback_details, | ||
| ) | ||
| self._record_updated_operation(operation.operation_id) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
Prune update IDs covered by a successful checkpoint delivery while retaining later changes. Keep token and watermark values, pure reads, rejected requests, cached responses, and error payloads unchanged. Cover early success, failure and timeout during submission with real public APIs and two later replays on both stores. Exercise rejection, read, retry and concurrent update boundaries. Pin the reusable workflow and conformance definitions to the coordinated 26-case shared revision fb95e20acb0b9b10f5fc032ac521a4fd45efdd73.
zhongkechen
changed the title
| end_timestamp=now if now is not None else real_now(), | ||
| callback_details=updated_callback_details, | ||
| ) | ||
| self._record_updated_operation(operation.operation_id) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Confirmed and fixed in 22004eb. The in-memory path retained an all-None ErrorObject, while the existing file-store/service parser already maps its empty serialized payload to None. The fix canonicalizes only that exactly empty payload; any present fields, including empty messages/types/data and StackTrace: [], retain their values and the nonempty in-memory error object retains its identity.
Four public regressions reproduced the discrepancy before the fix. Paired tests now cover 28 actual handler/store/view/payload runs and compare caller error, status and resumed failure behavior with the existing filesystem baseline. The full affected suite passed 2,168 tests; the final paired controls, workflow checks, types and formatting pass. Core/OTel/provider APIs and shared cloud assertions are unchanged.
There was a problem hiding this comment.
Follow-up in cd81a3d preserves the raw history contract as well: CallbackFailed with no error details and IncludeExecutionData=true retains Error.Payload={} and Truncated=false, matching the observed service response. SDK-facing state remains error=None and the leaf remains UNSET. Nonempty errors, metadata-only history and other terminal projections are unchanged.
The exact JS revision used by the failed CI (19ee19f) now passes all 148 unchanged example tests locally with zero retries. The full affected Python suite passes 2,166 tests, and 78 factory checks cover the narrow projection boundaries. Current-head remote CI is running.
This comment has been minimized.
This comment has been minimized.
| end_timestamp=now if now is not None else real_now(), | ||
| callback_details=updated_callback_details, | ||
| ) | ||
| self._record_updated_operation(operation.operation_id) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
Represent an exactly empty serialized callback error as absent, matching CallbackDetails.from_dict without requiring a file-store round trip. Preserve every present field, including empty strings and stack lists. Compare real public handler failures across memory/file stores and both OTel views, including caller error/status and replay behavior. Pin the coordinated shared S1c revision with the strict empty-payload, parent-occurrence and callback phase-gate assertions.
This comment has been minimized.
This comment has been minimized.
Match the observed service projection for a failed callback with no error
details when IncludeExecutionData is true: keep Payload {} and mark it
untruncated. SDK-facing state remains error=None, with an UNSET leaf and
the same caller failure. Other statuses, nonempty errors and metadata-only
history retain their existing representation.
Verify the raw public history shape, state/telemetry separation and direct
factory boundaries. The unchanged JS 19ee19f example suite passes all 148
tests without retries or assertion/selection changes.
| ): | ||
| # Detailed service history retains an empty Error.Payload object. | ||
| # This projection must not turn the SDK-facing absent error into one. | ||
| event_error = EventError(payload=ErrorObject.from_dict({}), truncated=False) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed in 2a10053. History and callback state now keep their separate contracts: EventError.from_dict() preserves a present Payload: {} through raw → typed → raw conversion, while rebuilding callback operations maps only an exactly empty serialized error to None. Absent/null payloads retain their existing behavior; Truncated, rich/partial fields, explicit empty strings, and StackTrace: [] are preserved.
Added round-trip and DurableFunctionTestResult.from_execution_history coverage for both direct and decoded history. Actual public callback runs also check memory/filesystem parity, both OTel views, and history with/without execution data. Four focused regressions and four public-runner controls fail before the fix. Validation passes: 2,193 testing/OTel/conformance tests, 28 public runner executions, all 148 unchanged JS examples without retries, type checks, and formatting. Core, OTel, and deployed conformance handler code are unchanged.
This comment has been minimized.
This comment has been minimized.
|
@hln33 Follow-up to your #752 coverage comment: this PR now adds case 25 for the missing non-success/no-error-details mapping, alongside the existing case 24 RETRYING/UNSET coverage. Case 25 uses the public callback-failure handler and sends failure without Error after InvocationCompleted. The shared requirement checks the typed empty raw error payload and requires the FAILED callback leaf to remain UNSET in both views. Returning OK for that branch would now fail conformance. Case 26 also exercises a root callback’s first completion, its second-invocation parent, and two subsequent replays without duplicate terminal exports. On current commit cd81a3d, S1c cloud run 37716434728 reports 26/26 passed in all four backend/view suites, with zero failed or uncovered cases; both long-running suites pass 4/4. The unchanged JS examples CI also passes 148 tests on this head. FAILED, CANCELLED, TIMED_OUT and STOPPED without error details remain covered by the two-view unit matrix. The new real-service case exercises FAILED; no cloud reachability claim is made for the other three statuses. The separate new typed-history round-trip review is being investigated as testing-API maintenance; it does not change this deployed handler coverage. |
Codex AI reviewNo actionable findings. Residual risk remains in cloud-only OTel conformance and service callback-history behavior. Reviewed commit |
This branch was successfully deployed
2 participants
Add Python handlers and SAM mappings for OTel cases 21–26: completed-step replay, active context in user functions and callbacks, invocation retry status, callback failure without error details, and external callback completion followed by two later replays. Handlers use public durable APIs and ordinary implicit span parenting.
Case 26 creates a root callback, saves its result in a durable observation step, and uses two callback barriers to force four invocations. The shared driver delivers each callback after
InvocationCompleted; raw S3 assertions require the first terminal export before the observation step, parented to the second Invocation in invocation view, and reject duplicate exports on later replays. Case 25 reuses the existing callback-failure handler with an omitted failure payload and a strict raw-history no-error-details precondition.The local testing library reports callback success, failure and timeout through
UpdatedOperationIds, and consumes versions already delivered by checkpoint responses while retaining newer updates. Core terminal notifications track actual delivery within each invocation, preserving first notifications, concurrent delivery and invocation reset behavior. The local tester canonicalizes only an exactly empty callback error container as absent, matching the existing service parser and filesystem behavior. Present fields, including empty strings and empty stack lists, retain their values; the caller failure is identical across stores. Detailed callback-failure history preserves the service’s emptyError.Payloadobject withTruncated: falsewhen execution data is requested; SDK-facing state remainserror=None. Typed history preserves the explicit empty payload through raw-to-typed-to-raw round trips; rebuilding callback state alone canonicalizes an exactly empty error as absent. Metadata-only history, nonempty errors and other terminal projections retain their existing behavior. Checkpoint token/watermark values are preserved.The handler-context prerequisite isolates failed plugin setup and preserves token ownership, caller restoration and the no-plugin path. Handler scopes require explicit concrete-class opt-in, retaining compatibility with legacy helpers/properties/dynamic attributes. Both OTel views preserve same-trace parents and baggage and bind their durable span when the worker parent is absent or unrelated. Runtime dependency floors and persisted checkpoint formats remain unchanged.
Shared requirements and the reusable workflow are pinned to
bdb4f1cd0f9252c1aaa978bb8b341b71f2b9d9dcin aws/aws-durable-execution-conformance-tests#131. The shared self-test fixture uses12e760cf72dc14055ae0dec48bf0a20ad73451b2; its deployed core, OTel and handler source is identical to this PR's current revision. Each SDK CI run uses its own exact PR head. Cases 1–24 and the long-running scenarios remain in the matrix, with failed-plus-uncovered gating enabled.Merge prerequisites: #756 and aws/aws-durable-execution-conformance-tests#131. The status-mapping prerequisite #752 is already merged.
Coverage boundary: case 24 verifies
RETRY→RETRYING/UNSET. Case 25 verifiesFAILEDwithout error details →UNSET, including the typed empty raw-history payload precondition in both views.CANCELLED,TIMED_OUTandSTOPPEDwithout error details retain explicit unit coverage in both views; their cloud paths are not established. Oncd81a3d, S1c cloud run 37716434728 verifies all four main backend/view reports at 26/26 and both long-running reports at 4/4, with zero failed/uncovered cases. The same head's JS examples CI passes all 148 tests without retries. The testing-library history round-trip follow-up in2a100531leaves the deployed fixture unchanged. Its remote CI is also green: OTel run 37719373329 passes all four 26-case and both 4-case reports, and JS examples run 37719372461 passes 148 tests.Validation: