Skip to content

[Fix #1414] Use try rather than do as position name#1432

Open
fjtirado wants to merge 1 commit into
serverlessworkflow:mainfrom
fjtirado:Fix_#1414
Open

[Fix #1414] Use try rather than do as position name#1432
fjtirado wants to merge 1 commit into
serverlessworkflow:mainfrom
fjtirado:Fix_#1414

Conversation

@fjtirado
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado commented Jun 2, 2026

Fix #1414

Copilot AI review requested due to automatic review settings June 2, 2026 18:01
@fjtirado fjtirado requested a review from wmedvede June 2, 2026 18:02
@fjtirado fjtirado force-pushed the Fix_#1414 branch 2 times, most recently from e7feb85 to 77c97d7 Compare June 2, 2026 18:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #1414 by adjusting how jsonPointer positions are generated for TryTask internals, so the try block is represented with a "try" container name and adding catch.do no longer causes inconsistent/duplicated container segments in the TryTask position.

Changes:

  • Update TryExecutor to build the try task list under the "try" position prefix (instead of defaulting to "do"), and avoid mutating the TryTask’s position when building catch.do executors.
  • Extend TaskExecutorHelper.createExecutorList(...) with an overload that allows callers to specify the container name used in position generation.
  • Update retry/position assertions in tests and add a sample workflow + test that exercises catch.do.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
impl/test/src/test/resources/workflows-samples/try-catch-with-retry-and-do.yaml Adds a workflow sample that includes catch.do for reproduction/testing.
impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java Updates expected jsonPointer keys (now using try) and adds a test that runs a workflow with catch.do.
impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java Generates try-block executors under "try" and uses a copied position for catch.do to keep TryTask position stable.
impl/core/src/main/java/io/serverlessworkflow/impl/executors/TaskExecutorHelper.java Adds an overload to customize the position container prefix used when building executor lists.
Comments suppressed due to low confidence (1)

impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java:61

  • The removal of new TraceExecutionListener() leaves TraceExecutionListener as an unused import in this file, which will fail compilation. Please remove the unused import (or reintroduce the listener if it’s still intended).
  }

  @AfterEach
  void tearDown() throws IOException {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java Outdated
Copilot AI review requested due to automatic review settings June 2, 2026 18:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TryTask: wrong jsonPosition generation when using the catch.do field

2 participants