From e4ab108a10a57c82d1e76772542a44b2cc167fd3 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Tue, 16 Sep 2014 19:25:26 +0900 Subject: [PATCH] Add AbstractUnsafe.annotateConnectException() Motivation: JDK's exception messages triggered by a connection attempt failure do not contain the related remote address in its message. We currently append the remote address to ConnectException's message, but I found that we need to cover more exception types such as SocketException. Modifications: - Add AbstractUnsafe.annotateConnectException() to de-duplicate the code that appends the remote address Result: - Less duplication - A transport implementor can annotate connection attempt failure message more easily --- .../channel/epoll/EpollSocketChannel.java | 15 ++--------- .../io/netty/channel/AbstractChannel.java | 25 ++++++++++++++++++- .../netty/channel/nio/AbstractNioChannel.java | 16 ++---------- .../netty/channel/oio/AbstractOioChannel.java | 8 +----- 4 files changed, 29 insertions(+), 35 deletions(-) diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java index 16a21ff8d9..c40352d245 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java @@ -549,13 +549,8 @@ public final class EpollSocketChannel extends AbstractEpollChannel implements So }); } } catch (Throwable t) { - if (t instanceof ConnectException) { - Throwable newT = new ConnectException(t.getMessage() + ": " + remoteAddress); - newT.setStackTrace(t.getStackTrace()); - t = newT; - } closeIfClosed(); - promise.tryFailure(t); + promise.tryFailure(annotateConnectException(t, remoteAddress)); } } @@ -607,13 +602,7 @@ public final class EpollSocketChannel extends AbstractEpollChannel implements So } fulfillConnectPromise(connectPromise, wasActive); } catch (Throwable t) { - if (t instanceof ConnectException) { - Throwable newT = new ConnectException(t.getMessage() + ": " + requestedRemoteAddress); - newT.setStackTrace(t.getStackTrace()); - t = newT; - } - - fulfillConnectPromise(connectPromise, t); + fulfillConnectPromise(connectPromise, annotateConnectException(t, requestedRemoteAddress)); } finally { if (!connectStillInProgress) { // Check for null as the connectTimeoutFuture is only created if a connectTimeoutMillis > 0 is used diff --git a/transport/src/main/java/io/netty/channel/AbstractChannel.java b/transport/src/main/java/io/netty/channel/AbstractChannel.java index 7f076aa2c5..8cef806819 100644 --- a/transport/src/main/java/io/netty/channel/AbstractChannel.java +++ b/transport/src/main/java/io/netty/channel/AbstractChannel.java @@ -24,12 +24,14 @@ import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; +import java.net.ConnectException; import java.net.InetSocketAddress; +import java.net.NoRouteToHostException; import java.net.SocketAddress; +import java.net.SocketException; import java.nio.channels.ClosedChannelException; import java.nio.channels.NotYetConnectedException; import java.util.concurrent.RejectedExecutionException; -import java.util.concurrent.atomic.AtomicBoolean; /** * A skeletal {@link Channel} implementation. @@ -847,6 +849,27 @@ public abstract class AbstractChannel extends DefaultAttributeMap implements Cha logger.warn("Can't invoke task later as EventLoop rejected it", e); } } + + /** + * Appends the remote address to the message of the exceptions caused by connection attempt failure. + */ + protected final Throwable annotateConnectException(Throwable cause, SocketAddress remoteAddress) { + if (cause instanceof ConnectException) { + Throwable newT = new ConnectException(cause.getMessage() + ": " + remoteAddress); + newT.setStackTrace(cause.getStackTrace()); + cause = newT; + } else if (cause instanceof NoRouteToHostException) { + Throwable newT = new NoRouteToHostException(cause.getMessage() + ": " + remoteAddress); + newT.setStackTrace(cause.getStackTrace()); + cause = newT; + } else if (cause instanceof SocketException) { + Throwable newT = new SocketException(cause.getMessage() + ": " + remoteAddress); + newT.setStackTrace(cause.getStackTrace()); + cause = newT; + } + + return cause; + } } /** diff --git a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java index 3fed6c7048..f75c4307b3 100644 --- a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java +++ b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java @@ -34,7 +34,6 @@ import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; import java.io.IOException; -import java.net.ConnectException; import java.net.SocketAddress; import java.nio.channels.CancelledKeyException; import java.nio.channels.SelectableChannel; @@ -228,12 +227,7 @@ public abstract class AbstractNioChannel extends AbstractChannel { }); } } catch (Throwable t) { - if (t instanceof ConnectException) { - Throwable newT = new ConnectException(t.getMessage() + ": " + remoteAddress); - newT.setStackTrace(t.getStackTrace()); - t = newT; - } - promise.tryFailure(t); + promise.tryFailure(annotateConnectException(t, remoteAddress)); closeIfClosed(); } } @@ -282,13 +276,7 @@ public abstract class AbstractNioChannel extends AbstractChannel { doFinishConnect(); fulfillConnectPromise(connectPromise, wasActive); } catch (Throwable t) { - if (t instanceof ConnectException) { - Throwable newT = new ConnectException(t.getMessage() + ": " + requestedRemoteAddress); - newT.setStackTrace(t.getStackTrace()); - t = newT; - } - - fulfillConnectPromise(connectPromise, t); + fulfillConnectPromise(connectPromise, annotateConnectException(t, requestedRemoteAddress)); } finally { // Check for null as the connectTimeoutFuture is only created if a connectTimeoutMillis > 0 is used // See https://github.com/netty/netty/issues/1770 diff --git a/transport/src/main/java/io/netty/channel/oio/AbstractOioChannel.java b/transport/src/main/java/io/netty/channel/oio/AbstractOioChannel.java index bd163b2799..1c7619714b 100644 --- a/transport/src/main/java/io/netty/channel/oio/AbstractOioChannel.java +++ b/transport/src/main/java/io/netty/channel/oio/AbstractOioChannel.java @@ -21,7 +21,6 @@ import io.netty.channel.ChannelPromise; import io.netty.channel.EventLoop; import io.netty.channel.ThreadPerChannelEventLoop; -import java.net.ConnectException; import java.net.SocketAddress; /** @@ -75,12 +74,7 @@ public abstract class AbstractOioChannel extends AbstractChannel { pipeline().fireChannelActive(); } } catch (Throwable t) { - if (t instanceof ConnectException) { - Throwable newT = new ConnectException(t.getMessage() + ": " + remoteAddress); - newT.setStackTrace(t.getStackTrace()); - t = newT; - } - safeSetFailure(promise, t); + safeSetFailure(promise, annotateConnectException(t, remoteAddress)); closeIfClosed(); } }