Skip to content

Commit

Permalink
Add AbstractUnsafe.annotateConnectException()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
trustin committed Oct 14, 2014
1 parent 0b935b8 commit f8349f8
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import io.netty.util.internal.StringUtil;

import java.io.IOException;
import java.net.ConnectException;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.nio.ByteBuffer;
Expand Down Expand Up @@ -549,13 +548,8 @@ public void operationComplete(ChannelFuture future) throws Exception {
});
}
} 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));
}
}

Expand Down Expand Up @@ -607,13 +601,7 @@ private void finishConnect() {
}
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
Expand Down
24 changes: 24 additions & 0 deletions transport/src/main/java/io/netty/channel/AbstractChannel.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@
import io.netty.util.internal.logging.InternalLoggerFactory;

import java.io.IOException;
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;
Expand Down Expand Up @@ -810,6 +813,27 @@ private void invokeLater(Runnable task) {
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;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
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;
Expand Down Expand Up @@ -233,12 +232,7 @@ public void operationComplete(ChannelFuture future) throws Exception {
});
}
} 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();
}
}
Expand Down Expand Up @@ -287,13 +281,7 @@ public final void finishConnect() {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import io.netty.channel.EventLoop;
import io.netty.channel.ThreadPerChannelEventLoop;

import java.net.ConnectException;
import java.net.SocketAddress;

/**
Expand Down Expand Up @@ -75,12 +74,7 @@ public void connect(
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();
}
}
Expand Down

0 comments on commit f8349f8

Please sign in to comment.