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/db3cbfac...9baad557 src/java.net.http/share/classes/jdk/internal/net/http/Http2TerminationCause.java line 234: > 232: // From the given exception's chain of causes, this method finds and > returns an exception > 233: // whose class type matches any of the given candidate types. > Returns null if none found. > 234: private static Throwable findInCause(final Throwable exception, Would it be worth adding this method to Utils and making sure it's use at the other places where we look for a cause? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32278#discussion_r3765857386
