RFR: 8371471: HttpClient: Log HTTP/3 handshake failures if logging errors is enabled [v2]

[Daniel Fuchs]

On Fri, 7 Nov 2025 15:07:13 GMT, Daniel Fuchs <dfuchs at openjdk.org> 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