Re: HttpClient API usage feedback/questions

2018-06-02 Thread Pavel Rappo


> On 2 Jun 2018, at 07:22, Jaikiran Pai  wrote:
> 
> Some of the places where we parse the (response) headers, we check for the 
> presence of some headers and if not present then we default them to certain 
> values (based on certain application specific context). In its current form 
> of the HttpHeaders API, we would either have to write some kind of wrapper 
> API which returns a default value for those headers or keep repeating the 
> logic of checking the returned Optional for empty value, before defaulting it 
> to something.

Probably asking the obvious, but why can't you just write:

headers.firstValue(name).orElse(defaultValue)

instead of

headers.firstValueOrDefault(name, defaultValue) ?

I'm not sure the value provided by introducing these methods is worth the
increase in this API's size. Optionals provide a toolset for such tasks. 
This toolset is much richer than just "defaulting".



Re: HttpClient API usage feedback/questions

2018-06-02 Thread Jaikiran Pai
You are right. That didn't occur to me. So yes, clearly, adding those 2 new
methods won't add value.

-Jaikiran

On Sunday, June 3, 2018, Pavel Rappo  wrote:
>
>> On 2 Jun 2018, at 07:22, Jaikiran Pai  wrote:
>>
>> Some of the places where we parse the (response) headers, we check for
the presence of some headers and if not present then we default them to
certain values (based on certain application specific context). In its
current form of the HttpHeaders API, we would either have to write some
kind of wrapper API which returns a default value for those headers or keep
repeating the logic of checking the returned Optional for empty value,
before defaulting it to something.
>
> Probably asking the obvious, but why can't you just write:
>
> headers.firstValue(name).orElse(defaultValue)
>
> instead of
>
> headers.firstValueOrDefault(name, defaultValue) ?
>
> I'm not sure the value provided by introducing these methods is worth the
> increase in this API's size. Optionals provide a toolset for such tasks.
> This toolset is much richer than just "defaulting".
>
>


HttpClient API expectations for connection failures

2018-06-02 Thread Jaikiran Pai

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