HTTPCLIENT-1362: better error messages for connect timed out and connection refused exceptions

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1488966 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -1,6 +1,10 @@
 Changes since release 4.3 BETA1
 -------------------
 
+* [HTTPCLIENT-1362] Better error messages for connect timed out and connection refused 
+  exceptions.
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
 * [HTTPCLIENT-1360] separate out DeflateInputStream as an independent class,
   so it can be used by others.
   Contributed by Karl Wright <kwright at apache.org>

httpclient/src/main/java/org/apache/http/conn/ConnectTimeoutException.java

@@ -27,6 +27,7 @@
 
 package org.apache.http.conn;
 
+import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.net.InetSocketAddress;
 
@@ -45,11 +46,16 @@ public class ConnectTimeoutException extends InterruptedIOException {
 
     private static final long serialVersionUID = -4816682903149535989L;
 
+    private final HttpHost host;
+    private final InetSocketAddress remoteAddress;
+
     /**
      * Creates a ConnectTimeoutException with a <tt>null</tt> detail message.
      */
     public ConnectTimeoutException() {
         super();
+        this.host = null;
+        this.remoteAddress = null;
     }
 
     /**
@@ -57,18 +63,40 @@ public class ConnectTimeoutException extends InterruptedIOException {
      */
     public ConnectTimeoutException(final String message) {
         super(message);
+        this.host = null;
+        this.remoteAddress = null;
     }
 
     /**
-     * Creates a ConnectTimeoutException with the specified detail message.
+     * Creates a ConnectTimeoutException based on original {@link IOException}.
      *
      * @since 4.3
      */
-    public ConnectTimeoutException(final HttpHost host, final InetSocketAddress remoteAddress) {
+    public ConnectTimeoutException(
+            final HttpHost host,
+            final InetSocketAddress remoteAddress,
+            final IOException cause) {
         super("Connect to " +
-                (host != null ? host.toHostString() : " remote host") +
+                (host != null ? host.toHostString() : "remote host") +
                 (remoteAddress != null ? " (" + remoteAddress.getAddress() + ")" : "")
-                + " timed out");
+                + ((cause != null && cause.getMessage() != null) ? " failed: " + cause.getMessage() : " timed out"));
+        this.host = host;
+        this.remoteAddress = remoteAddress;
+        initCause(cause);
+    }
+
+    /**
+     * @since 4.3
+     */
+    public HttpHost getHost() {
+        return host;
+    }
+
+    /**
+     * @since 4.3
+     */
+    public InetSocketAddress getRemoteAddress() {
+        return remoteAddress;
     }
 
 }

httpclient/src/main/java/org/apache/http/conn/HttpHostConnectException.java

@@ -26,7 +26,9 @@
  */
 package org.apache.http.conn;
 
+import java.io.IOException;
 import java.net.ConnectException;
+import java.net.InetSocketAddress;
 
 import org.apache.http.HttpHost;
 import org.apache.http.annotation.Immutable;
@@ -43,10 +45,27 @@ public class HttpHostConnectException extends ConnectException {
     private static final long serialVersionUID = -3194482710275220224L;
 
     private final HttpHost host;
+    private final InetSocketAddress remoteAddress;
 
     public HttpHostConnectException(final HttpHost host, final ConnectException cause) {
-        super("Connection to " + host + " refused");
+        this(host, null, cause);
+    }
+
+    /**
+     * Creates a HttpHostConnectException based on original {@link java.io.IOException}.
+     *
+     * @since 4.3
+     */
+    public HttpHostConnectException(
+            final HttpHost host,
+            final InetSocketAddress remoteAddress,
+            final IOException cause) {
+        super("Connect to " +
+                (host != null ? host.toHostString() : "remote host") +
+                (remoteAddress != null ? " (" + remoteAddress.getAddress() + ")" : "")
+                + ((cause != null && cause.getMessage() != null) ? " failed: " + cause.getMessage() : " refused"));
         this.host = host;
+        this.remoteAddress = remoteAddress;
         initCause(cause);
     }
 
@@ -54,4 +73,11 @@ public class HttpHostConnectException extends ConnectException {
         return this.host;
     }
 
+    /**
+     * @since 4.3
+     */
+    public InetSocketAddress getRemoteAddress() {
+        return remoteAddress;
+    }
+
 }

