Skip to content

fix(testing): handle workflow failure states correctly - #682

Merged
hln33 merged 1 commit into
mainfrom
fix/656-emulator-null-invocation-error-causes-successful-execution
Aug 27, 2026
Merged

fix(testing): handle workflow failure states correctly#682
hln33 merged 1 commit into
mainfrom
fix/656-emulator-null-invocation-error-causes-successful-execution

Conversation

@hln33

@hln33 hln33 commented Aug 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Issue #, if available:
#656

Description of changes:

  • Modify handling of failed invocations to authoratively call self._fail_workflow() instead of self._complete_workflow() so that executions fail despite not having an error payload
    • Confirmed against the real backend service that failed invocations without error payloads should result in a failed execution
    • Prior to this change, failed invocations with no error payloads would result in a successful execution
  • Add tests covering the following behavior (requested in [Bug]: FAILED invocation without ErrorObject is completed as SUCCEEDED by local emulator #656):
    • A failed invocation results in a failed execution
    • Caught Child context failures do not cause execution failure

Testing:

  • Ran all tests in package via hatch run test:all
  • Invoked SAM CLI with local emulator changes and a reproducing Java durable function. This confirms failed invocations with empty error payloads result in failed execution:
sam local invoke WaitForConditionChildFailureJavaFunction

...

Execution Summary:
=========================
ARN:      31d25a2e-2e37-4bea-b67e-02441a5fa80c/55c89648-9aca-4b8e-8147-f01658dabf0a                                                                           Name:     N/A                                                                  Duration: 6.58s
Status:   FAILED ❌
Input:    {}

Commands you can use next
=========================
[*] Get execution details: sam local execution get 31d25a2e-2e37-4bea-b67e-02441a5fa80c/55c89648-9aca-4b8e-8147-f01658dabf0a
[*] View execution history: sam local execution history 31d25a2e-2e37-4bea-b67e-02441a5fa80c/55c89648-9aca-4b8e-8147-f01658dabf0a

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

@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 25, 2026 22:56 — with GitHub Actions Inactive
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 25, 2026 22:56 — with GitHub Actions Inactive
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 25, 2026 22:56 — with GitHub Actions Inactive
@hln33
hln33 marked this pull request as draft August 25, 2026 22:56
@hln33
hln33 force-pushed the fix/656-emulator-null-invocation-error-causes-successful-execution branch from f4129a2 to b6a604a Compare August 25, 2026 22:59
@hln33
hln33 force-pushed the fix/656-emulator-null-invocation-error-causes-successful-execution branch from b6a604a to 861756a Compare August 25, 2026 23:20
@hln33
hln33 force-pushed the fix/656-emulator-null-invocation-error-causes-successful-execution branch from 861756a to 09acaf6 Compare August 25, 2026 23:22
@hln33
hln33 deployed to ai-pr-review August 25, 2026 23:22 — with GitHub Actions Active
@hln33
hln33 had a problem deploying to ai-pr-review-runtime August 25, 2026 23:35 — with GitHub Actions Failure
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 25, 2026 23:35 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

Sign in to view
@hln33
hln33 marked this pull request as ready for review August 26, 2026 00:24
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 26, 2026 00:24 — with GitHub Actions Inactive
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 26, 2026 00:24 — with GitHub Actions Inactive
@hln33
hln33 had a problem deploying to ai-pr-review-runtime August 26, 2026 00:24 — with GitHub Actions Failure
@github-actions

This comment has been minimized.

Sign in to view
bchampp
bchampp previously approved these changes Aug 26, 2026
View reviewed changes
Comment thread ...urable-execution-sdk-python-testing/src/aws_durable_execution_sdk_python_testing/executor.py
yaythomas
yaythomas requested changes Aug 26, 2026
View reviewed changes

@yaythomas yaythomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thank you! just some nits.

Comment thread packages/aws-durable-execution-sdk-python-testing/tests/executor_test.py Outdated
Comment thread packages/aws-durable-execution-sdk-python-testing/tests/executor_test.py Outdated
Comment thread packages/aws-durable-execution-sdk-python-testing/tests/e2e/child_context_failure_test.py Outdated
Comment thread packages/aws-durable-execution-sdk-python-testing/tests/execution_test.py Outdated
Comment thread ...urable-execution-sdk-python-testing/src/aws_durable_execution_sdk_python_testing/executor.py
Comment thread ...urable-execution-sdk-python-testing/src/aws_durable_execution_sdk_python_testing/executor.py
@hln33
hln33 had a problem deploying to ai-pr-review-runtime August 26, 2026 22:00 — with GitHub Actions Failure
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 26, 2026 22:00 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

Sign in to view
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 26, 2026 22:15 — with GitHub Actions Inactive
@hln33
hln33 had a problem deploying to ai-pr-review-runtime August 26, 2026 22:15 — with GitHub Actions Failure
hln33
hln33 commented Aug 26, 2026
View reviewed changes
Comment thread ...-durable-execution-sdk-python-testing/src/aws_durable_execution_sdk_python_testing/runner.py
if execution.status == "FAILED":
logger.warning("Execution failed")
return execution
if execution.status in ["TIMED_OUT", "ABORTED"]:

@hln33 hln33 Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think "ABORTED" is a valid execution status. Could not find any references to it in other SDKs. The API documentation also does not list "ABORTED" as being a valid execution status.

I think it should instead be "STOPPED", as that specific status is not covered in this function.

I noticed this when implementing a new "execution_status" field on the test result class and my AI coding agent flagged this to me.

@hln33
hln33 requested a review from yaythomas August 26, 2026 22:18
@github-actions

This comment has been minimized.

Sign in to view
yaythomas
yaythomas approved these changes Aug 27, 2026
View reviewed changes

@yaythomas yaythomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

friendly reminder to squash at merge with a conventional-commits message (lint-commits is currently red only because of the squash: markers).

@hln33
fix(testing): handle workflow failure states correctly
79d088b 
Preserve failed invocation results when Lambda returns FAILED without an
error payload. The local test runner now reports a failed execution
instead of treating the result as successful.

Expose terminal execution status on durable function test results and
align cloud runner polling with the STOPPED terminal status.
@hln33
hln33 force-pushed the fix/656-emulator-null-invocation-error-causes-successful-execution branch from d8badbe to 79d088b Compare August 27, 2026 21:10
@hln33
hln33 had a problem deploying to ai-pr-review-runtime August 27, 2026 21:10 — with GitHub Actions Failure
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 27, 2026 21:10 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Codex AI review

No actionable findings. Residual risk: tests were not executed under the read-only review constraints.

Reviewed commit 79d088bd25a47062e21374631c53e5477dae09e6. Workflow run

@hln33
hln33 merged commit 24c7536 into main Aug 27, 2026
16 of 17 checks passed
@hln33
hln33 deleted the fix/656-emulator-null-invocation-error-causes-successful-execution branch August 27, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments
@zhongkechen zhongkechen zhongkechen left review comments
@yaythomas yaythomas yaythomas approved these changes
@bchampp bchampp bchampp left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hln33 @yaythomas @bchampp @zhongkechen