> On 4 Aug 2018, at 14:08, Jaikiran Pai <jai.forums2...@gmail.com> wrote:
>
> ...
>
> Do you think this can be reworded a bit? Although I understand what's
> being said here, the wording doesn't seem right. Maybe something like:
>
> * <p> In the case where a new connection needs to be
> established, if
> * the connection cannot be established within the given {@code
> * duration}, then {@link HttpClient#send(java.net.http.HttpRequest,
> * java.net.http.HttpResponse.BodyHandler) HttpClient::send}
> throws a
> * {@link HttpConnectTimeoutException}, or
> * {@link HttpClient#sendAsync(java.net.http.HttpRequest,
> * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync}
> * completes exceptionally with an {@code
> HttpConnectTimeoutException}.
Agreed. This wording avoids the previous awkwardness.
> src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java
>
> - this.timeout = null;
> + this.timeout = Duration.ofSeconds(30);
>
> Is this an intentional change of default value for the timeout? Is that
> something that needs to be documented?
Accidental, left over from a previous hacking session. Removed.
Updated webrev:
http://cr.openjdk.java.net/~chegar/8208391/webrev.01/
> One other thing - maybe not directly related to this single patch, but
> as you are aware, recently as part of [1], a new system property (and a
> security property) was introduced to optionally include host info in the
> exception messages thrown for socket exceptions
I am aware of this.
> . Does the HttpClient
> honour that system property in the exceptions it throws?
The HTPT Client does not have any special handing for this property.
It may not be necessary, at least not for low-level NIO exceptions, once
the exception, or its cause, is preserved.
> I don't see it
> being used in this patch for the timeout exceptions. I haven't checked
> the code, outside of this patch, to see if it's dealt with in other
> parts of this API.
Separately, I will look into the possibility of where such an extension
can be used.
-Chris.
