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 error as the cause of the stream failure and propagate that as an `IOException`. I decided to check for `SocketException` and `EOFException` so as to avoid blindly considering any arbitrary stream failures as caused by the error reported in the `GOAWAY` frame. I've updated an existing test to verify this new implementation. This and other existing tests continue to pass with this change (both in a tier testing run as well as several hundreds of test-repeats). Note that this is specific to HTTP/2 and although HTTP/3 has a `GOAWAY` frame of its own, that frame doesn't have any error codes. For HTTP/3, through QUIC, the connection termination reason is passed to the peer in a `CONNECTION_CLOSE` frame itself, so no special tracking of error codes is needed in the `HttpClient`'s HTTP/3 code. --------- - [x] I confirm that I make this contribution in accordance with the [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). ------------- Commit messages: - 8371903: add test - 8371903: HttpClient: improve handling of HTTP/2 GOAWAY frames with error code Changes: https://git.openjdk.org/jdk/pull/32278/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=32278&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8371903 Stats: 225 lines in 3 files changed: 190 ins; 9 del; 26 mod Patch: https://git.openjdk.org/jdk/pull/32278.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/32278/head:pull/32278 PR: https://git.openjdk.org/jdk/pull/32278
