Skip to content

Support Multi<byte[]> as a streamed REST Client request body - #56465

Open
jnbdz wants to merge 1 commit into
quarkusio:mainfrom
SiteNetSoft:rest-client-multi-bytes-upload
Open

Support Multi<byte[]> as a streamed REST Client request body#56465
jnbdz wants to merge 1 commit into
quarkusio:mainfrom
SiteNetSoft:rest-client-multi-bytes-upload

Conversation

@jnbdz

@jnbdz jnbdz commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

The REST Client streams a Multi<io.vertx.core.buffer.Buffer> request body without buffering it, but that is the only Multi item type accepted by the build-time check in JaxrsClientReactiveProcessor. A Multi<byte[]> (for example the one returned by another client method declared with that return type) is rejected with When using Multi as body parameter only Multi<io.vertx.core.buffer.Buffer> is supported, so the caller has to map it to Buffer first.

This PR accepts Multi<byte[]> as well:

  • the build-time check allows Multi<byte[]> next to Multi<Buffer>;
  • ClientSendRequestHandler converts each item to a Buffer before handing the stream to Vert.x, so Buffer items pass through unchanged and byte[] items are wrapped. Programmatic clients (Entity.entity(multi, ...)) have no build-time check, so an unsupported item type now fails the request with a clear message instead of a ClassCastException;
  • a Multi body that fails part-way is now reported as a failure. HttpClientRequest.send(ReadStream) pipes with the default endOnFailure(true) and the Mutiny ReadStreamSubscriber invokes the end handler after the exception handler, so the chunked body was ended normally and the server response to the truncated body was returned as a success. The stream is now piped with endOnFailure(false) and the request is reset with the source failure, as the multipart path already does, so the call fails with a ProcessingException;
  • the "Sending large payloads" section of the REST Client guide listed Multi<io.vertx.mutiny.core.buffer.Buffer>, a type that no longer exists with the Vert.x 5 Mutiny bindings. It now lists the two accepted types.

Relates to #21440 (request side; the response side is #56462).

@quarkus-bot

This comment has been minimized.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

🎊 PR Preview 24f4e67 has been successfully built and deployed to https://quarkus-pr-main-56465-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@quarkus-bot

This comment has been minimized.

@geoand geoand changed the title Support Multi<byte[]> as a streamed REST Client request body Support Multi<byte[]> as a streamed REST Client request body Sep 7, 2026

@geoand geoand 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.

Nice!

I will also what I've also asked for in other similar PRs: The test should be updated to use Multi that does not fit in memory, thus ensuring that no buffering is happening

@quarkus-bot

This comment has been minimized.

@jnbdz
jnbdz force-pushed the rest-client-multi-bytes-upload branch from b9c2037 to 90109a0 Compare September 7, 2026 16:01
@jnbdz

jnbdz commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Done in 90109a0d3c1: SendMultiBytesTest now sends 2 GB (32768 chunks of 64 KB, one shared array), above the 1.5 GB heap of the test JVM, and the server counts the bytes from an InputStream instead of taking a byte[]. Both the direct upload and the download-piped-into-upload variant pass in about 2 seconds each, so nothing is buffered on either side.

@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@geoand geoand 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.

The SendMultiBytesTest failure in the Semeru run is very suspicious. We really need to understand what's going on

The REST Client already streams a `Multi<io.vertx.core.buffer.Buffer>`
body without buffering it, but this was the only accepted `Multi` item
type, so a `Multi<byte[]>` produced by another client call (or by any
byte-oriented source) had to be converted by the caller.

Accept `Multi<byte[]>` in the build-time check and convert items to
`Buffer` when the body is sent. A downloaded `Multi<byte[]>` can now be
passed straight to another client method as the request body.