httpclient/src/main/java/org/apache/http/conn/socket/AbstractConnectionSocketFactory.java

@@ -0,0 +1,104 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.http.conn.socket;
+
+import org.apache.http.HttpHost;
+import org.apache.http.annotation.Immutable;
+import org.apache.http.conn.ConnectTimeoutException;
+import org.apache.http.conn.HttpHostConnectException;
+import org.apache.http.protocol.HttpContext;
+
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketTimeoutException;
+
+/**
+ * This is a base class for {@link ConnectionSocketFactory} implementations.
+ * This class provides a common exception handling logic for connect operations.
+ *
+ * @since 4.3
+ */
+public abstract class AbstractConnectionSocketFactory implements ConnectionSocketFactory {
+
+     /**
+     * Connects a socket to the target host with the given resolved remote address.
+     *
+     * @param connectTimeout connect timeout.
+     * @param socket the socket to connect, as obtained from {@link #createSocket(HttpContext)}.
+     * <code>null</code> indicates that a new socket should be created and connected.
+     * @param host target host as specified by the caller (end user).
+     * @param remoteAddress the resolved remote address to connect to.
+     * @param localAddress the local address to bind the socket to, or <code>null</code> for any.
+     * @param context the actual HTTP context.
+     *
+      * @return  the connected socket. The returned object may be different
+      *          from the <code>sock</code> argument if this factory supports
+      *          a layered protocol.
+     * @throws org.apache.http.conn.ConnectTimeoutException if the socket cannot be connected
+     *          within the time limit defined by connectTimeout parameter.
+     * @throws org.apache.http.conn.HttpHostConnectException if the connection is refused
+     *          by the opposite endpoint.
+     */
+    public Socket connectSocket(
+            final int connectTimeout,
+            final Socket socket,
+            final HttpHost host,
+            final InetSocketAddress remoteAddress,
+            final InetSocketAddress localAddress,
+            final HttpContext context) throws IOException {
+        final Socket sock = socket != null ? socket : createSocket(context);
+        if (localAddress != null) {
+            sock.bind(localAddress);
+        }
+        try {
+            sock.connect(remoteAddress, connectTimeout);
+        } catch (final SocketTimeoutException ex) {
+            closeSocket(socket);
+            throw new ConnectTimeoutException(host, remoteAddress, ex);
+        } catch (final ConnectException ex) {
+            closeSocket(socket);
+            String msg = ex.getMessage();
+            if ("Connection timed out".equals(msg)) {
+                throw new ConnectTimeoutException(host, remoteAddress, ex);
+            } else {
+                throw new HttpHostConnectException(host, remoteAddress, ex);
+            }
+        }
+        return sock;
+    }
+
+    private void closeSocket(final Socket sock) {
+        try {
+            sock.close();
+        } catch (final IOException ignore) {
+        }
+    }
+
+}

httpclient/src/main/java/org/apache/http/conn/socket/ConnectionSocketFactory.java

@@ -32,7 +32,6 @@ import java.net.InetSocketAddress;
 import java.net.Socket;
 
 import org.apache.http.HttpHost;
-import org.apache.http.conn.ConnectTimeoutException;
 import org.apache.http.protocol.HttpContext;
 
 /**
@@ -69,8 +68,6 @@ public interface ConnectionSocketFactory {
      *          a layered protocol.
      *
      * @throws IOException if an I/O error occurs
-     * @throws ConnectTimeoutException if the socket cannot be connected
-     *          within the time limit defined in the <code>params</code>
      */
     Socket connectSocket(
         int connectTimeout,
@@ -78,6 +75,6 @@ public interface ConnectionSocketFactory {
         HttpHost host,
         InetSocketAddress remoteAddress,
         InetSocketAddress localAddress,
-        HttpContext context) throws IOException, ConnectTimeoutException;
+        HttpContext context) throws IOException;
 
 }

