On Fri, 7 Nov 2025 15:07:13 GMT, Daniel Fuchs <[email protected]> wrote:
>> src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicConnectionImpl.java
>> line 596:
>>
>>> 594: if (Log.errors()) {
>>> 595: Log.logError("%s QUIC handshake failed: %s"
>>> 596: .formatted(logTag(), message(cause)));
>>
>> I think we don't need the new `message(...)` method here. Perhaps just doing:
>>
>> Log.logError("%s QUIC handshake failed: %s".formatted(logTag(), cause));
>>
>> is enough? For example, in jshell I see the following:
>>
>>
>> jshell> Throwable x = new Throwable()
>> jshell> Throwable y = new Throwable("foo bar")
>>
>> jshell> "%s QUIC handshake failed: %s".formatted("tag", x)
>> $5 ==> "tag QUIC handshake failed: java.lang.Throwable"
>>
>> jshell> "%s QUIC handshake failed: %s".formatted("tag", y)
>> $6 ==> "tag QUIC handshake failed: java.lang.Throwable: foo bar"
>
> Right - the reason I did that originally is that I thought we might get a
> non-exported exception here. But I see that the terminator already translate
> the QuicTransportException before completing the handshake.
Done
>> test/jdk/java/net/httpclient/http3/H3LogHandshakeErrors.java line 126:
>>
>>> 124: final HttpRequest.Builder reqBuilder =
>>> HttpRequest.newBuilder(reqURI)
>>> 125: .version(HTTP_3);
>>> 126: Logger serverLogger =
>>> Logger.getLogger("jdk.httpclient.HttpClient");
>>
>> Should we add a reachability fence for this `serverLogger`?
>
> Good point. When I started coding I thought it would be used in the finally
> block but in the end I forgot about it.
I stuck it in a static field and renamed it to clientLogger.
>> test/jdk/java/net/httpclient/http3/H3LogHandshakeErrors.java line 132:
>>
>>> 130: @Override
>>> 131: public void publish(LogRecord record) {
>>> 132: record.getSourceClassName();
>>
>> Is this line a leftover? Or does it intentionally call this method and not
>> use the return value?
>
> It does intentionally call it to force the LogRecord to infer the caller at
> the right place. Inferring the caller has to happen in the handler to ensure
> the right caller will be found.
added a comment
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28196#discussion_r2504141697
PR Review Comment: https://git.openjdk.org/jdk/pull/28196#discussion_r2504137443
PR Review Comment: https://git.openjdk.org/jdk/pull/28196#discussion_r2504140582
