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)

@djelinski  
Thanks for the feedback! Made all the changes:

- Now using `cause.getCloseCause()` for consistency
- Merged sendGoAway methods as suggested
- Test now verifies unprocessed stream retries with lastStreamId=1

Let me know if anything else needs adjustment!

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

PR Comment: https://git.openjdk.org/jdk/pull/28632#issuecomment-3611468711

Reply via email to