Skip to content

[Fix #1737] Calculate communication status error code from cause. - #1739

Merged
fjtirado merged 1 commit into
open-workflow-specification:mainfrom
fjtirado:Fix_1737
Oct 5, 2026
Merged

fjtirado merged 1 commit into
open-workflow-specification:mainfrom
fjtirado:Fix_1737

Conversation

@fjtirado

@fjtirado fjtirado commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Fix #1737

Copilot AI balanced review requested due to automatic review settings October 5, 2026 11:18
Comment thread types/src/main/java/io/serverlessworkflow/types/Errors.java
Comment thread types/src/main/java/io/serverlessworkflow/types/Errors.java

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The timeout check misses the HTTP timeout exception types produced by JAX-RS connectors.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates HTTP transport error mapping to derive communication status codes from exception causes.

Changes:

  • Changes the default communication status from 502 to 500.
  • Maps recognized timeout causes to status 408.
File Description
types/​src/​main/​java/​io/​serverlessworkflow/​types/​Errors.java Updates the communication error default.
impl/​http/​src/​main/​java/​io/​serverlessworkflow/​impl/​executors/​http/​AbstractRequestExecutor.java Derives transport status from the failure cause.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@gmunozfe gmunozfe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me, thanks for quick fix @fjtirado !

Copilot AI balanced review requested due to automatic review settings October 5, 2026 11:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Read timeouts remain misclassified, existing exception handling regresses, and the global status change affects unrelated APIs.

Review effort: Balanced
Findings: 3 Medium severity · 1 Low severity

Open (4)

Comment thread types/src/main/java/io/serverlessworkflow/types/Errors.java
… error code from cause.

Fix open-workflow-specification#1737

Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>
Copilot AI balanced review requested due to automatic review settings October 5, 2026 11:47
Comment thread types/src/main/java/io/serverlessworkflow/types/Errors.java

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

It removes existing IllegalStateException handling and lacks regression coverage for the new mappings.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
Resolved since last review (3)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The updated mapping addresses both timeout and non-timeout transport scenarios from issue #1737.

Review effort: Balanced
Findings: None

Resolved since last review (2)

@fjtirado
fjtirado merged commit bfdffd0 into open-workflow-specification:main Oct 5, 2026
4 checks passed
@fjtirado
fjtirado deleted the Fix_1737 branch October 5, 2026 19:57
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.

HTTP transport failures are mapped to communication/422 instead of appropriate status codes

3 participants