httpclient/src/main/java/org/apache/http/conn/socket/PlainSocketFactory.java

@@ -28,13 +28,9 @@
 package org.apache.http.conn.socket;
 
 import java.io.IOException;
-import java.net.InetSocketAddress;
 import java.net.Socket;
-import java.net.SocketTimeoutException;
 
-import org.apache.http.HttpHost;
 import org.apache.http.annotation.Immutable;
-import org.apache.http.conn.ConnectTimeoutException;
 import org.apache.http.protocol.HttpContext;
 
 /**
@@ -43,7 +39,7 @@ import org.apache.http.protocol.HttpContext;
  * @since 4.3
  */
 @Immutable
-public class PlainSocketFactory implements ConnectionSocketFactory {
+public class PlainSocketFactory extends AbstractConnectionSocketFactory {
 
     public static final PlainSocketFactory INSTANCE = new PlainSocketFactory();
 
@@ -59,23 +55,4 @@ public class PlainSocketFactory implements ConnectionSocketFactory {
         return new Socket();
     }
 
-    public Socket connectSocket(
-            final int connectTimeout,
-            final Socket socket,
-            final HttpHost host,
-            final InetSocketAddress remoteAddress,
-            final InetSocketAddress localAddress,
-            final HttpContext context) throws IOException, ConnectTimeoutException {
-        final Socket sock = socket != null ? socket : createSocket(context);
-        if (localAddress != null) {
-            sock.bind(localAddress);
-        }
-        try {
-            sock.connect(remoteAddress, connectTimeout);
-        } catch (final SocketTimeoutException ex) {
-            throw new ConnectTimeoutException(host, remoteAddress);
-        }
-        return sock;
-    }
-
 }

httpclient/src/main/java/org/apache/http/conn/ssl/SSLSocketFactory.java

@@ -51,6 +51,7 @@ import org.apache.http.conn.scheme.HostNameResolver;
 import org.apache.http.conn.scheme.LayeredSchemeSocketFactory;
 import org.apache.http.conn.scheme.LayeredSocketFactory;
 import org.apache.http.conn.scheme.SchemeLayeredSocketFactory;
+import org.apache.http.conn.socket.AbstractConnectionSocketFactory;
 import org.apache.http.conn.socket.LayeredConnectionSocketFactory;
 import org.apache.http.params.HttpConnectionParams;
 import org.apache.http.params.HttpParams;
@@ -91,7 +92,8 @@ import org.apache.http.util.TextUtils;
  */
 @SuppressWarnings("deprecation")
 @ThreadSafe
-public class SSLSocketFactory implements LayeredConnectionSocketFactory, SchemeLayeredSocketFactory,
+public class SSLSocketFactory extends AbstractConnectionSocketFactory
+                              implements LayeredConnectionSocketFactory, SchemeLayeredSocketFactory,
                                          LayeredSchemeSocketFactory, LayeredSocketFactory {
 
     public static final String TLS   = "TLS";
@@ -548,19 +550,11 @@ public class SSLSocketFactory implements LayeredConnectionSocketFactory, SchemeL
             final HttpHost host,
             final InetSocketAddress remoteAddress,
             final InetSocketAddress localAddress,
-            final HttpContext context) throws IOException, ConnectTimeoutException {
+            final HttpContext context) throws IOException {
         Args.notNull(host, "HTTP host");
         Args.notNull(remoteAddress, "Remote address");
-        final Socket sock = socket != null ? socket : createSocket(context);
+        final Socket sock = super.connectSocket(
-        if (localAddress != null) {
+                connectTimeout, socket, host, remoteAddress, localAddress, context);
-            sock.bind(localAddress);
-        }
-        try {
-            sock.connect(remoteAddress, connectTimeout);
-        } catch (final SocketTimeoutException ex) {
-            closeSocket(sock);
-            throw new ConnectTimeoutException(host, remoteAddress);
-        }
         // Setup SSL layering if necessary
         if (sock instanceof SSLSocket) {
             final SSLSocket sslsock = (SSLSocket) sock;
@@ -572,18 +566,11 @@ public class SSLSocketFactory implements LayeredConnectionSocketFactory, SchemeL
         }
     }
 
-    private void closeSocket(final Socket sock) {
-        try {
-            sock.close();
-        } catch (final IOException ignore) {
-        }
-    }
-
     public Socket createLayeredSocket(
             final Socket socket,
             final String target,
             final int port,
-            final HttpContext context) throws IOException, UnknownHostException {
+            final HttpContext context) throws IOException {
         final SSLSocket sslsock = (SSLSocket) this.socketfactory.createSocket(
                 socket,
                 target,

httpclient/src/main/java/org/apache/http/impl/conn/DefaultClientConnectionOperator.java

@@ -186,7 +186,7 @@ public class DefaultClientConnectionOperator implements ClientConnectionOperator
                 return;
             } catch (final ConnectException ex) {
                 if (last) {
-                    throw new HttpHostConnectException(target, ex);
+                    throw ex;
                 }
             } catch (final ConnectTimeoutException ex) {
                 if (last) {
@@ -215,13 +215,8 @@ public class DefaultClientConnectionOperator implements ClientConnectionOperator
         Asserts.check(schm.getSchemeSocketFactory() instanceof LayeredConnectionSocketFactory,
             "Socket factory must implement SchemeLayeredSocketFactory");
         final SchemeLayeredSocketFactory lsf = (SchemeLayeredSocketFactory) schm.getSchemeSocketFactory();
-        Socket sock;
+        Socket sock = lsf.createLayeredSocket(
-        try {
+                conn.getSocket(), target.getHostName(), schm.resolvePort(target.getPort()), params);
-            sock = lsf.createLayeredSocket(
-                    conn.getSocket(), target.getHostName(), schm.resolvePort(target.getPort()), params);
-        } catch (final ConnectException ex) {
-            throw new HttpHostConnectException(target, ex);
-        }
         prepareSocket(sock, context, params);
         conn.update(sock, target, lsf.isSecure(sock), params);
     }

httpclient/src/main/java/org/apache/http/impl/conn/HttpClientConnectionOperator.java

@@ -124,7 +124,7 @@ class HttpClientConnectionOperator {
                 return;
             } catch (final ConnectException ex) {
                 if (last) {
-                    throw new HttpHostConnectException(host, ex);
+                    throw ex;
                 }
             } catch (final ConnectTimeoutException ex) {
                 if (last) {
@@ -155,12 +155,8 @@ class HttpClientConnectionOperator {
         }
         final LayeredConnectionSocketFactory lsf = (LayeredConnectionSocketFactory) sf;
         Socket sock = conn.getSocket();
-        try {
+        final int port = this.schemePortResolver.resolve(host);
-            final int port = this.schemePortResolver.resolve(host);
+        sock = lsf.createLayeredSocket(sock, host.getHostName(), port, context);
-            sock = lsf.createLayeredSocket(sock, host.getHostName(), port, context);
-        } catch (final ConnectException ex) {
-            throw new HttpHostConnectException(host, ex);
-        }
         conn.bind(sock);
     }
 

httpclient/src/test/java/org/apache/http/conn/TestExceptions.java

@@ -27,9 +27,14 @@
 
 package org.apache.http.conn;
 
+import org.apache.http.HttpHost;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+
 /**
  * Unit tests for exceptions.
  * Trivial, but it looks better in the Clover reports.
@@ -37,22 +42,83 @@ import org.junit.Test;
 public class TestExceptions {
 
     @Test
-    public void testCTX() {
+    public void testConnectTimeoutExceptionNullMessage() {
-        final String msg = "sample exception message";
+        final ConnectTimeoutException ctx = new ConnectTimeoutException();
-        ConnectTimeoutException ctx =
-            new ConnectTimeoutException(msg);
-        Assert.assertFalse(ctx.toString().indexOf(msg) < 0);
-        Assert.assertSame(msg, ctx.getMessage());
-
-        ctx = new ConnectTimeoutException();
         Assert.assertNull(ctx.getMessage());
     }
 
     @Test
-    public void testCPTX() {
+    public void testConnectTimeoutExceptionSimpleMessage() {
+        final ConnectTimeoutException ctx = new ConnectTimeoutException("sample exception message");
+        Assert.assertEquals("sample exception message", ctx.getMessage());
+    }
+
+    @Test
+    public void testConnectTimeoutExceptionFromNullCause() {
+        final ConnectTimeoutException ctx = new ConnectTimeoutException(null, null, null);
+        Assert.assertEquals("Connect to remote host timed out", ctx.getMessage());
+    }
+
+    @Test
+    public void testConnectTimeoutExceptionFromCause() {
+        final IOException cause = new IOException("something awful");
+        final ConnectTimeoutException ctx = new ConnectTimeoutException(null, null, cause);
+        Assert.assertEquals("Connect to remote host failed: something awful", ctx.getMessage());
+    }
+
+    @Test
+    public void testConnectTimeoutExceptionFromCauseAndHost() {
+        final HttpHost target = new HttpHost("localhost");
+        final IOException cause = new IOException();
+        final ConnectTimeoutException ctx = new ConnectTimeoutException(target, null, cause);
+        Assert.assertEquals("Connect to localhost timed out", ctx.getMessage());
+    }
+
+    @Test
+    public void testConnectTimeoutExceptionFromCauseHostAndRemoteAddress() throws Exception {
+        final HttpHost target = new HttpHost("localhost");
+        final InetSocketAddress remoteAddress = new InetSocketAddress(
+                InetAddress.getByAddress(new byte[] {1,2,3,4}), 1234);
+        final IOException cause = new IOException();
+        final ConnectTimeoutException ctx = new ConnectTimeoutException(target, remoteAddress, cause);
+        Assert.assertEquals("Connect to localhost (/1.2.3.4) timed out", ctx.getMessage());
+    }
+
+    @Test
+    public void testHttpHostConnectExceptionFromNullCause() {
+        final HttpHostConnectException ctx = new HttpHostConnectException(null, null, null);
+        Assert.assertEquals("Connect to remote host refused", ctx.getMessage());
+    }
+
+    @Test
+    public void testHttpHostConnectExceptionFromCause() {
+        final IOException cause = new IOException("something awful");
+        final HttpHostConnectException ctx = new HttpHostConnectException(null, null, cause);
+        Assert.assertEquals("Connect to remote host failed: something awful", ctx.getMessage());
+    }
+
+    @Test
+    public void testHttpHostConnectExceptionFromCauseAndHost() {
+        final HttpHost target = new HttpHost("localhost");
+        final IOException cause = new IOException();
+        final HttpHostConnectException ctx = new HttpHostConnectException(target, null, cause);
+        Assert.assertEquals("Connect to localhost refused", ctx.getMessage());
+    }
+
+    @Test
+    public void testHttpHostConnectExceptionFromCauseHostAndRemoteAddress() throws Exception {
+        final HttpHost target = new HttpHost("localhost");
+        final InetSocketAddress remoteAddress = new InetSocketAddress(
+                InetAddress.getByAddress(new byte[] {1,2,3,4}), 1234);
+        final IOException cause = new IOException();
+        final HttpHostConnectException ctx = new HttpHostConnectException(target, remoteAddress, cause);
+        Assert.assertEquals("Connect to localhost (/1.2.3.4) refused", ctx.getMessage());
+    }
+
+    @Test
+    public void testConnectionPoolTimeoutException() {
         final String msg = "sample exception message";
-        ConnectionPoolTimeoutException cptx =
+        ConnectionPoolTimeoutException cptx = new ConnectionPoolTimeoutException(msg);
-            new ConnectionPoolTimeoutException(msg);
         Assert.assertFalse(cptx.toString().indexOf(msg) < 0);
         Assert.assertSame(msg, cptx.getMessage());