You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2019/08/24 21:13:07 UTC
[httpcomponents-client] 01/01: HTTPCLIENT-2013: revised handling of
connect exceptions;
improved consistency in behavior of the classic and async clients;
ConnectTimeoutException now extends SocketTimeoutException
This is an automated email from the ASF dual-hosted git repository.
olegk pushed a commit to branch development
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git
commit d5e1392840fcbd9ff9d9c5b19d8ed9da2883c224
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sat Aug 24 15:34:22 2019 +0200
HTTPCLIENT-2013: revised handling of connect exceptions; improved consistency in behavior of the classic and async clients; ConnectTimeoutException now extends SocketTimeoutException
---
.../hc/client5/http/ConnectExceptionSupport.java | 87 ++++++++++++++++++++++
.../hc/client5/http/ConnectTimeoutException.java | 36 ++-------
.../hc/client5/http/HttpHostConnectException.java | 28 +++----
.../io/DefaultHttpClientConnectionOperator.java | 22 +-----
.../http/impl/nio/MultihomeIOSessionRequester.java | 4 +-
...tions.java => ConnectExceptionSupportTest.java} | 32 +++-----
.../client5/http/examples/ClientExecuteSOCKS.java | 8 +-
.../impl/TestDefaultHttpRequestRetryHandler.java | 6 +-
8 files changed, 120 insertions(+), 103 deletions(-)
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ConnectExceptionSupport.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ConnectExceptionSupport.java
new file mode 100644
index 0000000..49ab397
--- /dev/null
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ConnectExceptionSupport.java
@@ -0,0 +1,87 @@
+/*
+ * ====================================================================
+ * 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.hc.client5.http;
+
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.InetAddress;
+import java.net.SocketTimeoutException;
+import java.util.Arrays;
+
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.net.NamedEndpoint;
+
+/**
+ * Connect exception support methods.
+ *
+ * @since 5.0
+ */
+@Internal
+public final class ConnectExceptionSupport {
+
+ public static ConnectTimeoutException createConnectTimeoutException(
+ final IOException cause, final NamedEndpoint namedEndpoint, final InetAddress... remoteAddresses) {
+ final String message = "Connect to " +
+ (namedEndpoint != null ? namedEndpoint : "remote endpoint") +
+ (remoteAddresses != null && remoteAddresses.length > 0 ? " " + Arrays.asList(remoteAddresses) : "") +
+ ((cause != null && cause.getMessage() != null) ? " failed: " + cause.getMessage() : " timed out");
+ return new ConnectTimeoutException(message, namedEndpoint);
+ }
+
+ public static HttpHostConnectException createHttpHostConnectException(
+ final IOException cause,
+ final NamedEndpoint namedEndpoint,
+ final InetAddress... remoteAddresses) {
+ final String message = "Connect to " +
+ (namedEndpoint != null ? namedEndpoint : "remote endpoint") +
+ (remoteAddresses != null && remoteAddresses.length > 0 ? " " + Arrays.asList(remoteAddresses) : "") +
+ ((cause != null && cause.getMessage() != null) ? " failed: " + cause.getMessage() : " refused");
+ return new HttpHostConnectException(message, namedEndpoint);
+ }
+
+ public static IOException enhance(
+ final IOException cause, final NamedEndpoint namedEndpoint, final InetAddress... remoteAddresses) {
+ if (cause instanceof SocketTimeoutException) {
+ final IOException ex = createConnectTimeoutException(cause, namedEndpoint, remoteAddresses);
+ ex.setStackTrace(cause.getStackTrace());
+ return ex;
+ } else if (cause instanceof ConnectException) {
+ if ("Connection timed out".equals(cause.getMessage())) {
+ final IOException ex = createConnectTimeoutException(cause, namedEndpoint, remoteAddresses);
+ ex.initCause(cause);
+ return ex;
+ } else {
+ final IOException ex = createHttpHostConnectException(cause, namedEndpoint, remoteAddresses);
+ ex.setStackTrace(cause.getStackTrace());
+ return ex;
+ }
+ } else {
+ return cause;
+ }
+ }
+
+}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ConnectTimeoutException.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ConnectTimeoutException.java
index 9b74377..efb0f0a 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/ConnectTimeoutException.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ConnectTimeoutException.java
@@ -27,34 +27,22 @@
package org.apache.hc.client5.http;
-import java.io.IOException;
-import java.io.InterruptedIOException;
-import java.net.InetAddress;
-import java.util.Arrays;
+import java.net.SocketTimeoutException;
import org.apache.hc.core5.net.NamedEndpoint;
/**
- * A timeout while connecting to an HTTP server or waiting for an
- * available connection from an HttpConnectionManager.
+ * A timeout while connecting to an HTTP server or waiting for an available connection from a connection manager.
*
* @since 4.0
*/
-public class ConnectTimeoutException extends InterruptedIOException {
+public class ConnectTimeoutException extends SocketTimeoutException {
private static final long serialVersionUID = -4816682903149535989L;
private final NamedEndpoint namedEndpoint;
/**
- * Creates a ConnectTimeoutException with a {@code null} detail message.
- */
- public ConnectTimeoutException() {
- super();
- this.namedEndpoint = null;
- }
-
- /**
* Creates a ConnectTimeoutException with the specified detail message.
*/
public ConnectTimeoutException(final String message) {
@@ -62,23 +50,9 @@ public class ConnectTimeoutException extends InterruptedIOException {
this.namedEndpoint = null;
}
- /**
- * Creates a ConnectTimeoutException based on original {@link IOException}.
- *
- * @since 4.3
- */
- public ConnectTimeoutException(
- final IOException cause,
- final NamedEndpoint namedEndpoint,
- final InetAddress... remoteAddresses) {
- super("Connect to " +
- (namedEndpoint != null ? namedEndpoint : "remote endpoint") +
- (remoteAddresses != null && remoteAddresses.length > 0 ?
- " " + Arrays.asList(remoteAddresses) : "") +
- ((cause != null && cause.getMessage() != null) ?
- " failed: " + cause.getMessage() : " timed out"));
+ public ConnectTimeoutException(final String message, final NamedEndpoint namedEndpoint) {
+ super(message);
this.namedEndpoint = namedEndpoint;
- initCause(cause);
}
/**
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/HttpHostConnectException.java b/httpclient5/src/main/java/org/apache/hc/client5/http/HttpHostConnectException.java
index b261266..8ec2be2 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/HttpHostConnectException.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/HttpHostConnectException.java
@@ -26,16 +26,12 @@
*/
package org.apache.hc.client5.http;
-import java.io.IOException;
import java.net.ConnectException;
-import java.net.InetAddress;
-import java.util.Arrays;
import org.apache.hc.core5.net.NamedEndpoint;
/**
- * A {@link ConnectException} that specifies the {@link NamedEndpoint} that was
- * being connected to.
+ * A {@link ConnectException} that specifies the {@link NamedEndpoint} that was being connected to.
*
* @since 4.0
*/
@@ -46,22 +42,16 @@ public class HttpHostConnectException extends ConnectException {
private final NamedEndpoint namedEndpoint;
/**
- * Creates a HttpHostConnectException based on original {@link java.io.IOException}.
- *
- * @since 5.0
+ * Creates a HttpHostConnectException with the specified detail message.
*/
- public HttpHostConnectException(
- final IOException cause,
- final NamedEndpoint namedEndpoint,
- final InetAddress... remoteAddresses) {
- super("Connect to " +
- (namedEndpoint != null ? namedEndpoint : "remote endpoint") +
- (remoteAddresses != null && remoteAddresses .length > 0 ?
- " " + Arrays.asList(remoteAddresses) : "") +
- ((cause != null && cause.getMessage() != null) ?
- " failed: " + cause.getMessage() : " refused"));
+ public HttpHostConnectException(final String message) {
+ super(message);
+ this.namedEndpoint = null;
+ }
+
+ public HttpHostConnectException(final String message, final NamedEndpoint namedEndpoint) {
+ super(message);
this.namedEndpoint = namedEndpoint;
- initCause(cause);
}
/**
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java
index 7313ebc..2f0b5f2 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java
@@ -27,16 +27,12 @@
package org.apache.hc.client5.http.impl.io;
import java.io.IOException;
-import java.net.ConnectException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
-import java.net.NoRouteToHostException;
import java.net.Socket;
-import java.net.SocketTimeoutException;
-import org.apache.hc.client5.http.ConnectTimeoutException;
+import org.apache.hc.client5.http.ConnectExceptionSupport;
import org.apache.hc.client5.http.DnsResolver;
-import org.apache.hc.client5.http.HttpHostConnectException;
import org.apache.hc.client5.http.SchemePortResolver;
import org.apache.hc.client5.http.SystemDefaultDnsResolver;
import org.apache.hc.client5.http.UnsupportedSchemeException;
@@ -155,21 +151,9 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection
this.log.debug(ConnPoolSupport.getId(conn) + ": connection established " + conn);
}
return;
- } catch (final SocketTimeoutException ex) {
+ } catch (final IOException ex) {
if (last) {
- throw new ConnectTimeoutException(ex, host, addresses);
- }
- } catch (final ConnectException ex) {
- if (last) {
- final String msg = ex.getMessage();
- if ("Connection timed out".equals(msg)) {
- throw new ConnectTimeoutException(ex, host, addresses);
- }
- throw new HttpHostConnectException(ex, host, addresses);
- }
- } catch (final NoRouteToHostException ex) {
- if (last) {
- throw ex;
+ throw ConnectExceptionSupport.enhance(ex, host, addresses);
}
}
if (this.log.isDebugEnabled()) {
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/MultihomeIOSessionRequester.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/MultihomeIOSessionRequester.java
index 9eb5186..db047b4 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/MultihomeIOSessionRequester.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/MultihomeIOSessionRequester.java
@@ -36,8 +36,8 @@ import java.util.Arrays;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hc.client5.http.ConnectExceptionSupport;
import org.apache.hc.client5.http.DnsResolver;
-import org.apache.hc.client5.http.HttpHostConnectException;
import org.apache.hc.client5.http.SystemDefaultDnsResolver;
import org.apache.hc.core5.concurrent.ComplexFuture;
import org.apache.hc.core5.concurrent.FutureCallback;
@@ -129,7 +129,7 @@ final class MultihomeIOSessionRequester {
"(" + cause.getClass() + "); terminating operation");
}
if (cause instanceof IOException) {
- future.failed(new HttpHostConnectException((IOException) cause, remoteEndpoint, remoteAddresses));
+ future.failed(ConnectExceptionSupport.enhance((IOException) cause, remoteEndpoint, remoteAddresses));
} else {
future.failed(cause);
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/TestExceptions.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ConnectExceptionSupportTest.java
similarity index 75%
rename from httpclient5/src/test/java/org/apache/hc/client5/http/TestExceptions.java
rename to httpclient5/src/test/java/org/apache/hc/client5/http/ConnectExceptionSupportTest.java
index bd25fe5..8842123 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/TestExceptions.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/ConnectExceptionSupportTest.java
@@ -38,30 +38,18 @@ import org.junit.Test;
* Unit tests for exceptions.
* Trivial, but it looks better in the Clover reports.
*/
-public class TestExceptions {
+public class ConnectExceptionSupportTest {
@Test
- public void testConnectTimeoutExceptionNullMessage() {
- final ConnectTimeoutException ctx = new ConnectTimeoutException();
- Assert.assertNull(ctx.getMessage());
- }
-
- @Test
- 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);
+ public void testConnectTimeoutExceptionFromNullMessageAndHost() {
+ final ConnectTimeoutException ctx = ConnectExceptionSupport.createConnectTimeoutException(null, null);
Assert.assertEquals("Connect to remote endpoint timed out", ctx.getMessage());
}
@Test
public void testConnectTimeoutExceptionFromCause() {
final IOException cause = new IOException("something awful");
- final ConnectTimeoutException ctx = new ConnectTimeoutException(cause, null);
+ final ConnectTimeoutException ctx = ConnectExceptionSupport.createConnectTimeoutException(cause, null);
Assert.assertEquals("Connect to remote endpoint failed: something awful", ctx.getMessage());
}
@@ -69,7 +57,7 @@ public class TestExceptions {
public void testConnectTimeoutExceptionFromCauseAndHost() {
final HttpHost target = new HttpHost("localhost");
final IOException cause = new IOException();
- final ConnectTimeoutException ctx = new ConnectTimeoutException(cause, target);
+ final ConnectTimeoutException ctx = ConnectExceptionSupport.createConnectTimeoutException(cause, target);
Assert.assertEquals("Connect to http://localhost timed out", ctx.getMessage());
}
@@ -78,13 +66,13 @@ public class TestExceptions {
final HttpHost target = new HttpHost("localhost");
final InetAddress remoteAddress = InetAddress.getByAddress(new byte[] {1,2,3,4});
final IOException cause = new IOException();
- final ConnectTimeoutException ctx = new ConnectTimeoutException(cause, target, remoteAddress);
+ final ConnectTimeoutException ctx = ConnectExceptionSupport.createConnectTimeoutException(cause, target, remoteAddress);
Assert.assertEquals("Connect to http://localhost [/1.2.3.4] timed out", ctx.getMessage());
}
@Test
public void testHttpHostConnectExceptionFromNullCause() {
- final HttpHostConnectException ctx = new HttpHostConnectException(null, null,
+ final HttpHostConnectException ctx = ConnectExceptionSupport.createHttpHostConnectException(null, null,
(InetAddress [])null);
Assert.assertEquals("Connect to remote endpoint refused", ctx.getMessage());
}
@@ -92,7 +80,7 @@ public class TestExceptions {
@Test
public void testHttpHostConnectExceptionFromCause() {
final IOException cause = new IOException("something awful");
- final HttpHostConnectException ctx = new HttpHostConnectException(cause, null);
+ final HttpHostConnectException ctx = ConnectExceptionSupport.createHttpHostConnectException(cause, null);
Assert.assertEquals("Connect to remote endpoint failed: something awful", ctx.getMessage());
}
@@ -100,7 +88,7 @@ public class TestExceptions {
public void testHttpHostConnectExceptionFromCauseAndHost() {
final HttpHost target = new HttpHost("localhost");
final IOException cause = new IOException();
- final HttpHostConnectException ctx = new HttpHostConnectException(cause, target);
+ final HttpHostConnectException ctx = ConnectExceptionSupport.createHttpHostConnectException(cause, target);
Assert.assertEquals("Connect to http://localhost refused", ctx.getMessage());
}
@@ -110,7 +98,7 @@ public class TestExceptions {
final InetAddress remoteAddress1 = InetAddress.getByAddress(new byte[] {1,2,3,4});
final InetAddress remoteAddress2 = InetAddress.getByAddress(new byte[] {5,6,7,8});
final IOException cause = new IOException();
- final HttpHostConnectException ctx = new HttpHostConnectException(cause, target,
+ final HttpHostConnectException ctx = ConnectExceptionSupport.createHttpHostConnectException(cause, target,
remoteAddress1, remoteAddress2);
Assert.assertEquals("Connect to http://localhost [/1.2.3.4, /5.6.7.8] refused", ctx.getMessage());
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientExecuteSOCKS.java b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientExecuteSOCKS.java
index 9c1abf8..29f04a6 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientExecuteSOCKS.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientExecuteSOCKS.java
@@ -31,9 +31,7 @@ import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.net.Socket;
-import java.net.SocketTimeoutException;
-import org.apache.hc.client5.http.ConnectTimeoutException;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
@@ -106,11 +104,7 @@ public class ClientExecuteSOCKS {
if (localAddress != null) {
sock.bind(localAddress);
}
- try {
- sock.connect(remoteAddress, connectTimeout != null ? connectTimeout.toMillisIntBound() : 0);
- } catch (final SocketTimeoutException ex) {
- throw new ConnectTimeoutException(ex, host, remoteAddress.getAddress());
- }
+ sock.connect(remoteAddress, connectTimeout != null ? connectTimeout.toMillisIntBound() : 0);
return sock;
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryHandler.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryHandler.java
index 3a77524..faf13af 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryHandler.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryHandler.java
@@ -27,9 +27,9 @@
package org.apache.hc.client5.http.impl;
import java.io.IOException;
+import java.net.SocketTimeoutException;
import java.net.UnknownHostException;
-import org.apache.hc.client5.http.ConnectTimeoutException;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.junit.Assert;
import org.junit.Test;
@@ -45,7 +45,7 @@ public class TestDefaultHttpRequestRetryHandler {
final DefaultHttpRequestRetryHandler retryHandler = new DefaultHttpRequestRetryHandler();
Assert.assertEquals(3, retryHandler.getRetryCount());
- Assert.assertFalse(retryHandler.retryRequest(request, new ConnectTimeoutException(), 1, null));
+ Assert.assertFalse(retryHandler.retryRequest(request, new SocketTimeoutException(), 1, null));
}
@Test
@@ -82,7 +82,7 @@ public class TestDefaultHttpRequestRetryHandler {
final DefaultHttpRequestRetryHandler retryHandler = new DefaultHttpRequestRetryHandler();
- Assert.assertFalse(retryHandler.retryRequest(request, new ConnectTimeoutException(), 3, null));
+ Assert.assertFalse(retryHandler.retryRequest(request, new SocketTimeoutException(), 3, null));
}
}