RFR: 8310645: CancelledResponse.java does not use HTTP/2 when testing the HttpClient

[Conor Cleary]

On Mon, 3 Jul 2023 09:39:13 GMT, Conor Cleary <ccleary at openjdk.org> wrote:

>> test/jdk/java/net/httpclient/CancelledResponse2.java line 107:
>> 
>>> 105:             } catch (Exception e) {
>>> 106:                 e.printStackTrace();
>>> 107:                 assertTrue(e.getCause() instanceof IOException, "HTTP/2 should cancel with an IOException when the Subscription is cancelled.");
>> 
>> I believe we want to check that the exception we get as the cause is the expected CancelException, or at least that it can be found somewhere while following the cause chain. Otherwise looks good!
>
> So for HTTP/2, the `cancelImpl()` method in `Stream` looks like this. 
> 
> 
>     @Override
>     void cancel() {
>         if ((streamid == 0)) {
>             cancel(new IOException("Stream cancelled before streamid assigned"));
>         } else {
>             cancel(new IOException("Stream " + streamid + " cancelled"));
>         }
>     }
> 
> 
> When the new IOException is passed to cancel, it sets the `errorRef` field to the IOException which then gets returned to us as the cause of the CompletableFuture's cause for cancelling. I think this behaviour is acceptable as the IOException returned will say "Stream x cancelled" which is accurate.  Possible that I dont need to have this "CancelException" here
> 
> May be more difficult to verify the cause of cancellation beyond that which I'll think about.

There is a check that the cancelling BodyHandler sets the cancelled field which may be sufficient

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14625#discussion_r1250574856