From b5461ecdb95fd02c1cb8647f7a11ada1560646e5 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:01:56 -0700 Subject: [PATCH] Include channel diagnostics in Read/Write timeout error messages in netty --- .../bugfix-NettyNIOHTTPClient-ce5dc9d.json | 6 + .../nio/netty/internal/utils/NettyUtils.java | 31 ++-- .../netty/internal/utils/NettyUtilsTest.java | 145 +++++++++--------- 3 files changed, 97 insertions(+), 85 deletions(-) create mode 100644 .changes/next-release/bugfix-NettyNIOHTTPClient-ce5dc9d.json diff --git a/.changes/next-release/bugfix-NettyNIOHTTPClient-ce5dc9d.json b/.changes/next-release/bugfix-NettyNIOHTTPClient-ce5dc9d.json new file mode 100644 index 000000000000..ad58c85df355 --- /dev/null +++ b/.changes/next-release/bugfix-NettyNIOHTTPClient-ce5dc9d.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "Netty NIO HTTP Client", + "contributor": "", + "description": "Include channel diagnostics in Read/Write timeout error messages to aid debugging." +} diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java index 6a9d0eedb682..f8859f6993f6 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java @@ -78,14 +78,23 @@ private NettyUtils() { public static Throwable decorateException(Channel channel, Throwable originalCause) { if (isAcquireTimeoutException(originalCause)) { return new Throwable(getMessageForAcquireTimeoutException(), originalCause); - } else if (isTooManyPendingAcquiresException(originalCause)) { + } + + if (isTooManyPendingAcquiresException(originalCause)) { return new Throwable(getMessageForTooManyAcquireOperationsError(), originalCause); - } else if (originalCause instanceof ReadTimeoutException) { - return new IOException("Read timed out", originalCause); - } else if (originalCause instanceof WriteTimeoutException) { - return new IOException("Write timed out", originalCause); - } else if (originalCause instanceof ClosedChannelException || isConnectionResetException(originalCause)) { - return new IOException(NettyUtils.closedChannelMessage(channel), originalCause); + } + + if (originalCause instanceof ReadTimeoutException) { + return new IOException(errorMessageWithChannelDiagnostics(channel, "Read timed out"), + originalCause); + } + + if (originalCause instanceof WriteTimeoutException) { + return new IOException(errorMessageWithChannelDiagnostics(channel, "Write timed out"), originalCause); + } + + if (originalCause instanceof ClosedChannelException || isConnectionResetException(originalCause)) { + return new IOException(closedChannelMessage(channel), originalCause); } return originalCause; @@ -152,7 +161,7 @@ private static String getMessageForTooManyAcquireOperationsError() { + "AWS, or by increasing the number of hosts sending requests."; } - public static String closedChannelMessage(Channel channel) { + public static String errorMessageWithChannelDiagnostics(Channel channel, String message) { ChannelDiagnostics channelDiagnostics = channel != null && channel.attr(CHANNEL_DIAGNOSTICS) != null ? channel.attr(CHANNEL_DIAGNOSTICS).get() : null; ChannelDiagnostics parentChannelDiagnostics = channel != null && channel.parent() != null && @@ -160,7 +169,7 @@ public static String closedChannelMessage(Channel channel) { channel.parent().attr(CHANNEL_DIAGNOSTICS).get() : null; StringBuilder error = new StringBuilder(); - error.append(CLOSED_CHANNEL_ERROR_MESSAGE); + error.append(message); if (channelDiagnostics != null) { error.append(" Channel Information: ").append(channelDiagnostics); @@ -173,6 +182,10 @@ public static String closedChannelMessage(Channel channel) { return error.toString(); } + public static String closedChannelMessage(Channel channel) { + return errorMessageWithChannelDiagnostics(channel, CLOSED_CHANNEL_ERROR_MESSAGE); + } + /** * Creates a {@link BiConsumer} that notifies the promise of any failures either via the {@link Throwable} passed into the * BiConsumer of as a result of running the successFunction. diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtilsTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtilsTest.java index ce3526ba8efd..d5c02a1f46b4 100644 --- a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtilsTest.java +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtilsTest.java @@ -43,12 +43,17 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Stream; import javax.net.ssl.SSLEngine; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mockito; import org.slf4j.Logger; +import software.amazon.awssdk.http.nio.netty.internal.ChannelDiagnostics; import software.amazon.awssdk.http.nio.netty.internal.MockChannel; public class NettyUtilsTest { @@ -274,108 +279,96 @@ public void closedChannelMessage_with_nullParentChannelAttribute() throws Except .isEqualTo(NettyUtils.CLOSED_CHANNEL_ERROR_MESSAGE); } - @Test - public void decorateException_with_TimeoutException() { + private static Stream decorateExceptionWrappedCases() { + return Stream.of( + Arguments.of(new TimeoutException("...Acquire operation took longer..."), + Throwable.class, TimeoutException.class), + Arguments.of(new IllegalStateException("...Too many outstanding acquire operations..."), + Throwable.class, IllegalStateException.class), + Arguments.of(new ReadTimeoutException(), + IOException.class, ReadTimeoutException.class), + Arguments.of(new WriteTimeoutException(), + IOException.class, WriteTimeoutException.class), + Arguments.of(new ClosedChannelException(), + IOException.class, ClosedChannelException.class), + Arguments.of(new IOException("...Connection reset by peer..."), + IOException.class, IOException.class) + ); + } + @ParameterizedTest + @MethodSource("decorateExceptionWrappedCases") + public void decorateException_wrapsException(Throwable input, + Class expectedType, + Class expectedCauseType) { Channel channel = mock(Channel.class); - Throwable timeoutException = new TimeoutException("...Acquire operation took longer..."); - Throwable output = NettyUtils.decorateException(channel, timeoutException); + Throwable output = NettyUtils.decorateException(channel, input); - assertThat(output).isInstanceOf(Throwable.class); - assertThat(output.getCause()).isInstanceOf(TimeoutException.class); + assertThat(output).isInstanceOf(expectedType); + assertThat(output.getCause()).isInstanceOf(expectedCauseType); assertThat(output.getMessage()).isNotNull(); } - @Test - public void decorateException_with_TimeoutException_noMsg() { - - Channel channel = mock(Channel.class); - Throwable timeoutException = new TimeoutException(); - Throwable output = NettyUtils.decorateException(channel, timeoutException); - - assertThat(output).isInstanceOf(TimeoutException.class); - assertThat(output.getCause()).isNull(); + private static Stream decorateExceptionPassthroughCases() { + return Stream.of( + Arguments.of(new TimeoutException()), + Arguments.of(new IllegalStateException()), + Arguments.of(new IOException()) + ); } - @Test - public void decorateException_with_IllegalStateException() { - + @ParameterizedTest + @MethodSource("decorateExceptionPassthroughCases") + public void decorateException_noMatchingMessage_returnsOriginal(Throwable input) { Channel channel = mock(Channel.class); - Throwable illegalStateException = new IllegalStateException("...Too many outstanding acquire operations..."); - Throwable output = NettyUtils.decorateException(channel, illegalStateException); + Throwable output = NettyUtils.decorateException(channel, input); - assertThat(output).isInstanceOf(Throwable.class); - assertThat(output.getCause()).isInstanceOf(IllegalStateException.class); - assertThat(output.getMessage()).isNotNull(); + assertThat(output).isSameAs(input); } - @Test - public void decorateException_with_IllegalStateException_noMsg() { - - Channel channel = mock(Channel.class); - Throwable illegalStateException = new IllegalStateException(); - Throwable output = NettyUtils.decorateException(channel, illegalStateException); - - assertThat(output).isInstanceOf(IllegalStateException.class); - assertThat(output.getCause()).isNull(); + private static Stream channelDiagnosticsExceptionCases() { + return Stream.of( + Arguments.of(new ReadTimeoutException(), "Read timed out"), + Arguments.of(new WriteTimeoutException(), "Write timed out") + ); } - @Test - public void decorateException_with_ReadTimeoutException() { - + @ParameterizedTest + @MethodSource("channelDiagnosticsExceptionCases") + public void decorateException_includesChannelDiagnostics(Throwable input, String expectedPrefix) { Channel channel = mock(Channel.class); - Throwable readTimeoutException = new ReadTimeoutException(); - Throwable output = NettyUtils.decorateException(channel, readTimeoutException); - - assertThat(output).isInstanceOf(IOException.class); - assertThat(output.getCause()).isInstanceOf(ReadTimeoutException.class); - assertThat(output.getMessage()).isNotNull(); - } - - @Test - public void decorateException_with_WriteTimeoutException() { + Attribute attribute = mock(Attribute.class); + ChannelDiagnostics diagnostics = new ChannelDiagnostics(channel); + when(channel.attr(any())).thenReturn(attribute); + when(attribute.get()).thenReturn(diagnostics); + when(channel.parent()).thenReturn(null); - Channel channel = mock(Channel.class); - Throwable writeTimeoutException = new WriteTimeoutException(); - Throwable output = NettyUtils.decorateException(channel, writeTimeoutException); + Throwable output = NettyUtils.decorateException(channel, input); assertThat(output).isInstanceOf(IOException.class); - assertThat(output.getCause()).isInstanceOf(WriteTimeoutException.class); - assertThat(output.getMessage()).isNotNull(); + assertThat(output.getMessage()).startsWith(expectedPrefix); + assertThat(output.getMessage()).contains("Channel Information:"); } @Test - public void decorateException_with_ClosedChannelException() { - - Channel channel = mock(Channel.class); - Throwable closedChannelException = new ClosedChannelException(); - Throwable output = NettyUtils.decorateException(channel, closedChannelException); - - assertThat(output).isInstanceOf(IOException.class); - assertThat(output.getCause()).isInstanceOf(ClosedChannelException.class); - assertThat(output.getMessage()).isNotNull(); + public void errorMessageWithChannelDiagnostics_nullChannel_returnsBaseMessage() { + assertThat(NettyUtils.errorMessageWithChannelDiagnostics(null, "test message")) + .isEqualTo("test message"); } @Test - public void decorateException_with_IOException_reset() { - + public void errorMessageWithChannelDiagnostics_withDiagnostics_appendsChannelInfo() { Channel channel = mock(Channel.class); - Throwable closedChannelException = new IOException("...Connection reset by peer..."); - Throwable output = NettyUtils.decorateException(channel, closedChannelException); - - assertThat(output).isInstanceOf(IOException.class); - assertThat(output.getCause()).isInstanceOf(IOException.class); - assertThat(output.getMessage()).isNotNull(); - } - - @Test - public void decorateException_with_IOException_noMsg() { + Attribute attribute = mock(Attribute.class); + ChannelDiagnostics diagnostics = new ChannelDiagnostics(channel); + when(channel.attr(any())).thenReturn(attribute); + when(attribute.get()).thenReturn(diagnostics); + when(channel.parent()).thenReturn(null); - Channel channel = mock(Channel.class); - Throwable closedChannelException = new IOException(); - Throwable output = NettyUtils.decorateException(channel, closedChannelException); + String result = NettyUtils.errorMessageWithChannelDiagnostics(channel, "custom error"); - assertThat(output).isInstanceOf(IOException.class); - assertThat(output.getCause()).isNull(); + assertThat(result).startsWith("custom error"); + assertThat(result).contains("Channel Information:"); } + }