fix: avoid spurious "exception in shielded future" logs on cancellation#1624
Open
Bathula-Adiseshu wants to merge 1 commit into
Open
fix: avoid spurious "exception in shielded future" logs on cancellation#1624Bathula-Adiseshu wants to merge 1 commit into
Bathula-Adiseshu wants to merge 1 commit into
Conversation
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
Replaced internal
asyncio.shield()usage in workflow outbound cancellation loops with a custom_shield_await()helper.The helper uses
asyncio.wait()to preserve the behavior we need:asyncio.shield()exception logging callback that caused spurious ERROR logsAdded regression coverage to verify cancelling workflows with in-flight activities does not emit
"exception in shielded future"ERROR logs.Why?
On Python 3.11+,
asyncio.shield()can attach an internal exception logging callback when the outer task is cancelled.Temporal workflow cancellation handling intentionally catches
CancelledError, clears cancellation state, and waits for the operation to finish. In this flow, the original shield callback could remain attached to the future. When the activity later completed with anActivityErrordue to cancellation, the workflow handled it correctly, but the leftover callback produced an orphan ERROR log.This change removes that behavior while keeping the cancellation semantics required by Temporal.
Checklist
Closes [Bug] Single spurious "exception in shielded future" ERROR log per cancelled in-flight activity on Python 3.11+ (residual after #1523) #1600
How was this tested:
test_workflow_cancel_no_shielded_future_logpoe lintpytest tests/worker/test_workflow.py::test_workflow_cancel_no_shielded_future_logpytest tests/worker/test_workflow.pyNo docs updates needed.