Python: Surface switch-case condition errors instead of routing to the default branch - #8490
Open
Ruiqi Wang (RachelWanggg) wants to merge 2 commits into
Open
Ruiqi Wang (RachelWanggg) wants to merge 2 commits into
Ruiqi Wang (RachelWanggg) wants to merge 2 commits into
Conversation
…fault SwitchCaseEdgeGroup wrapped every case predicate in a bare `except Exception` that only logged a warning, so a predicate that raised was treated exactly like one that returned False: the message fell through to the default branch and the workflow completed successfully. A broken routing predicate became silently wrong routing rather than a visible failure. This contradicted `Edge.should_route`, which documents that predicate errors are deliberately allowed to surface "to avoid masking logic bugs", and made the existing `FanOutEdgeRunner` error path (span status EXCEPTION, re-raise) unreachable for switch-case groups, so the failures were missing from telemetry too. The block was marked `# pragma: no cover`. Also align `FanOutEdgeRunner` with `SingleEdgeRunner`, which checks `_can_handle` before evaluating a condition: drop messages no target can handle before running the selection function. Without this, removing the catch would turn an undeliverable message into a raised AttributeError instead of a type-mismatch drop. Fixes microsoft#8489 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ruiqi Wang (RachelWanggg)
deployed
to
github-app-auth
September 18, 2026 00:17 — with
GitHub Actions
Active
Ruiqi Wang (RachelWanggg)
deployed
to
github-app-auth
September 18, 2026 00:17 — with
GitHub Actions
Active
Ruiqi Wang (RachelWanggg)
deployed
to
github-app-auth
September 18, 2026 00:17 — with
GitHub Actions
Active
Author
|
@microsoft-github-policy-service agree |
…function The runner fans every message out to all of the source's edge runners and gathers them with `gather_cancelling_siblings_on_error`. `FanOutEdgeRunner` checked `message.target_id` only after running the selection function, so a message addressed to an executor reached through a different edge still had this group's selection function evaluated against it - and a raise there cancelled the sibling runner that was actually delivering the message. Check `message.target_id` against `_target_map` first and return DROPPED_TARGET_MISMATCH, ahead of the `_can_handle` check, matching `SingleEdgeRunner`, which checks the target and then `_can_handle` before evaluating `Edge.should_route`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ruiqi Wang (RachelWanggg)
deployed
to
github-app-auth
September 18, 2026 05:42 — with
GitHub Actions
Active
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.
Motivation & Context
SwitchCaseEdgeGroupwrapped every case predicate in a bareexcept Exceptionthat only logged awarning. A predicate that raised was treated exactly like one that returned
False, so the messagefell through to the default branch and the workflow completed successfully. A typo or a missing
attribute in a routing predicate became silently wrong routing instead of a visible failure.
This contradicted the rest of the routing layer:
Edge.should_routedocuments the opposite policy for the same concept — "Any exception raised bythe callable is deliberately allowed to surface to the caller to avoid masking logic bugs."
FanOutEdgeRunner.send_messagealready handles a selection function that raises: it marks theedge-group span
EDGE_GROUP_DELIVERY_STATUS = EXCEPTIONand re-raises. The inner catch made thatpath unreachable for switch-case groups, so these failures were absent from telemetry as well.
SwitchBuilder.cs/WorkflowBuilder.CreateConditionFunc) has noequivalent catch.
The block was marked
# pragma: no cover, so nothing exercised it.Description & Review Guide
What are the major changes?
_edge.py—SwitchCaseEdgeGroup.selection_funcno longer catches predicate errors; theypropagate to the caller, matching
Edge.should_route._edge_runner.py—FanOutEdgeRunner.send_messagedrops a message that no target can handlebefore running the selection function, marking the span
DROPPED_TYPE_MISMATCHandreturning
False. This mirrorsSingleEdgeRunner, which checks_can_handlebeforeevaluating
Edge.should_route.test_edge.pycovering both behaviours.Why change 2 is part of this PR.
SingleEdgeRunneronly ever evaluates a predicate for amessage the target could actually receive;
FanOutEdgeRunnerevaluated the selection functionfirst. The existing
test_switch_case_edge_group_send_message_with_invalid_datapasses astrinto a
lambda x: x.data < 0predicate and depended on the swallow to reach itssuccess is Falseassertion. Without the ordering fix, change 1 alone would turn thatundeliverable-message case into a raised
AttributeError. With it, that test passes unchanged andgenuine predicate errors still surface. I'm happy to split this into a separate PR if you'd
rather keep the two concerns apart.
What is the impact of these changes?
the failure is recorded on the edge-group span.
outcome for those messages is unchanged (
False); they are just dropped earlier.What do you want reviewers to focus on? Whether bundling the
FanOutEdgeRunnerorderingchange with the swallow removal is the right call, or whether you'd prefer the existing
..._with_invalid_datatest to be updated instead.Verification
uv run poe test -P core— full core + workflow suite passes (0 failures).uv run poe syntax -P core,uv run poe pyright -P core,uv run poe test-typing -P core(mypy, pyrefly, ty, zuban, pyright) — all pass.
reverting change 2 fails the fan-out ordering test and the pre-existing
..._with_invalid_datatest.Related Issue
Fixes #8489
Contribution Checklist