On Mon, 17 Nov 2025 15:40:30 GMT, Daniel Fuchs <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fix typo in test code comment
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java
> line 2022:
>
>> 2020: if (debug.on()) {
>> 2021: debug.log("sending GOAWAY " + goAway);
>> 2022: }
>
> Suggestion:
>
> if (Log.trace()) {
> Log.logTrace("{0} sending GOAWAY {1}", connection, goAway);
> } else if (debug.on()) {
> debug.log("sending GOAWAY " + goAway);
> }
Done, updated the PR with this change.
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java
> line 2083:
>
>> 2081: ", GOAWAY sent=" + goAwaySent.get();
>> 2082: debug.log("Closing connection (" + stateStr + ") due
>> to: " + tc);
>> 2083: }
>
> The message logged for debug is sufficiently different from the one logged
> with Log that we should consider actually logging both without fearing the
> repetition.
> Either that, or consider logging the same message in both cases, in which
> case the else could stay.
>
> Suggestion:
>
> if (Log.errors()) {
> Log.logError("Closing connection due to: {0}", tc);
> }
> if (debug.on()) {
> final String stateStr = "Abnormal close=" +
> tc.isAbnormalClose() +
> ", has active streams=" + isActive() +
> ", GOAWAY received=" + goAwayRecvd.get() +
> ", GOAWAY sent=" + goAwaySent.get();
> debug.log("Closing connection (" + stateStr + ") due to: " +
> tc);
> }
I decided to use the same level of information in both these logs. While at it,
it felt odd to use `Log.logError()` to log a connection being closed, but it
looks like we don't have a more specific/appropriate log method for this case.
So I let it stay `Log.logError()` for now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2541345597
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2541351458
