On Wed, 3 Dec 2025 12:08:33 GMT, EunHyunsu <[email protected]> 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
