On Wed, 12 Aug 2026 08:00:07 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this change which proposes to implement the 
>> enhancement requested in https://bugs.openjdk.org/browse/JDK-8371903?
>> 
>> HTTP/2 protocol allows endpoints to send a `GOAWAY` frame 
>> https://www.rfc-editor.org/rfc/rfc9113.html#name-goaway. The `GOAWAY` frame 
>> contains an error code which can be `0` (implying no error) or some other 
>> code (implying some error). The `GOAWAY` frame is a signal to the peer that 
>> the connection will (soon) be closed by the endpoint and no new requests 
>> should be issued on it.
>> 
>> When a server sends a `GOAWAY`, the `HttpClient` implementation in the JDK 
>> when processing that frame will (rightly) mark the connection as unusable 
>> for new requests and if there are no active streams on that connection will 
>> (rightly) close the connection. The HttpClient implementation will then 
>> create new connection as and when needed for any subsequent requests.
>> 
>> In the case where the `HttpClient` receives the `GOAWAY` when there are 
>> current active streams, then the implementation marks the connection as 
>> unusable for new requests and just moves along. Depending on why the 
>> `GOAWAY` was issued by the server, the server may either let the requests 
>> complete normally or may close the connection before the requests complete. 
>> This can then lead to the `HttpClient` rightly failing such requests. Such 
>> failures get propagated to the application code as (subtypes) of 
>> `IOException`. All this is expected and complies with the API specification 
>> of `HttpClient` as well as the HTTP/2 protocol.
>> 
>> One detail of the `GOAWAY` frame is that the error code in that frame is 
>> specified by the HTTP/2 protocol. These error codes have specific meaning 
>> and sometimes can be a useful detail to include when a request is being 
>> failed due to the connection being closed by the server. In the current 
>> implementation of the `HttpClient`, this detail doesn't show up in the 
>> stacktrace or exception message of the `IOException` that reaches the 
>> application.
>> 
>> The changes in this PR enhance the implementation of `HttpClient` to keep 
>> track of the error code from a `GOAWAY` frame and if some stream fails due 
>> to a connection termination, then the error code from the `GOAWAY` is 
>> included in the termination cause's exception message that reaches the 
>> application. In the proposed implementation if a stream fails exceptionally, 
>> then we check the exception type for `SocketException` and `EOFException` 
>> and if it is either of these then we consider the `GOAWAY` frame's er...
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 12 additional 
> commits since the last revision:
> 
>  - merge latest from master branch
>  - 8390186: [Valhalla] LoadNode::Value should check for ary->is_not_flat()
>    
>    Reviewed-by: thartmann, chagedorn
>  - 8353624: C2: Re-enable malformed graph assert removed with JDK-8317998 to 
> reduce noise
>    
>    Reviewed-by: qamai, thartmann
>  - 8389671: (se) Blocking selection op in virtual thread does not keep spare 
> alive beyond scheduler keep alive time (win)
>    
>    Reviewed-by: jpai
>  - minor change to exception cause traversal
>  - merge latest from master branch
>  - fix major typo in test assertion
>  - merge latest from master branch
>  - add 8371903 to the test @bug ids
>  - read incomingGoAway just once
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/74495c08...9baad557

Marked as reviewed by vyazici (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/32278#pullrequestreview-4916100899

Reply via email to