RFR: 8253005: Add `@throws IOException` in javadoc for `HttpEchange.sendResponseHeaders` [v7]

[Chris Hegarty]

On Thu, 5 Nov 2020 17:08:10 GMT, Patrick Concannon <pconcannon at openjdk.org> wrote:

>> Hi,
>> 
>> Could someone please review my fix for JDK-8253005: 'Add `@throws IOException` in javadoc for `HttpEchange.sendResponseHeaders`' ?
>> 
>> The method `HttpEchange.sendResponseHeaders` throws an `IOException` but is unspecified in its javadoc. This fix adds an `@throws IOException` to its specification and a description of the conditions under which the exception is thrown.
>> 
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8252304: Added read to TestHandler to ensure requestBody consumed before closing exchange

Changes requested by chegar (Reviewer).

test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 63:

> 61:         ExecutorService executor = Executors.newCachedThreadPool();
> 62:         server.setExecutor (executor);
> 63:         server.start();

Being a little pedantic, then the setup and teardown of the server could be moved into an `@BeforeTest` and `@AfterTest`, which would leave the client part of the test uncluttered.

test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 69:

> 67:                 server.getAddress().getPort(),
> 68:                 path, null, null);
> 69: 

This maybe fine, but the pattern we've used elsewhere ( and have proven to be reliable ) is: InetAddress.getLoopbackAddress().getHostName() and server.getAddress().getPort(), but now I think that maybe these could be an issue for IPv6-only hosts?

test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 75:

> 73:                     .GET()
> 74:                     .build();
> 75:             HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString());

How about we import java.net.http.HttpResponse.BodyHandlers?

test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 99:

> 97:                     // unexpected exception thrown, return error to client
> 98:                     t.printStackTrace();
> 99:                     os.write(("Unexpected error: " + t).getBytes());

Hmm... I'd be tempted to drop this catch block altogether, it's not possible to know exactly what to do there? given that we don't know what the failure is.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1014