RFR: 8371903: HttpClient: improve handling of HTTP/2 GOAWAY frames with error code

[Daniel Jeliński]

On Wed, 3 Dec 2025 12:08:33 GMT, EunHyunsu <duke at openjdk.org> wrote:

> ### Problem
> 
>   When the HTTP/2 client receives a GOAWAY frame with a non-zero error code, the current implementation discards both the error code and debug data. Users only see generic "Connection closed by peer" errors without any information about why the server terminated the connection.
> 
>   ### Solution
> 
>   Per [RFC 9113 §5.4.1](https://www.rfc-editor.org/rfc/rfc9113.html#section-5.4.1), a GOAWAY frame with a non-zero error code indicates a connection error requiring immediate closure. This fix:
> 
>   1. **Distinguishes** graceful shutdown (NO_ERROR) from connection errors
>   2. **Preserves** error code and debug data in exception messages
>   3. **Categorizes** streams based on `lastStreamId`:
>      - Streams with ID > `lastStreamId`: Marked as unprocessed for automatic retry
>      - Streams with ID ≤ `lastStreamId`: Failed with detailed error information
> 
>   ### Changes
> 
>   **Core Implementation** (`Http2Connection.java`):
>   - Modified `handleGoAway()` to check error code and route appropriately
>   - Added `handleGoAwayWithError()` method that:
>     - Extracts error code and debug data from GOAWAY frame
>     - Creates meaningful error messages with error name, hex code, and debug data
>     - Properly categorizes streams for retry or failure
> 
>   **Test Infrastructure**:
>   - `Http2TestServerConnection.sendGoAway(int, int, byte[])`: Supports custom error codes
>   - `Http2TestExchangeImpl.getServerConnection()`: Accessor for test handlers
>   - `GoAwayWithErrorTest`: Verifies proper error propagation
> 
>   ### Example
> 
>   **Before:**
>   IOException: Connection closed by peer
> 
>   **After:**
>   IOException: Received GOAWAY with error code Protocol error (0x1): Invalid HEADERS frame
> 
>   ### Testing
> 
>   - New `GoAwayWithErrorTest` passes
>   - Existing HTTP/2 tests unaffected (NO_ERROR path unchanged)
>   - Backward compatible (no public API changes)

src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 1481:

> 1479:                 // Stream was being processed - fail it with the connection error
> 1480:                 final IOException error = new IOException(errorMsg);
> 1481:                 stream.connectionClosing(error);

Suggestion:

                stream.connectionClosing(cause.getCloseCause());

test/jdk/java/net/httpclient/http2/GoAwayWithErrorTest.java line 173:

> 171:                 System.out.println("Handler: sending GOAWAY(PROTOCOL_ERROR) and closing connection");
> 172: 
> 173:                 impl.getServerConnection().sendGoAway(Integer.MAX_VALUE, PROTOCOL_ERROR, DEBUG_DATA);

Please add a test to verify that the requests above lastStreamId are retried

test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http2/Http2TestServerConnection.java line 283:

> 281:      * @throws IOException if an I/O error occurs
> 282:      */
> 283:     public void sendGoAway(int lastStreamId, int errorCode, byte[] debugData) throws IOException {

please merge this method with the other sendGoAway (make the other one call this one?)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28632#discussion_r2585342808
PR Review Comment: https://git.openjdk.org/jdk/pull/28632#discussion_r2585689936
PR Review Comment: https://git.openjdk.org/jdk/pull/28632#discussion_r2585370317