On Fri, 7 Nov 2025 15:00:39 GMT, Jaikiran Pai <[email protected]> wrote:
>> Daniel Fuchs has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - More copyright dates
>> - small test cleanup
>> - Fix copyright dates
>
> 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.
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28196#discussion_r2504048630
PR Review Comment: https://git.openjdk.org/jdk/pull/28196#discussion_r2504055194
