Hi,

This is a code review to add connection specific timeout support to the
new HTTP Client, as discussed recently over on another thread [1].

The connection timeout duration is proposed to be added at the level of
the client, and if specified applies to all requests sent by that
client. The connect timeout mechanism is orthogonal to the HttpRequest
`timeout`, but can be used to complement it. For example,

    HttpClient client = HttpClient.newBuilder()
         .connectTimeout(Duration.ofSeconds(20))
         .build();
    HttpRequest request = HttpRequest.newBuilder()
         .uri(URI.create("https://foo.com/";))
         .timeout(Duration.ofMinutes(2))
         .header("Content-Type", "application/json")
         .POST(BodyPublishers.ofFile(Paths.get("file.json")))
         .build();
    client.sendAsync(request, BodyHandlers.ofString())

  sends the request with a 20 second connect timeout and a response
timeout of 2 minutes.

The connect timeout, as implemented in the webrev below, covers the
DNS lookup, the TCP SYN handshake, and the TLS handshake. The proposed
spec wording deliberately avoids adding such low-level details.

Outline of the specification changes:

To HttpClient.Builder add:

  /**
   * Sets the connect timeout duration for this client.
   *
   * <p> In the case where a new connection needs to be established, if
   * the connection cannot be established within the given {@code
   * duration}, then an {@link HttpConnectTimeoutException} is thrown
   * from {@link HttpClient#send(java.net.http.HttpRequest,
   * java.net.http.HttpResponse.BodyHandler) HttpClient::send}, or
   * {@link HttpClient#sendAsync(java.net.http.HttpRequest,
   * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync}
   * completes exceptionally with an {@code HttpConnectTimeoutException}.
   * If a new connection does not need to be established, for example
   * if a connection can be reused from a previous request, then this
   * timeout duration has no effect.
   *
   * @param duration the duration to allow the underlying connection to be
   *                 established
   * @return this builder
   * @throws IllegalArgumentException if the duration is non-positive
   */
  public Builder connectTimeout(Duration duration);

To HttpClient add an accessor:

  /**
   * Returns an {@code Optional} containing the <i>connect timeout duration</i>
   * for this client. If the {@linkplain Builder#connectTimeout(Duration)
   * connect timeout duration} was not set in the client's builder, then the
   * {@code Optional} is empty.
   *
   * @return an {@code Optional} containing this client's connect timeout
   *         duration
   */
   public abstract Optional<Duration> connectTimeout();

Add new Exception type:

  /**
   * Thrown when a connection, over which an {@code HttpRequest} is intended to 
be
   * sent, is not successfully established within a specified time period.
   */
   public class HttpConnectTimeoutException extends HttpTimeoutException {
     ...
   }

Webrev:
  http://cr.openjdk.java.net/~chegar/8208391/webrev.00/

-Chris

[1] http://mail.openjdk.java.net/pipermail/net-dev/2018-August/011683.html

Reply via email to