Skip to content

Run Lambda worker shutdown hooks when an invocation fails - #2073

Open
Jackmeson1 wants to merge 2 commits into
temporalio:mainfrom
Jackmeson1:fix/lambda-shutdown-hooks-on-failure
Open

Jackmeson1 wants to merge 2 commits into
temporalio:mainfrom
Jackmeson1:fix/lambda-shutdown-hooks-on-failure

Conversation

@Jackmeson1

Copy link
Copy Markdown
Contributor

Run Lambda invocation shutdown hooks from the configuration scope's finally, before configure teardown. This covers connection and worker failures, cancellation, and the short-deadline rejection that previously skipped cleanup.

Hooks still require successful configuration setup and task queue validation. Ordinary hook exceptions are logged without replacing the invocation error; a hook's BaseException can still interrupt cleanup and replace it. This is separate from #2041's tracer-provider reuse.

Outside-sandbox implementation validation on Linux x86_64/Colab, Python 3.12.3 and pytest 9.1.1: the Lambda-worker directory passed 79 tests and the AWS directory passed 165. On d488ff5 with the new tests, 10 failed at missing-hook or ordering assertions and 69 passed. On the final commit with Python 3.13.18, tests/contrib/aws/lambda_worker passed 79 tests, the new tests failed against d488ff5's _run_worker.py, and ruff, pyright, mypy, basedpyright and pydocstyle passed.

AI disclosure: this was prepared with AI assistance (Claude Opus 5.5 and Claude Sonnet 5.5).

🤖 Generated with Claude Code

https://claude.ai/code/session_01JfvcPZyKcKQUG9fQGYfRMw

Invoke shutdown hooks from a finally inside the yielded configuration
scope so invocation errors and cancellation no longer skip them. Keep
hook-before-teardown ordering and ordinary hook-exception handling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JfvcPZyKcKQUG9fQGYfRMw
Signed-off-by: Jackmeson1 <130693642+Jackmeson1@users.noreply.github.com>
@Jackmeson1
Jackmeson1 requested a review from a team as a code owner November 3, 2026 10:22

@mani-j9 mani-j9 left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you @Jackmeson1, LGTM if @johanngan doesn’t have any concerns either. I saw the unit test and validation in the description, just curious whether you were also able to try it on a real Lambda?

@johanngan johanngan 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.

Thanks for the fix @Jackmeson1! Mostly LGTM, just one small suggestion.

# close resources used by the hooks.
if cm is not None:
async with cm:
_validate_task_queue(config)

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.

I think it might make more sense to run hooks even on task queue validation failure. If configure succeeded, any resources it instantiated would still need to be torn down, regardless of whether or not the task queue was properly configured. Can we wrap the _validate_task_queue calls in the try-finally blocks as well?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Plus 1 to this

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.

Done, both task queue validation calls are now inside the cleanup try/finally blocks. Added regression coverage for plain and async-generator configure callbacks, including verifying that hooks run before generator teardown when validation fails.

Include task queue validation in the invocation cleanup scope for plain
and generator configure callbacks. Preserve hook-before-teardown ordering
and cover missing task queues in both existing shutdown-hook tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JfvcPZyKcKQUG9fQGYfRMw
Signed-off-by: Jackmeson1 <130693642+Jackmeson1@users.noreply.github.com>

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants