feat(core): exception transitions via on_error capture and wildcard routing (#30)#796
Open
andreahlert wants to merge 2 commits into
Open
feat(core): exception transitions via on_error capture and wildcard routing (#30)#796andreahlert wants to merge 2 commits into
andreahlert wants to merge 2 commits into
Conversation
Introduces wildcard transition sources so a graph can route from ANY action
to a handler, e.g. ("*", "handler", expr("error is not None")).
Resolves the transition-precedence bug that blocked issue #30: an action's
own default transition would shadow a guarded wildcard route, so the route
never fired. get_next_node now resolves in 4 tiers, first match wins:
1. source-specific non-default transitions (insertion order)
2. wildcard non-default transitions (insertion order)
3. source-specific default transition
4. wildcard default transition
This is a core-routing semantic change: in default-FIRST graphs a previously
dead non-default transition becomes live. For the common default-LAST graphs
(and graphs without wildcards) resolution is identical to before; covered by
a regression test.
The wildcard source is carried by a sentinel Result().with_name("*") used
only as a transition from_; it is never added to the action map, so
introspection (visualize, ApplicationModel serialization) is unaffected.
Part of #30.
Signed-off-by: André Ahlert <andre@aex.partners>
Adds the ability to suppress an action exception and route to an
error-handling action instead of breaking control flow.
* capture_as(field, include_traceback=True): an on_error handler that
writes a JSON-serializable record {type, message, traceback} into a
state field. Exported from burr.core.
* @action(..., on_error=handler): per-action error handler, any callable
(State, Exception) -> State.
* ApplicationBuilder.with_error_handling(handler): builder-level global
handler applied to actions without their own on_error. Per-action wins.
When an action raises, the effective handler suppresses the exception,
writes captured state, and execution continues; a wildcard transition
("*", handler, expr("error is not None")) then routes to the handler.
The captured field bypasses reducer write-validation and need not be in
the action's declared writes. A failing handler never masks the original
exception (re-raised with the handler error as cause). The capture field
must be seeded (e.g. .with_state(error=None)) since expr() raises on a
missing key; documented in the capture_as example.
Also restructures _astep so a SYNC action driven through the async path
reports the real result/state to the async post_run_step hook instead of
stale values (previously delegated to _step and discarded). Behavior is
locked by a regression test.
Streaming error handling (@streaming_action on_error during incremental
consumption) is intentionally out of scope and left as follow-up.
Closes #30.
Signed-off-by: André Ahlert <andre@aex.partners>
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.
Summary
Implements #30 "Add exception transitions". Currently an exception in an action breaks control flow and stops the program. This adds the ability to suppress an exception, capture it into state, and route to an error-handling action via a wildcard transition.
Scope chosen: capture + wildcard (combines ideas #3 and #4 from the issue thread).
What's included
capture_as(field, include_traceback=True)— anon_errorhandler that writes a JSON-serializable{type, message, traceback}record into a state field. Exported fromburr.core.@action(..., on_error=handler)— per-action handler, any callable(State, Exception) -> State.ApplicationBuilder.with_error_handling(handler)— builder-level global handler for actions without their ownon_error. Per-action takes precedence.("*", target, condition)transitions — route from any action.The captured field bypasses reducer write-validation (need not be in
writes). A failing handler never masks the original exception (re-raised with the handler error as cause). Captured records are JSON-serializable, so persistence/tracking are unaffected.Behavior changes to call out for review
These go beyond the headline feature, kept as separate commits:
fix(core)commit). The fix for the precedence bug that kept Add exception transitions #30 open: an action's owndefaulttransition shadowed a guarded wildcard route, so the route never fired.get_next_nodenow resolves: source non-default, wildcard non-default, source default, wildcard default. Semantic change: in default-FIRST graphs a previously-dead non-default transition becomes live. Default-LAST graphs (the common case) and graphs without wildcards are unchanged, covered by a regression test._asteprestructure. A SYNC action driven through the async path now reports the realresult/stateto the asyncpost_run_stephook instead of stale values (it previously delegated to_stepand discarded them). Locked by a regression test.Out of scope (follow-up)
Streaming error handling (
@streaming_actionon_errorduring incrementalstream_result/astream_resultconsumption) is intentionally not included, to avoid shipping a silently inert API. Tracked as a follow-up on #30.Testing
pytest tests/core/green (363 tests, +17 new).visualize()andApplicationModelserialization (the tracking UI path).flake8clean,black/isortapplied.Process
The design never converged on the list (the issue thread has 4 competing shapes). This PR is a concrete proposal. I will start a
[DISCUSS]thread on dev@burr.apache.org referencing this branch before merge, per podling process. Feedback on the precedence semantics in particular is welcome.Closes #30.