[Fix #1601] Implement then on catch - #1609
Merged
Merged
Conversation
Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for a then directive on try...catch blocks (issue #1601), and refactors the executor builder generics to preserve concrete executor types through the builder hierarchy.
Changes:
- Implement handling of
catch.theninTryExecutorby building a separate catch transition and applying it when a catch matches. - Refactor
RegularTaskExecutorBuilderto be generic on both task type and executor type, updating all affected executors’ builders accordingly. - Add a workflow sample and extend an existing parameterized test to cover
catch.then.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| impl/test/src/test/resources/workflows-samples/try-catch-match-then.yaml | Adds a sample workflow that uses catch.then to redirect execution after a handled error. |
| impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java | Extends parameterized coverage to include the new catch.then sample. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/WaitExecutor.java | Updates builder type parameters to match the new RegularTaskExecutorBuilder<T, V> signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java | Implements catch.then transition wiring and alters execution/transition behavior for try/catch. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/SetExecutor.java | Updates builder generics to the new base builder signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunTaskExecutor.java | Updates builder generics to the new base builder signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/RegularTaskExecutor.java | Refactors the regular executor builder to carry the concrete executor type and simplifies execute chaining. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/RaiseExecutor.java | Updates builder generics to the new base builder signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/ListenExecutor.java | Updates builder generics to the new base builder signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/ForkExecutor.java | Updates builder generics to the new base builder signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/ForExecutor.java | Updates builder generics to the new base builder signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/EmitExecutor.java | Updates builder generics to the new base builder signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/DoExecutor.java | Updates builder generics to the new base builder signature. |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/CallTaskExecutor.java | Updates builder generics to the new base builder signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ricardozanini
approved these changes
Aug 7, 2026
Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (2)
impl/test/src/test/resources/workflows-samples/try-catch-retry-inline-then.yaml:21
- The
catch.thenblock is currently written as a nested mapping (then:followed by an indented bare key), which YAML parses as an object (e.g.,{endGracefully: null}) rather than the intended scalar flow directive. This is inconsistent with the other sample (then: endGracefully) and can break parsing/binding toFlowDirective.
then:
endGracefully
impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:80
catchTransitionBuilderis never initialized, so if this builder is built without a priorconnect(...)call (e.g.,TaskExecutorHelper.createBranchList(...)builds executors without callingconnect),buildTransition(...)will throw a NullPointerException when it callscatchTransitionBuilder.map(...). Initialize it toOptional.empty()sobuildTransition(...)is safe regardless of whetherconnect(...)ran.
private final FlowDirective catchDirective;
private Optional<TransitionInfoBuilder> catchTransitionBuilder;
private String errorVariable;
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.
Fix #1601