Repository navigation
fix: async engine answers callers after their macrostep and hands the loop over on cancellation - #691
Merged
Merged
Conversation
…r on cancellation On the async engine a caller is now answered once the macrostep of its event ends, so an exception raised by the internal events or eventless transitions that follow it reaches that caller. When nobody awaits the failing event, the coroutine running the loop gets the exception, as on the sync engine. Cancelling the coroutine running the processing loop no longer leaves the callers of queued events waiting: one of them takes the loop over. Fixes #689 Fixes #690 Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
…e the runner's future A CancelledError for a caller whose task was not cancelled is swallowed by asyncio.TaskGroup, so the caller of the event interrupted by the cancellation now gets a StateMachineError. The loop runner whose own queued future was rejected awaits it instead of raising, so asyncio does not log the exception as never retrieved. Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
…he loop over Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #691 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 53 53
Lines 5859 5897 +38
Branches 936 940 +4
=========================================
+ Hits 5859 5897 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.



Why
Two defects in the async engine's
processing_loop, both present on v3.2.1 and onmainafter #688. #689: an exception raised in the macrostep after the lock holder's own event succeeded was lost, andawait send()returned the result. #690: cancelling the coroutine running the loop left every queuedsend()pending until some latersend()ran the loop again.Fixes #689
Fixes #690
What changes
catch_errors_as_events=False: a caller's future is answered when the macrostep of its event ends. An exception from the internal events or eventless transitions that follow the event reaches that caller, as the sync engine raises it fromsend(). This also applies to a queued caller: in the fix: answer callers queued behind a macrostep that raises #688 testtest_failure_after_a_queued_event_..., that caller used to get"crossed"and now gets the exception.__initial__activation), the coroutine running the loop raises the exception. It used to be dropped silently. Events still queued are dropped and their callers rejected, as in fix: answer callers queued behind a macrostep that raises #688.StateChartis unchanged.BaseException) the coroutine running the loop now stops only that coroutine. The internal queue is cleared, the lock is released, and a coroutine awaiting a queued event takes the loop over (woken via_lock_waiters), so each queued caller gets its own result, delayed events included.StateMachineError("The task running the processing loop was cancelled while it processed this event."). I chose anExceptionrather thanCancelledErrorbecause aCancelledErrorin a task that nobody cancelled is swallowed byasyncio.TaskGroupand is missed byexcept Exception. The event cannot be re-run, since its callbacks stopped halfway.release-3.2.2-async-loopsection indocs/releases/3.2.2.md, which includes a doctest of the Async: an exception in the macrostep after the lock holder's own event is silently lost #689 example. One "behavior changes" bullet is added. The fix: answer callers queued behind a macrostep that raises #688 sentence "the coroutine whose event enabled the failing transitions gets its own event's result" is corrected.docs/async.mdis updated.Evidence
The issue examples were run with
uv run python -I repro.py, the script built from the snippets in #689 and #690:send():pending: True steps: 0, thenlater: 2 other: 1.tests/test_async_futures.py, all withpytest.mark.timeout(5).TestMacrostepErrorReachesTheCallerOfItsEventruns on both engines viasm_runner.TestCancelledLockHoldercovers queued callers, a delayed event, the interrupted caller, a cancelled waiter, a cancelledactivate_initial_state(), and that the machine stays usable afterwards. Withstatemachine/reverted tomain, 17 of them fail, including the 13 async Async: an exception in the macrostep after the lock holder's own event is silently lost #689 cases and all 4 takeover cases, plus the release-notes doctest. With the fix, they all pass.timeout 120 uv run pytest -n 4 --cov=statemachine --cov-branch:2777 passed, 25 skipped, 44 xfailed, TOTAL 100%.ruff check,ruff format --check,mypy --namespace-packages --explicit-package-bases statemachine/ tests/andpyright statemachine/are clean. Pre-commit hooks ran on every commit.send()there, and both engines are covered bytest_caller_gets_the_exception[sync-*].test_caller_running_the_loop_retrieves_its_rejected_future.CancelledErrorfor the interrupted caller was swallowed byTaskGroup, so it becameStateMachineError. Docs and test nits were also fixed.Not verified
StateMachineErrorpath does not depend onCancelledErrormessage propagation, but CI is the first run on those versions.asyncio.waitfor its future or the lock release. 2000 concurrent sends were measured about 5-10% slower in the review run (0.155 s vs 0.145 s), with no benchmark in CI.tests/test_cancel_event.py::test_a_caller_awaiting_a_cancelled_event_gets_nonecounted ticks and now waits for the task.main:TransitionNotAllowed), and it keeps internal events after aBaseException.StateChartcancelled inside anoncallback can be left with an empty configuration, because the microstep restores the configuration only onException.send()coroutine starts leaves a future nobody awaits.