HttpClient API expectations for connection failures

[Chris Hegarty]

Jaikiran,

Apologies for the delay, I missed this email in my inbox.

> On 12 Jun 2018, at 01:47, Jaikiran Pai <jai.forums2013 at gmail.com> wrote:
> 
> Any thoughts?
> 

I filed the following JIRA to track this issue:
  https://bugs.openjdk.java.net/browse/JDK-8204864

I think that the most appropriate course of action is to consistently throw
the IOException subtype, ConnectionException ( with the original cause ),
for any failures encountered during a connect operation. Rather than trying
to deal with the low-level NIO exceptions.

I’ll resolve this issue in the sandbox, and it will likely make the next refresh
[1], along with all other comments and feedback that we’ve received to date.

-Chris.

[1] https://bugs.openjdk.java.net/browse/JDK-8204863



> -Jaikiran
> 
> On Sunday, June 3, 2018, Jaikiran Pai <jai.forums2013 at gmail.com> wrote:
> > Consider the following code:
> >
> > public class HttpClientTest {
> >     public static void main(final String[] args) {
> >         final HttpClient httpClient = HttpClient.newBuilder().build();
> >         final String targetURL = "http://unknown.host.foo.bar.com.nowhere";
> >         final HttpRequest request = HttpRequest.newBuilder().uri(URI.create(targetURL))
> >                                 .method("HEAD", HttpRequest.BodyPublishers.noBody())
> >                                 .build();
> >         try {
> >             httpClient.send(request, HttpResponse.BodyHandlers.discarding());
> >         } catch(IOException | InterruptedException e) {
> >             // intentionally ignore
> >             System.out.println("Caught an intentionally ignored exception");
> >         } catch (Exception e) {
> >             e.printStackTrace();
> >         }
> >     }
> > }
> >
> > which tries to send a request to (in this case an intentional) non-existent address. The exception that gets thrown is:
> >
> >
> > java.nio.channels.UnresolvedAddressException
> >     at java.base/sun.nio.ch.Net.checkAddress(Net.java:130)
> >     at java.base/sun.nio.ch.SocketChannelImpl.connect(SocketChannelImpl.java:674)
> >     at java.net.http/jdk.internal.net.http.PlainHttpConnection.lambda$connectAsync$0(PlainHttpConnection.java:113)
> >     at java.base/java.security.AccessController.doPrivileged(Native Method)
> >     at java.net.http/jdk.internal.net.http.PlainHttpConnection.connectAsync(PlainHttpConnection.java:115)
> >     at java.net.http/jdk.internal.net.http.Http1Exchange.sendHeadersAsync(Http1Exchange.java:261)
> >     at java.net.http/jdk.internal.net.http.Exchange.lambda$responseAsyncImpl0$8(Exchange.java:385)
> >     at java.net.http/jdk.internal.net.http.Exchange.checkFor407(Exchange.java:320)
> >     at java.net.http/jdk.internal.net.http.Exchange.lambda$responseAsyncImpl0$9(Exchange.java:389)
> >     at java.base/java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:930)
> >     at java.base/java.util.concurrent.CompletableFuture.uniHandleStage(CompletableFuture.java:946)
> >     at java.base/java.util.concurrent.CompletableFuture.handle(CompletableFuture.java:2266)
> >     at java.net.http/jdk.internal.net.http.Exchange.responseAsyncImpl0(Exchange.java:389)
> >     at java.net.http/jdk.internal.net.http.Exchange.responseAsyncImpl(Exchange.java:299)
> >     at java.net.http/jdk.internal.net.http.Exchange.responseAsync(Exchange.java:291)
> >     at java.net.http/jdk.internal.net.http.MultiExchange.responseAsyncImpl(MultiExchange.java:240)
> >     at java.net.http/jdk.internal.net.http.MultiExchange.lambda$responseAsync0$1(MultiExchange.java:205)
> >     at java.base/java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1072)
> >     at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:506)
> >     at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1705)
> >     at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
> >     at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
> >     at java.base/java.lang.Thread.run(Thread.java:832)
> >
> >
> > It's a different matter that this stacktrace is missing the caller's stack frames and that's being tracked in [1]. The javadoc of send method doesn't mention anything about what specific exception gets thrown in case of connection failures. It does say:
> >
> >      * @throws IllegalArgumentException if the {@code request} argument is not
> >      *         a request that could have been validly built as specified by {@link
> >      *         HttpRequest.Builder HttpRequest.Builder}.
> >
> > and the thrown java.nio.channels.UnresolvedAddressException indeed extends IllegalArgumentException. However, given that the API itself doesn't say anything about exceptions that get thrown on connection failures, adding a catch block specifically for java.nio.channels.UnresolvedAddressException seems like relying on the implementation detail. Can these APIs make it explicit what exception(s) can be expected to be thrown in case of connection failures, so that it's part of the contract?
> >
> > [1] https://bugs.openjdk.java.net/browse/JDK-8203298
> >
> > -Jaikiran
> >
> >