A `Multi` body that fails part-way used to be reported as a success:
`HttpClientRequest.send(ReadStream)` pipes the stream with the default
`endOnFailure(true)`, and the Mutiny `ReadStreamSubscriber` invokes the
end handler after the exception handler, so the chunked body was ended
normally and the server response to the truncated body was returned to
the caller. Pipe the stream explicitly with `endOnFailure(false)` and
reset the request with the source failure, as the multipart path already
does, so the call fails with a `ProcessingException`.

The documentation listed `Multi<io.vertx.mutiny.core.buffer.Buffer>`,
a type that no longer exists with the Vert.x 5 Mutiny bindings; it is
replaced by the two types that are actually accepted.

Relates to quarkusio#21440
@jnbdz
jnbdz force-pushed the rest-client-multi-bytes-upload branch from 90109a0 to 0c3abc9 Compare September 8, 2026 16:34
@jnbdz

jnbdz commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Agreed, it was a real bug, and it is on the request side, not in the test.

What happened on Semeru: the download Multi<byte[]> failed part-way (the first run shows the module running out of heap; MultiInvoker emits with the default unbounded buffer and never pauses the response, which is what #56462 addresses). What turned that into a wrong result instead of an error is how the body was sent: HttpClientRequest.send(ReadStream) calls pipeTo and drops the pipe future, and Vert.x's pipe defaults to endOnFailure(true). Mutiny's ReadStreamSubscriber invokes the end handler after the exception handler, so a failed Multi ends the chunked body normally, the server sees a complete request and its response to the truncated body is returned to the caller as a success. That is what the expected: 2147483648L but was: 1873247872L failure is. This was already the behaviour for Multi<Buffer> before this PR; pipeDownloadIntoUpload is the first test that exercises a failing source.

Fixed in 0c3abc9f3b4: the Multi body is now piped explicitly with endOnFailure(false) and the request is reset with the source failure, the same way the multipart path already does it, so the call fails with a ProcessingException carrying the original cause. SendMultiBytesTest.failingBodyFailsTheCall covers it: a body that fails after 100 chunks used to return the partial count and now throws (verified failing before the change and passing after it).

The 2 GB pipe test itself stays. Whether it passes on Semeru also depends on the download side not filling the heap, so it is worth a run once #56462 is in as well.

@quarkus-bot

quarkus-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 0c3abc9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot

This comment has been minimized.

Comment on lines +220 to +239
// a failing Multi must fail the request: with the default pipe behaviour the body would be ended
// normally and the server response to the truncated body would be reported as a success
ReadStream<Buffer> body = ReadStreamSubscriber.asReadStream(buffers,
new Function<>() {
@Override
public Buffer apply(Buffer buffer) {
return buffer;
}
}));
});
Pipe<Buffer> pipe = body.pipe();
pipe.endOnFailure(false);
pipe.to(httpClientRequest).onComplete(new Handler<>() {
@Override
public void handle(AsyncResult<Void> ar) {
if (ar.failed()) {
httpClientRequest.reset(0L, ar.cause());
}
}
});
Future<HttpClientResponse> sent = httpClientRequest.response();

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.

@jponge @vietj can you please verify that this is the correct thing to do?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the piping here is correct

@quarkus-bot

quarkus-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 0c3abc9.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
✔️ JVM Tests - JDK 21 Logs Raw logs 🚧
✔️ JVM Tests - JDK 25 Logs Raw logs 🚧
✔️ JVM Tests - JDK 25 Semeru Logs Raw logs 🚧
JVM Tests - JDK 21 Windows Build Failures Logs Raw logs 🔍

You can consult the Develocity build scans.

Failures

⚙️ JVM Tests - JDK 21 Windows #

- Failing: extensions/resteasy-reactive/rest-client/deployment 
! Skipped: extensions/keycloak-admin-rest-client/deployment extensions/liquibase/liquibase-mongodb/deployment extensions/micrometer-opentelemetry/deployment and 19 more

📦 extensions/resteasy-reactive/rest-client/deployment

Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.6:test (default-test) on project quarkus-rest-client-deployment: There was a timeout in the fork

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants