AsyncResponseHandler.onError(Throwable) throws an NPE if called before prepare().

[joelc]

Describe the bug

software.amazon.awssdk.core.internal.http.async.AsyncResponseHandler.onError(Throwable) throws an NPE if called before prepare().

This causes the Throwable passed to onError to be swallowed.

Expected Behavior

Do not swallow the provided Throwable. Callers have no way of knowing what the actual cause was.

Current Behavior

It throws an NPE and swallows the provided Throwable.

2021-01-21 19:37:38,564 [HOST] ERROR [FunctionalUtils.java: 42] - 0 0 - Error thrown from TransformingAsyncResponseHandler#onError, ignoring.
java.lang.NullPointerException: null
	at software.amazon.awssdk.core.internal.http.async.AsyncResponseHandler.onError(AsyncResponseHandler.java:76)
	at software.amazon.awssdk.core.internal.http.async.CombinedResponseAsyncHttpResponseHandler.onError(CombinedResponseAsyncHttpResponseHandler.java:70)
	at software.amazon.awssdk.core.internal.handler.BaseAsyncClientHandler.lambda$doExecute$5(BaseAsyncClientHandler.java:230)
	at software.amazon.awssdk.utils.FunctionalUtils.runAndLogError(FunctionalUtils.java:40)
	at software.amazon.awssdk.core.internal.handler.BaseAsyncClientHandler.doExecute(BaseAsyncClientHandler.java:227)
	at software.amazon.awssdk.core.internal.handler.BaseAsyncClientHandler.lambda$execute$1(BaseAsyncClientHandler.java:90)
	at software.amazon.awssdk.core.internal.handler.BaseAsyncClientHandler.measureApiCallSuccess(BaseAsyncClientHandler.java:275)
	at software.amazon.awssdk.core.internal.handler.BaseAsyncClientHandler.execute(BaseAsyncClientHandler.java:75)
	at software.amazon.awssdk.awscore.client.handler.AwsAsyncClientHandler.execute(AwsAsyncClientHandler.java:52)
	at software.amazon.awssdk.services.sns.DefaultSnsAsyncClient.publish(DefaultSnsAsyncClient.java:2281)

Steps to Reproduce

This unit test throws an NPE and there's no trace of the NumberFormatException that was provided to onError; it's written to pass with the code in "Possible Solution" below.

@Test
public void shouldNotSwallowThrowablePassedToOnErrorBeforePrepare() {
    // given
    AsyncResponseHandler sut = new AsyncResponseHandler(null, null, null);

    // when
    ThrowingCallable tc = () -> sut.onError(new NumberFormatException());

    // then
    assertThatThrownBy(tc).isInstanceOf(IllegalStateException.class)
                          .hasCauseInstanceOf(NumberFormatException.class);
}

Possible Solution

Do not throw an NPE that swallows err:

@Override
public void onError(Throwable err) {
    if (streamFuture == null) {
        throw new IllegalStateException("onError called before prepare", err);
    }

    streamFuture.completeExceptionally(err);
}

Context

Under high load, we have some software that's misbehaving. It's causing (we think) requests to be canceled before they're prepared for execution. With this onError implementation, we get to see the underlying exception in our logs.

Your Environment

• AWS Java SDK version used: 2.15.*
• JDK version used: java version "11.0.6" 2020-01-14 LTS
• Operating System and version: Various RHELs

[debora-ito]

Hi @joelc sorry for the delayed response. Looks like a bug, marking as a bug.

[debora-ito]

Related to #1812.

[debora-ito]

This should be fixed by this commit: https://github.com/aws/aws-sdk-java-v2/pull/4567/files#diff-18f6745a85126bf1b23736af8857375b8e7328956262a3d7f08e5bb5b7684303.

The fix is included in SDK version 2.21.1.

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.