Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Raw output can contain malformed HTTP status lines and body framing, misidentify the protocol version, and fail for custom model factories because it bypasses the byte-array conversion.
4 open issues
What changed in this PR
This PR addresses #1829 by adding HTTP call output selection, allowing workflows to access response headers and status alongside the body.
Changes:
- Routes
content,response, andrawoutput modes through the HTTP executor. - Extends response adapters and model factories to support full responses and binary output.
- Adds a workflow sample and integration test for response headers.
| File | Description |
|---|---|
impl/test/src/test/resources/workflows-samples/call-http-get-headers.yaml |
Adds a workflow using output: response. |
impl/test/src/test/java/org/openworkflow/sdk/impl/test/HTTPWorkflowDefinitionTest.java |
Tests response headers and body content. |
impl/model/src/main/java/org/openworkflow/sdk/impl/model/jackson/JacksonModelFactory.java |
Adds byte-array model conversion. |
impl/http/src/main/java/org/openworkflow/sdk/impl/jakarta/http/JakartaHttpResponse.java |
Exposes headers, content, and raw response output. |
impl/core/src/main/java/org/openworkflow/sdk/impl/WorkflowModelFactory.java |
Adds default Base64 conversion for byte arrays. |
impl/core/src/main/java/org/openworkflow/sdk/impl/http/WorkflowHttpResponse.java |
Expands the response interface for output modes. |
impl/core/src/main/java/org/openworkflow/sdk/impl/http/WorkflowHttpRequestHelper.java |
Implements output handlers and response serialization. |
impl/core/src/main/java/org/openworkflow/sdk/impl/http/DefaultHttpResponse.java |
Adapts the default client response to the expanded interface. |
impl/core/src/main/java/org/openworkflow/sdk/impl/executors/http/HttpExecutor.java |
Passes the selected output handler to request execution. |
impl/core/src/main/java/org/openworkflow/sdk/impl/executors/http/CallableTaskHttpExecutorBuilder.java |
Selects the handler from the HTTP output setting. |
impl/core/src/main/java/org/openworkflow/sdk/impl/auth/DigestAuthProvider.java |
Removes a cast after header access becomes string-typed. |
Review effort: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The response and raw output modes bypass HTTP status validation, while raw serialization retains chunked-transfer headers without chunk framing, breaking error handling and producing invalid HTTP messages.
5 open issues
2 resolved since last review
Review effort: Balanced
mathecruz
reviewed
Nov 13, 2026
fjtirado
marked this pull request as draft
November 13, 2026 17:57
fjtirado
force-pushed
the
Fix_#1829
branch
2 times, most recently
from
November 13, 2026 19:19
e135133 to
0a29160
Compare
It also support http request & response additional content types Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>
fjtirado
force-pushed
the
Fix_#1829
branch
2 times, most recently
from
November 13, 2026 19:23
970dab5 to
f54e8c0
Compare
…er name Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Binary responses can be corrupted by text decoding, while ignored charset declarations and overwritten request Content-Type headers can cause incorrect payload encoding and server-side parsing.
6 open issues
5 resolved since last review
Review effort: Balanced
Comment on lines
+102
to
+103
| return HttpRequest.BodyPublishers.ofString( | ||
| body.asText() |
| } | ||
|
|
||
| private WorkflowModel textContent() { | ||
| return application.modelFactory().from(new String(response.body())); |
Comment on lines
+28
to
+29
| } else { | ||
| return String.class; |
| return inferContentType(); | ||
| } | ||
| }) | ||
| .orElse(inferContentType()); |
| } | ||
| } | ||
| return null; | ||
| candidate = candidate.trim().toLowerCase(); |
Comment on lines
+117
to
+120
| return Entity.text( | ||
| model | ||
| .asText() | ||
| .orElse("Workflow model should support asText for content type " + contentType)); |
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 #1829