On Wed, 10 Dec 2025 23:34:02 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) > > EunHyunsu has updated the pull request incrementally with one additional > commit since the last revision: > > 8371903: Use Utils.adjustTimeout() in GoAwayWithErrorTest src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 1473: > 1471: numUnprocessed.incrementAndGet(); > 1472: } else { > 1473: stream.connectionClosing(cause.getCloseCause()); Hello @ehs208, I think this should be done a bit differently. Here we are merely constructing a `TerminationCause` and then closing the streams with that termination cause. But that doesn't guarantee that the connection will have been closed with this termination cause. I think what we should do here is merely mark the unprocessed streams with closeAsUnprocessed() and then just call `close(Http2TerminationCause.forH2Error(errorCode, errorMsg);`. The implementation of the `Http2Connection.Terminator.doTerminate()` already has the necessary logic to close all these (processed) streams with the termination cause. So I think this code should look something like: final byte[] debugData = frame.getDebugData(); final String debugInfo = debugData.length > 0 ? new String(debugData, UTF_8) : ""; ... streams.forEach((id, stream) -> { if (id > lastProcessedStream) { stream.closeAsUnprocessed(); numUnprocessed.incrementAndGet(); } }); ... // closes the connection as well as the rest of the (processed) streams close(Http2TerminationCause.forH2Error(errorCode, errorMsg)); I haven't tested this myself. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28632#discussion_r2610351685
