On Sat, 13 Dec 2025 05:12:33 GMT, Christoph Läubrich <[email protected]> wrote:
>> EunHyunsu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8371903: Use Utils.adjustTimeout() in GoAwayWithErrorTest
>
> test/jdk/java/net/httpclient/http2/GoAwayWithErrorTest.java line 162:
>
>> 160: && goawayError.getMessage().contains("0x1")
>> 161: &&
>> goawayError.getMessage().contains(DEBUG_MESSAGE),
>> 162: "Failed request should contain GOAWAY error details: "
>> + goawayError.getMessage());
>
> I think this test assertion alone already shows that a client has only the
> chance to use String parsing what I think is bad (both for test and for
> production code).
>
> If its not planned to have an own public type, can it at least be an internal
> exception type that can be used in the test?
> Maybe a Http2ProtocolError?
@laeubi I appreciate your suggestion about semantic error handling. I looked
into how HTTP/3 handles similar cases for consistency.
HTTP/3 tests also use string parsing to verify error messages. For example, in
`test/jdk/java/net/httpclient/http3/H3ErrorHandlingTest.java:954-955`:
assertTrue(cause.getMessage().contains(reason),
cause.getMessage() + " does not contain " + reason);
This pattern is used throughout HTTP/3 tests (e.g., verifying "INTERNAL_ERROR",
"CRYPTO_ERROR", "H3_EXCESSIVE_LOAD" in error messages).
As @djelinski mentioned, this PR brings HTTP/2 error handling to parity with
HTTP/3's current approach. I agree that a specialized exception type would be
valuable, but it would ideally be done consistently across both HTTP/2 and
HTTP/3 as a future enhancement.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28632#discussion_r2616948240
