Skip to content

fix: surface CRT request-body publisher errors - #7307

Open
zoewangg wants to merge 1 commit into
masterfrom
zoewang/crt-request-body-publisher-error
Open

fix: surface CRT request-body publisher errors#7307
zoewangg wants to merge 1 commit into
masterfrom
zoewang/crt-request-body-publisher-error

Conversation

@zoewangg

Copy link
Copy Markdown
Contributor

Motivation and Context

When uploading through a CRT-based client with AsyncRequestBody.fromPublisher(publisher) and the publisher emits onError, the exception was printed to the process stderr on a CRT event-loop thread, e.g.:

Exception in thread "AwsEventLoop6" java.lang.IllegalArgumentException: test error

The upload still failed, so the symptom was unexpected stderr noise plus a misleading exception on the returned future. The Netty client does not do this. Reported in #6715 (CRT-based S3 client via S3AsyncClient.crtCreate() + S3TransferManager.upload()).

Root cause. ByteBufferStoringSubscriber.onError stores the error, and transferTo(...) re-throws it as a RuntimeException. That transferTo call runs inside the CRT request-body adapter's sendRequestBody, which is a native JNI upcall (the adapter implements CRT's HttpRequestBodyStream) with no try/catch. When the upcall returns with a pending Java exception, CRT's native glue calls ExceptionDescribe (which prints the exception + stack trace to the process's native stderr) and then ExceptionClear, and raises AWS_ERROR_HTTP_CALLBACK_FAILURE. So the exception is printed and discarded natively, and the future completes with a generic SdkClientException("Failed to send the request: ...CALLBACK_FAILURE") whose cause is null — the original error survives only on stderr.

This affects both CRT request-body adapters: S3CrtRequestBodyStreamAdapter (S3 CRT client, the one #6715 reports) and the structurally identical CrtRequestBodyAdapter (aws-crt-client).

Modifications

Two coordinated parts, no public API changes (all @SdkInternalApi):

1. Contain the error at the JNI boundary (both adapters). sendRequestBody now wraps the transferTo(...) call in try/catch (RuntimeException). On a publisher error it fails the operation with the original error instead of letting it escape the upcall, and returns false (never true — returning "body complete" would upload a zero-byte object and report success). ByteBufferStoringSubscriber.transferTo's throwing contract is unchanged; the fix is contained at the adapter.

2. Deliver the original error to the caller, exactly once (per-client wiring). The caller-facing future is driven by SdkAsyncHttpResponseHandler.onError, not by the HTTP client's execute future, so the two clients differ in how the error is surfaced:

  • S3 CRT (S3CrtRequestBodyStreamAdapter + S3CrtAsyncHttpClient + S3CrtResponseHandlerAdapter): the adapter completes the operation's execute future exceptionally with the original error. S3CrtAsyncHttpClient wires that future (the same resultFuture the response handler holds) into the adapter. S3CrtResponseHandlerAdapter's resultFuture.whenComplete now delivers responseHandler.onError(originalError) first and then cancels + closes the meta request (reusing the existing cleanup wiring). A one-shot AtomicBoolean guard around the single onError notification ensures the later, cancel-derived AWS_ERROR_S3_CANCELED onFinished does not re-notify — first error (the original) wins.

  • aws-crt-client (CrtRequestBodyAdapter + CrtRequestAdapter + CrtAsyncRequestExecutor + CrtResponseAdapter): the adapter routes the body error through CrtResponseAdapter.failRequest(...), which delivers onError(originalError) and completes the request future. Completing the future triggers closeConnection(), which cancels the stream so CRT delivers a cancel-derived onResponseComplete; a one-shot guard around CrtResponseAdapter's onError chokepoint suppresses that second notification. (This also fixes a latent double-onError on the existing onResponseBody-write-failure path.)

Net: on both clients, a request-body publisher error now produces no native stderr line and completes the caller's future exceptionally carrying the original error, delivered to onError exactly once — matching the Netty client.

Testing

Adapter/handler-level unit tests plus two end-to-end WireMock functional tests that exercise the real native CRT path.

  • S3CrtRequestBodyStreamAdapterTest: sendRequestBody no longer throws on a publisher error; the wired execute future completes exceptionally with the original error (RuntimeException as-is; IOException wrapped as UncheckedIOException); repeated calls stay quiet and preserve the original error. (Updated the two tests that previously asserted sendRequestBody throws.)
  • S3CrtResponseHandlerAdapterTest: completing the execute future with a body error delivers onError exactly once with the original error, cancels + closes the meta request, and a subsequent onFinished(S3_CANCELED) does not re-notify; same exactly-once behavior for a pipeline-forwarded timeout (ApiCallAttemptTimeoutException).
  • CrtRequestBodyAdapterTest (new) and CrtResponseHandlerTest: the body-error sink is invoked once with the original error; failRequest(...) followed by a cancel-derived onResponseComplete notifies onError exactly once with the original error.
  • AwsCrtAsyncHttpClientWireMockTest (new e2e): a real AwsCrtAsyncHttpClient PUT to WireMock with an erroring request-body publisher completes the returned future exceptionally with the original IllegalArgumentException.
  • S3CrtClientWiremockTest (new e2e): a real S3AsyncClient.crtBuilder() (endpoint-overridden to WireMock) putObject with an erroring body completes the future exceptionally with the original error.

Note: the stderr line is written by native ExceptionDescribe, which bypasses both System.setErr(...) and Thread.setDefaultUncaughtExceptionHandler, so it is not assertable at the Java layer; the tests verify the future carries the original error, which is the meaningful, executed proof that the exception no longer escapes the upcall.

Build scope: services/s3 and http-clients/aws-crt-client built with mvn install (module-level), all affected tests green. A full-repo mvn install was not run.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

License

  • I confirm that this pull request can be released under the Apache 2 license

Closes #6715

The CRT request-body adapters let a publisher onError escape the native
sendRequestBody upcall, so CRT printed it to stderr and failed the
future with a generic wrapper. Catch it at the adapter and fail the
operation with the original error, delivered to onError exactly once.

Closes #6715
@zoewangg
zoewangg requested a review from a team as a code owner August 21, 2026 19:46
@zoewangg
zoewangg requested a review from joviegas August 21, 2026 19:51
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS CRT Async HTTP Client",

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.

question (non-blocking): The sync path CrtRequestInputStreamAdapter#sendRequestBody also throws a RuntimeException out of the native CRT upcall. Does it hit the same stderr-and-discard issue as #6715 ?

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.

[CRT Client] Publisher error appears on AwsEventLoop threads in addition to CompletableFuture

2 participants