From 4cd45d7dd08689e36669170bd3a245ddffb6d1a7 Mon Sep 17 00:00:00 2001 From: David <3dgiordano@gmail.com> Date: Thu, 20 Aug 2026 15:50:51 -0300 Subject: [PATCH 1/2] Bring visibility to connection handshake --- .../http2/core/ConnectAttemptRecorder.java | 232 +++++++++++++++ .../jmeter/http2/core/HTTP2JettyClient.java | 53 +++- .../CustomHttpClientTransportOverHTTP2.java | 22 ++ .../jmeter/http2/sampler/HTTP2Sampler.java | 29 +- .../core/ConnectAttemptRecorderTest.java | 277 ++++++++++++++++++ .../core/ConnectAttemptVisibilityTest.java | 190 ++++++++++++ 6 files changed, 794 insertions(+), 9 deletions(-) create mode 100644 src/main/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorder.java create mode 100644 src/test/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorderTest.java create mode 100644 src/test/java/com/blazemeter/jmeter/http2/core/ConnectAttemptVisibilityTest.java diff --git a/src/main/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorder.java b/src/main/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorder.java new file mode 100644 index 00000000..a2c93064 --- /dev/null +++ b/src/main/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorder.java @@ -0,0 +1,232 @@ +package com.blazemeter.jmeter.http2.core; + +import java.net.ConnectException; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.net.SocketException; +import java.net.SocketTimeoutException; +import java.net.URL; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import javax.net.ssl.SSLException; +import org.eclipse.jetty.client.Connection; +import org.eclipse.jetty.util.Promise; + +/** + * Keeps the connection failures Jetty throws away, so a failed sample can report every address it + * actually tried instead of only the last one. + * + *

{@code HttpClient.connect(List, int, Map)} walks the resolved addresses recursively and, when + * one fails, calls itself for the next while discarding the exception - no + * {@code addSuppressed}, no log. Only the last address's failure survives. With the JDK default + * ({@code java.net.preferIPv6Addresses=false}) IPv4 is tried first, so a dual-stack host that + * fails on both reports the IPv6 error alone and the IPv4 one - usually the interesting one - is + * gone. Same code in 12.0.x and 12.1.x, so this is not a version regression. + * + *

The recovery point is {@code Connection.PROMISE_CONTEXT_KEY}: the promise Jetty puts in the + * context for one address, whose {@code failed} is exactly what decides to move on to the next. + * Every way of failing to establish a connection ends there - a refused socket, a TLS + * handshake, ALPN, and the HTTP/2 preface, since + * {@code HTTPSessionListenerPromise.failConnectionPromise} fails that same promise. Wrapping it + * in {@link #instrument} therefore covers all of them at once, with the address in hand, and + * without depending on per-phase listeners that each see only their own slice. + * + *

The recovered failures are attached to the sample's failure as suppressed + * exceptions: {@code HTTPSamplerBase.errorResult} writes {@code printStackTrace} into the response + * data, and that prints suppressed entries, so they show up in View Results Tree with no new + * formatting and without going through SLF4J - which the shaded jar has no provider for. + */ +public class ConnectAttemptRecorder { + + /** + * Recent failures kept per client. A run that cannot connect at all would otherwise grow this + * without bound; the last few are all a failing sample can use, since it only reads the ones + * recorded while it was running. + */ + private static final int DEFAULT_CAPACITY = 64; + + /** Cap on how many attempts are attached to a single failure, to keep the trace readable. */ + private static final int MAX_ATTACHED = 8; + + private static final String TCP_PHASE = "Connect to"; + private static final String TLS_PHASE = "TLS handshake with"; + private static final String SESSION_PHASE = "Connection to"; + + private final int capacity; + private final Deque attempts = new ArrayDeque<>(); + + public ConnectAttemptRecorder() { + this(DEFAULT_CAPACITY); + } + + public ConnectAttemptRecorder(int capacity) { + this.capacity = capacity; + } + + /** + * Wraps the per-address connection promise in {@code context} so this recorder sees its failure + * before Jetty decides to move on to the next address. + * + *

Called from the transport's {@code connect(SocketAddress, Map)}, which Jetty invokes once + * per resolved address with the promise for that address already in the context. Replacing it + * here is safe because everything downstream reads it back out of the context by key: + * {@code HttpClient.connect} binds {@code CONNECTION_PROMISE_CONTEXT_KEY} to it, and the HTTP/2 + * session listener resolves it lazily through {@code httpConnectionPromise()}. + */ + public void instrument(SocketAddress address, Map context) { + if (!(address instanceof InetSocketAddress) || context == null) { + return; + } + @SuppressWarnings("unchecked") + Promise promise = + (Promise) context.get(Connection.PROMISE_CONTEXT_KEY); + if (promise == null) { + return; + } + InetSocketAddress inetAddress = (InetSocketAddress) address; + context.put(Connection.PROMISE_CONTEXT_KEY, new Promise.Wrapper(promise) { + @Override + public void failed(Throwable failure) { + record(inetAddress, failure); + super.failed(failure); + } + }); + } + + /** + * Attaches to {@code target} every connection failure recorded for {@code url}'s origin while + * the sample was running. + * + *

Filtering by start time is what keeps the attribution honest without tracking which sample + * owns which socket: connects run on Jetty threads, and concurrent embedded resources share the + * client, so time plus origin is the correlation available. Attempts are attached as suppressed + * exceptions whose cause is the original failure, keeping its stack trace intact. + * + * @param target the failure about to be reported for the sample + * @param url the sampled URL, used to match host and port + * @param sinceMillis the sample start, in epoch milliseconds; {@code 0} takes everything held + */ + public void attachTo(Throwable target, URL url, long sinceMillis) { + if (target == null || url == null) { + return; + } + for (FailedAttempt attempt : failuresFor(url, sinceMillis)) { + // The attempt that survived Jetty's loop is already the cause of what is being reported; + // attaching it again would duplicate it and make printStackTrace emit a + // "[CIRCULAR REFERENCE]" marker. Only the discarded ones are missing. + if (!isInCauseChain(target, attempt.failure)) { + target.addSuppressed(new ConnectAttemptFailure(attempt)); + } + } + } + + private void record(InetSocketAddress address, Throwable failure) { + synchronized (attempts) { + attempts.addLast(new FailedAttempt(address, failure, System.currentTimeMillis())); + while (attempts.size() > capacity) { + attempts.removeFirst(); + } + } + } + + private static boolean isInCauseChain(Throwable target, Throwable failure) { + for (Throwable current = target; current != null; current = current.getCause()) { + if (current == failure) { + return true; + } + if (current.getCause() == current) { + break; + } + } + return false; + } + + private List failuresFor(URL url, long sinceMillis) { + String host = url.getHost(); + if (host == null || host.isEmpty()) { + return List.of(); + } + int port = url.getPort() >= 0 ? url.getPort() : url.getDefaultPort(); + List matches = new ArrayList<>(); + synchronized (attempts) { + for (FailedAttempt attempt : attempts) { + if (attempt.matches(host, port, sinceMillis)) { + matches.add(attempt); + } + } + } + if (matches.size() > MAX_ATTACHED) { + return matches.subList(matches.size() - MAX_ATTACHED, matches.size()); + } + return matches; + } + + /** + * Names the stage the attempt died at, from the failure itself. The cause is printed underneath + * anyway, so this is a label rather than a diagnosis - which is what makes deriving it cheaper + * than wiring one listener per phase. + */ + private static String phaseOf(Throwable failure) { + for (Throwable current = failure; current != null; current = current.getCause()) { + if (current instanceof SSLException) { + return TLS_PHASE; + } + if (current instanceof SocketException || current instanceof SocketTimeoutException + || current instanceof ConnectException) { + return TCP_PHASE; + } + if (current.getCause() == current) { + break; + } + } + return SESSION_PHASE; + } + + private static final class FailedAttempt { + + private final InetSocketAddress address; + private final Throwable failure; + private final long timestamp; + + private FailedAttempt(InetSocketAddress address, Throwable failure, long timestamp) { + this.address = address; + this.failure = failure; + this.timestamp = timestamp; + } + + private boolean matches(String host, int port, long sinceMillis) { + if (address.getPort() != port || timestamp < sinceMillis) { + return false; + } + // getHostString() carries the name the address was resolved from and never triggers a + // reverse lookup, so this stays off the network even for a literal-IP URL. + return address.getHostString().toLowerCase(Locale.ROOT).equals(host.toLowerCase(Locale.ROOT)); + } + + private String describe() { + String ip = address.getAddress() != null + ? address.getAddress().getHostAddress() + : address.getHostString(); + return phaseOf(failure) + " " + address.getHostString() + "/" + ip + ":" + address.getPort() + + " failed"; + } + } + + /** + * Carrier for one discarded attempt. Its own stack trace is deliberately not filled in: the + * frames that matter belong to the cause, and this exception is never thrown - it exists only to + * be printed under {@code Suppressed:}. + */ + public static class ConnectAttemptFailure extends Exception { + + private static final long serialVersionUID = 1L; + + private ConnectAttemptFailure(FailedAttempt attempt) { + super(attempt.describe(), attempt.failure, true, false); + } + } +} diff --git a/src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java b/src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java index 348d15db..7cb66063 100644 --- a/src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java +++ b/src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java @@ -18,6 +18,7 @@ import java.io.UnsupportedEncodingException; import java.lang.reflect.Method; import java.net.MalformedURLException; +import java.net.SocketAddress; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -317,6 +318,12 @@ public class HTTP2JettyClient { * {@code findAuthentication} (realm/URI matching quirks) and prevents per-sample list growth. */ private final Set registeredAuthFingerprints = ConcurrentHashMap.newKeySet(); + /** + * Recovers the per-address connection failures Jetty discards while walking the resolved + * addresses. Shared by every connector this client builds, so an attempt is captured whichever + * protocol variant made it. + */ + private final ConnectAttemptRecorder connectAttempts = new ConnectAttemptRecorder(); public HTTP2JettyClient(boolean http1UpgradeRequired, String name) { this(http1UpgradeRequired, name, null); @@ -568,7 +575,8 @@ public HTTP2JettyClient(boolean http1UpgradeRequired, String name, // even though ALPN negotiates HTTP/2 successfully. This is a regression from Jetty 11. // We try HTTP/2 first, then fallback to HTTP/1.1 if needed. ClientConnectionFactory.Info[] mainProtocols = buildMainProtocols(http3, http2, http11); - HttpClientTransport transport = new HttpClientTransportDynamic(clientConnector, mainProtocols); + HttpClientTransport transport = + new RecordingHttpClientTransportDynamic(clientConnector, mainProtocols); mainProtocolsSnapshot = protocolList(mainProtocols); lowLevelDebug("HttpClientTransportDynamic configured with protocols: {}", mainProtocolsSnapshot); @@ -585,13 +593,14 @@ public HTTP2JettyClient(boolean http1UpgradeRequired, String name, ClientConnector noH3Connector = createClientConnector(name + "-noh3"); ClientConnectionFactory.Info[] noH3Protocols = buildNoH3Protocols(http2, http11); HttpClientTransport noH3Transport = - new HttpClientTransportDynamic(noH3Connector, noH3Protocols); + new RecordingHttpClientTransportDynamic(noH3Connector, noH3Protocols); configureTransport(noH3Transport); this.httpClientNoH3 = new HttpClient(noH3Transport); configureHttpClient(this.httpClientNoH3, noH3Connector); } ClientConnector http1Connector = createClientConnector(name + "-http1"); - HttpClientTransport http1Transport = new HttpClientTransportDynamic(http1Connector, http11); + HttpClientTransport http1Transport = + new RecordingHttpClientTransportDynamic(http1Connector, http11); // HTTP/1.1 has no multiplexing (Jetty rejects a 2nd in-flight exchange per connection). configureTransport(http1Transport, 1); this.httpClientHttp1Only = new HttpClient(http1Transport); @@ -611,7 +620,7 @@ public HTTP2JettyClient(boolean http1UpgradeRequired, String name, ClientConnectionFactory.Info[] h2cUpgradeProtocols = buildH2cUpgradeProtocols(http11, http2cUpgrade); HttpClientTransport h2cUpgradeTransport = - new HttpClientTransportDynamic(h2cUpgradeConnector, h2cUpgradeProtocols); + new RecordingHttpClientTransportDynamic(h2cUpgradeConnector, h2cUpgradeProtocols); configureTransport(h2cUpgradeTransport); this.httpClientH2cUpgrade = new HttpClient(h2cUpgradeTransport); configureHttpClient(this.httpClientH2cUpgrade, h2cUpgradeConnector); @@ -625,7 +634,8 @@ public HTTP2JettyClient(boolean http1UpgradeRequired, String name, } else { http2cClient.setMaxConcurrentPushedStreams(maxConcurrentPushedStreams); } - HttpClientTransport h2cTransport = new CustomHttpClientTransportOverHTTP2(http2cClient); + HttpClientTransport h2cTransport = + new CustomHttpClientTransportOverHTTP2(http2cClient, connectAttempts); configureTransport(h2cTransport); this.httpClientH2cPrior = new HttpClient(h2cTransport); configureHttpClient(this.httpClientH2cPrior, h2cConnector); @@ -1228,7 +1238,8 @@ private HttpClient createHTTP11OnlyClient(String name) throws Exception { // Create transport with ONLY HTTP/1.1 (no HTTP/2) ClientConnectionFactory.Info http11 = HttpClientConnectionFactory.HTTP11; - HttpClientTransport transport = new HttpClientTransportDynamic(clientConnector, http11); + HttpClientTransport transport = + new RecordingHttpClientTransportDynamic(clientConnector, http11); lowLevelDebug("HttpClientTransportDynamic configured with HTTP/1.1 only (fallback mode)"); HttpClient http11Client = new HttpClient(transport); @@ -3330,6 +3341,28 @@ private Request cloneRequest(Request originalRequest, HttpClient client) return request; } + /** + * {@link HttpClientTransportDynamic} that lets {@link ConnectAttemptRecorder} see the failure of + * each resolved address before Jetty silently moves on to the next one. + * + *

This is the one point where the address being attempted and the promise that decides the + * roll-over are both in hand, which is why it covers a refused socket, a TLS handshake and the + * HTTP/2 preface alike. + */ + private class RecordingHttpClientTransportDynamic extends HttpClientTransportDynamic { + + RecordingHttpClientTransportDynamic(ClientConnector connector, + ClientConnectionFactory.Info... infos) { + super(connector, infos); + } + + @Override + public void connect(SocketAddress address, Map context) { + connectAttempts.instrument(address, context); + super.connect(address, context); + } + } + private static class HappyEyeballsThreadFactory implements ThreadFactory { private final String prefix; private final AtomicInteger counter = new AtomicInteger(1); @@ -3437,6 +3470,14 @@ private static Path resolveAlpnLogPath() { .resolve("http2-client-alpn.log"); } + /** + * Attaches every connection failure recorded for {@code url} since {@code sinceMillis} to + * {@code failure} as suppressed exceptions, recovering the attempts Jetty discarded. + */ + public void attachConnectAttempts(Throwable failure, URL url, long sinceMillis) { + connectAttempts.attachTo(failure, url, sinceMillis); + } + private ClientConnector createClientConnector(String name) { ClientConnector connector = new ClientConnector(); if (sharedThreadPoolEnabled) { diff --git a/src/main/java/com/blazemeter/jmeter/http2/core/jetty/custom/http2/CustomHttpClientTransportOverHTTP2.java b/src/main/java/com/blazemeter/jmeter/http2/core/jetty/custom/http2/CustomHttpClientTransportOverHTTP2.java index e0112f13..0ffd1cfe 100644 --- a/src/main/java/com/blazemeter/jmeter/http2/core/jetty/custom/http2/CustomHttpClientTransportOverHTTP2.java +++ b/src/main/java/com/blazemeter/jmeter/http2/core/jetty/custom/http2/CustomHttpClientTransportOverHTTP2.java @@ -1,6 +1,8 @@ package com.blazemeter.jmeter.http2.core.jetty.custom.http2; +import com.blazemeter.jmeter.http2.core.ConnectAttemptRecorder; import java.io.IOException; +import java.net.SocketAddress; import java.util.Map; import org.eclipse.jetty.http2.client.HTTP2Client; import org.eclipse.jetty.http2.client.transport.HttpClientTransportOverHTTP2; @@ -14,9 +16,29 @@ public class CustomHttpClientTransportOverHTTP2 extends HttpClientTransportOverH private final CustomHTTP2ClientConnectionFactory connectionFactory = new CustomHTTP2ClientConnectionFactory(); + private final ConnectAttemptRecorder connectAttempts; public CustomHttpClientTransportOverHTTP2(HTTP2Client http2Client) { + this(http2Client, null); + } + + public CustomHttpClientTransportOverHTTP2(HTTP2Client http2Client, + ConnectAttemptRecorder connectAttempts) { super(http2Client); + this.connectAttempts = connectAttempts; + } + + /** + * Lets the recorder see this address's failure before Jetty moves on to the next resolved + * address. This transport carries h2c prior knowledge, where the preface is sent with no ALPN + * to fall back on, so a server that does not speak HTTP/2 fails exactly here. + */ + @Override + public void connect(SocketAddress address, Map context) { + if (connectAttempts != null) { + connectAttempts.instrument(address, context); + } + super.connect(address, context); } @Override diff --git a/src/main/java/com/blazemeter/jmeter/http2/sampler/HTTP2Sampler.java b/src/main/java/com/blazemeter/jmeter/http2/sampler/HTTP2Sampler.java index 6840a2cf..9cd40f94 100644 --- a/src/main/java/com/blazemeter/jmeter/http2/sampler/HTTP2Sampler.java +++ b/src/main/java/com/blazemeter/jmeter/http2/sampler/HTTP2Sampler.java @@ -569,9 +569,32 @@ private HTTPSampleResult buildErrorResult(Exception e, HTTPSampleResult result) result.sampleEnd(); } } - return errorResult( - JmeterHttpClientExceptionMapper.forSampleResult(e, getAutoRedirects(), result.getURL()), - result); + Throwable failure = + JmeterHttpClientExceptionMapper.forSampleResult(e, getAutoRedirects(), result.getURL()); + attachConnectAttempts(failure, result); + return errorResult(failure, result); + } + + /** + * Recovers the per-address connection failures Jetty discarded while walking the resolved + * addresses, so the response data shows every address tried and why, not only the last one. + * + *

Reads the client already cached for this thread instead of asking the factory: building one + * here would be a side effect on the error path, and a sample that never got that far has + * nothing recorded anyway. + */ + private void attachConnectAttempts(Throwable failure, HTTPSampleResult result) { + if (failure == null || result.getURL() == null) { + return; + } + try { + HTTP2JettyClient client = CONNECTIONS.get().get(buildConnectionKey()); + if (client != null) { + client.attachConnectAttempts(failure, result.getURL(), result.getStartTime()); + } + } catch (Exception ignored) { + // Diagnostics must never replace the failure the sample is actually reporting. + } } /** diff --git a/src/test/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorderTest.java b/src/test/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorderTest.java new file mode 100644 index 00000000..c9e12ea7 --- /dev/null +++ b/src/test/java/com/blazemeter/jmeter/http2/core/ConnectAttemptRecorderTest.java @@ -0,0 +1,277 @@ +package com.blazemeter.jmeter.http2.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.net.ConnectException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.SocketException; +import java.net.URL; +import java.nio.channels.ClosedChannelException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import javax.net.ssl.SSLHandshakeException; +import org.eclipse.jetty.client.Connection; +import org.eclipse.jetty.util.Promise; +import org.junit.Test; + +/** + * Jetty keeps only the last address's failure when it walks the resolved addresses of a host, so + * on a dual-stack origin the IPv4 error - normally the interesting one, since the JDK tries it + * first - disappears. This pins the bookkeeping that gets it back: what the recorder captures when + * it wraps the per-address promise, which attempts belong to a given sample, and that wrapping the + * promise does not disturb the roll-over it is listening to. + */ +public class ConnectAttemptRecorderTest { + + private static final long EVERYTHING = 0; + + @Test + public void attachesEveryFailureRecordedForTheSampledOrigin() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + failAttempt(recorder, address("127.0.0.1", 8443), + new ConnectException("Connection refused: connect")); + failAttempt(recorder, address("::1", 8443), + new SocketException("Network is unreachable: connect")); + Throwable reported = new ConnectException("Connection refused: connect"); + + recorder.attachTo(reported, new URL("https://localhost:8443/x"), EVERYTHING); + + assertThat(suppressedMessages(reported)).containsExactly( + "Connect to localhost/127.0.0.1:8443 failed", + "Connect to localhost/0:0:0:0:0:0:0:1:8443 failed"); + } + + @Test + public void keepsTheOriginalFailureAsTheCauseOfEachAttempt() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + IOException original = new ConnectException("Connection refused: connect"); + failAttempt(recorder, address("127.0.0.1", 8443), original); + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://localhost:8443/x"), EVERYTHING); + + assertThat(reported.getSuppressed()[0].getCause()).isSameAs(original); + } + + @Test + public void namesTheStageEachAttemptDiedAt() throws Exception { + // One wrapper covers all three because they all fail the same per-address promise: the socket, + // the TLS handshake, and - after TLS already succeeded - the HTTP/2 preface. + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + failAttempt(recorder, address("127.0.0.1", 8443), new ConnectException("refused")); + failAttempt(recorder, address("127.0.0.1", 8443), new SSLHandshakeException("bad cert")); + failAttempt(recorder, address("127.0.0.1", 8443), new ClosedChannelException()); + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://localhost:8443/x"), EVERYTHING); + + assertThat(suppressedMessages(reported)).containsExactly( + "Connect to localhost/127.0.0.1:8443 failed", + "TLS handshake with localhost/127.0.0.1:8443 failed", + "Connection to localhost/127.0.0.1:8443 failed"); + } + + @Test + public void leavesTheWrappedPromiseDrivingTheRollOver() throws Exception { + // The wrapped promise is what makes Jetty try the next address. Swallowing either outcome + // here would turn a diagnostic into a hang or a lost connection. + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + AtomicReference delegatedFailure = new AtomicReference<>(); + AtomicReference delegatedSuccess = new AtomicReference<>(); + Map context = contextWith(new Promise() { + @Override + public void succeeded(Connection result) { + delegatedSuccess.set(result); + } + + @Override + public void failed(Throwable failure) { + delegatedFailure.set(failure); + } + }); + recorder.instrument(address("127.0.0.1", 8443), context); + ConnectException failure = new ConnectException("refused"); + + promiseIn(context).succeeded(null); + promiseIn(context).failed(failure); + + assertThat(delegatedSuccess.get()).isNull(); + assertThat(delegatedFailure.get()).isSameAs(failure); + } + + @Test + public void leavesTheContextAloneWhenThereIsNoPromiseToWrap() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + Map context = new HashMap<>(); + + recorder.instrument(address("127.0.0.1", 8443), context); + recorder.instrument(null, contextWith(noOpPromise())); + + assertThat(context).isEmpty(); + } + + @Test + public void ignoresAttemptsAgainstAnotherPort() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + failAttempt(recorder, address("127.0.0.1", 9999), new ConnectException("refused")); + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://localhost:8443/x"), EVERYTHING); + + assertThat(reported.getSuppressed()).isEmpty(); + } + + @Test + public void ignoresAttemptsAgainstAnotherHost() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + failAttempt(recorder, address("127.0.0.1", 8443), new ConnectException("refused")); + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://elsewhere.invalid:8443/x"), EVERYTHING); + + assertThat(reported.getSuppressed()).isEmpty(); + } + + @Test + public void ignoresAttemptsRecordedBeforeTheSampleStarted() throws Exception { + // Connects run on Jetty threads and concurrent embedded resources share the client, so the + // sample start is what keeps a neighbour's stale failure out of this result. + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + failAttempt(recorder, address("127.0.0.1", 8443), new ConnectException("refused")); + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://localhost:8443/x"), + System.currentTimeMillis() + 60_000); + + assertThat(reported.getSuppressed()).isEmpty(); + } + + @Test + public void matchesTheHostCaseInsensitively() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + failAttempt(recorder, address("127.0.0.1", 8443), new ConnectException("refused")); + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://LOCALHOST:8443/x"), EVERYTHING); + + assertThat(reported.getSuppressed()).hasSize(1); + } + + @Test + public void matchesTheDefaultPortOfTheScheme() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + failAttempt(recorder, address("127.0.0.1", 443), new ConnectException("refused")); + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://localhost/x"), EVERYTHING); + + assertThat(reported.getSuppressed()).hasSize(1); + } + + @Test + public void skipsTheAttemptThatIsAlreadyTheReportedCause() throws Exception { + // Re-attaching it would make printStackTrace emit a "[CIRCULAR REFERENCE]" marker in place of + // the failure. Only the discarded attempts are missing from the trace. + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + ConnectException survivor = new ConnectException("refused"); + failAttempt(recorder, address("127.0.0.1", 8443), survivor); + Throwable reported = new IOException("wrapper", survivor); + + recorder.attachTo(reported, new URL("https://localhost:8443/x"), EVERYTHING); + + assertThat(reported.getSuppressed()).isEmpty(); + } + + @Test + public void keepsOnlyTheMostRecentAttemptsWithinItsCapacity() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(2); + for (int i = 0; i < 5; i++) { + failAttempt(recorder, address("127.0.0.1", 8443), new ConnectException("refused " + i)); + } + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://localhost:8443/x"), EVERYTHING); + + assertThat(suppressedCauseMessages(reported)).containsExactly("refused 3", "refused 4"); + } + + @Test + public void attachesAtMostEightAttemptsToKeepTheTraceReadable() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + for (int i = 0; i < 20; i++) { + failAttempt(recorder, address("127.0.0.1", 8443), new ConnectException("refused " + i)); + } + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(reported, new URL("https://localhost:8443/x"), EVERYTHING); + + assertThat(reported.getSuppressed()).hasSize(8); + assertThat(suppressedCauseMessages(reported)).last().isEqualTo("refused 19"); + } + + @Test + public void toleratesMissingInputsRatherThanReplacingTheReportedFailure() throws Exception { + ConnectAttemptRecorder recorder = new ConnectAttemptRecorder(); + failAttempt(recorder, address("127.0.0.1", 8443), new ConnectException("refused")); + Throwable reported = new ConnectException("boom"); + + recorder.attachTo(null, new URL("https://localhost:8443/x"), EVERYTHING); + recorder.attachTo(reported, null, EVERYTHING); + + assertThat(reported.getSuppressed()).isEmpty(); + } + + /** Drives one address through the recorder the way Jetty's transport does. */ + private static void failAttempt(ConnectAttemptRecorder recorder, InetSocketAddress address, + Throwable failure) { + Map context = contextWith(noOpPromise()); + recorder.instrument(address, context); + promiseIn(context).failed(failure); + } + + private static Map contextWith(Promise promise) { + Map context = new HashMap<>(); + context.put(Connection.PROMISE_CONTEXT_KEY, promise); + return context; + } + + @SuppressWarnings("unchecked") + private static Promise promiseIn(Map context) { + return (Promise) context.get(Connection.PROMISE_CONTEXT_KEY); + } + + private static Promise noOpPromise() { + return new Promise<>() { + }; + } + + /** + * Mirrors how Jetty builds the address it hands to the transport: an {@code InetAddress} that + * still carries the name it was resolved from, so {@code getHostString()} returns that name. + * Built explicitly instead of resolving {@code localhost} so the test does not depend on whether + * this machine maps it to one address family or two. + */ + private static InetSocketAddress address(String ip, int port) throws Exception { + InetAddress resolved = + InetAddress.getByAddress("localhost", InetAddress.getByName(ip).getAddress()); + return new InetSocketAddress(resolved, port); + } + + private static List suppressedMessages(Throwable reported) { + return Arrays.stream(reported.getSuppressed()) + .map(Throwable::getMessage) + .collect(Collectors.toList()); + } + + private static List suppressedCauseMessages(Throwable reported) { + return Arrays.stream(reported.getSuppressed()) + .map(suppressed -> suppressed.getCause().getMessage()) + .collect(Collectors.toList()); + } +} diff --git a/src/test/java/com/blazemeter/jmeter/http2/core/ConnectAttemptVisibilityTest.java b/src/test/java/com/blazemeter/jmeter/http2/core/ConnectAttemptVisibilityTest.java new file mode 100644 index 00000000..ad66fb93 --- /dev/null +++ b/src/test/java/com/blazemeter/jmeter/http2/core/ConnectAttemptVisibilityTest.java @@ -0,0 +1,190 @@ +package com.blazemeter.jmeter.http2.core; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assume.assumeTrue; + +import com.blazemeter.jmeter.http2.HTTP2TestBase; +import com.blazemeter.jmeter.http2.sampler.HTTP2Sampler; +import java.io.IOException; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.util.ArrayList; +import java.util.List; +import org.apache.jmeter.protocol.http.util.HTTPConstants; +import org.apache.jmeter.samplers.SampleResult; +import org.apache.jmeter.util.JMeterUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * The end of the thread this started from: a host that resolves to more than one address and + * refuses all of them used to report only the last failure, because + * {@code HttpClient.connect(List, int, Map)} drops the earlier ones without a log or an + * {@code addSuppressed}. With the JDK trying IPv4 first, that meant the IPv6 error was the only + * one anybody ever saw. + * + *

{@code localhost} is the portable way to get a multi-address origin: it maps to + * {@code 127.0.0.1} and, on most machines, {@code ::1} as well. The assertion is written against + * whatever this machine actually resolves, so it still means something where only one family is + * configured - it just proves more where both are. + * + *

The two stages covered here, a refused socket and a failed TLS handshake, share the single + * recovery point with the third one - a failed HTTP/2 preface. Driving that one end to end would + * need either a hand-rolled ALPN server that goes quiet after the handshake, or an H2C upgrade + * timeout to reach {@code sendWithH2cPriorKnowledge}; it is covered in + * {@link ConnectAttemptRecorderTest} instead. + */ +public class ConnectAttemptVisibilityTest extends HTTP2TestBase { + + private static final String ENABLE_HTTP1 = "httpJettyClient.enableHttp1"; + private static final String ENABLE_HTTP2 = "httpJettyClient.enableHttp2"; + private static final String ENABLE_HTTP3 = "httpJettyClient.enableHttp3"; + + private final List savedProperties = new ArrayList<>(); + + private HTTP2Sampler sampler; + + @Before + public void setUp() throws Exception { + // Nothing is listening, so every protocol variant would just repeat the same refusals; one + // transport keeps the recorded attempts to the addresses actually under test. + overrideProperty(ENABLE_HTTP1, "true"); + overrideProperty(ENABLE_HTTP2, "false"); + overrideProperty(ENABLE_HTTP3, "false"); + HTTP2JettyClientTestIsolation.resetSharedClientState(); + } + + @After + public void tearDown() throws Exception { + if (sampler != null) { + sampler.threadFinished(); + } + for (String[] saved : savedProperties) { + if (saved[1] == null) { + JMeterUtils.getJMeterProperties().remove(saved[0]); + } else { + JMeterUtils.setProperty(saved[0], saved[1]); + } + } + } + + @Test + public void reportsTheAttemptsJettyDiscardedNotOnlyTheLastOne() throws Exception { + InetAddress[] addresses = InetAddress.getAllByName("localhost"); + assumeTrue("localhost resolves to a single address here, so nothing gets discarded", + addresses.length > 1); + int closedPort = closedPort(); + sampler = samplerAgainst(closedPort); + + SampleResult result = sampler.sample(); + + assertThat(result.isSuccessful()).isFalse(); + String responseData = result.getResponseDataAsString(); + // Jetty walks the addresses in resolution order and propagates only the last failure, so + // every address but the last is the one that used to vanish. + for (int i = 0; i < addresses.length - 1; i++) { + assertThat(responseData) + .as("the attempt against %s must be visible, not dropped by Jetty's connect loop", + addresses[i].getHostAddress()) + .contains("Connect to localhost/" + addresses[i].getHostAddress() + ":" + closedPort + + " failed"); + } + } + + @Test + public void doesNotDuplicateTheFailureThatSurvivedJettysLoop() throws Exception { + sampler = samplerAgainst(closedPort()); + + SampleResult result = sampler.sample(); + + // Re-attaching the surviving attempt would make printStackTrace emit this marker instead of + // the failure, which is worse than what it replaced. + assertThat(result.getResponseDataAsString()).doesNotContain("CIRCULAR REFERENCE"); + } + + @Test + public void reportsTheTlsHandshakesJettyDiscardedNotOnlyTheLastOne() throws Exception { + InetAddress[] addresses = InetAddress.getAllByName("localhost"); + assumeTrue("localhost resolves to a single address here, so nothing gets discarded", + addresses.length > 1); + // A plain HTTP server answering an https:// request fails the handshake deterministically: + // the ClientHello gets plaintext back. The socket connects first, so this is invisible to the + // connector's ConnectListener - it is the case the SslHandshakeListener exists for. + ServerBuilder.TeardownableServer server = new ServerBuilder().withHTTP1().buildServer(); + server.start(); + try { + int port = ((org.eclipse.jetty.server.ServerConnector) server.getConnectors()[0]) + .getLocalPort(); + sampler = samplerAgainst(port); + sampler.setProtocol(HTTPConstants.PROTOCOL_HTTPS); + + SampleResult result = sampler.sample(); + + assertThat(result.isSuccessful()).isFalse(); + String responseData = result.getResponseDataAsString(); + for (int i = 0; i < addresses.length - 1; i++) { + assertThat(responseData) + .as("the handshake against %s must be visible, not dropped by Jetty's connect loop", + addresses[i].getHostAddress()) + .contains("TLS handshake with localhost/" + addresses[i].getHostAddress() + ":" + port + + " failed"); + } + } finally { + server.stop(); + } + } + + @Test + public void keepsTheReportedFailureItselfUnchanged() throws Exception { + sampler = samplerAgainst(closedPort()); + + SampleResult result = sampler.sample(); + + // The attempts are added as suppressed exceptions, so the failure the sample reports - and the + // HttpClient4-shaped code JMeter puts in the result - must be exactly what it was before. + assertThat(result.getResponseCode()) + .isEqualTo("Non HTTP response code: org.apache.http.conn.HttpHostConnectException"); + } + + @Test + public void attachesNothingWhenTheSampleSucceeds() throws Exception { + ServerBuilder.TeardownableServer server = new ServerBuilder().withHTTP1().buildServer(); + server.start(); + try { + int port = ((org.eclipse.jetty.server.ServerConnector) server.getConnectors()[0]) + .getLocalPort(); + sampler = samplerAgainst(port); + sampler.setPath(ServerBuilder.SERVER_PATH_200); + + SampleResult result = sampler.sample(); + + assertThat(result.isSuccessful()).isTrue(); + assertThat(result.getResponseDataAsString()).doesNotContain("Connect to localhost/"); + } finally { + server.stop(); + } + } + + private HTTP2Sampler samplerAgainst(int port) { + HTTP2Sampler sampler = new HTTP2Sampler(); + sampler.setMethod(HTTPConstants.GET); + sampler.setDomain("localhost"); + sampler.setPort(port); + sampler.setPath("/"); + sampler.setProtocol("http"); + return sampler; + } + + private void overrideProperty(String key, String value) { + savedProperties.add(new String[] {key, JMeterUtils.getProperty(key)}); + JMeterUtils.setProperty(key, value); + } + + /** A port nothing is listening on: bound to learn it is free, then released. */ + private static int closedPort() throws IOException { + try (ServerSocket socket = new ServerSocket(0)) { + return socket.getLocalPort(); + } + } +} From 75e732171328cb92db3643e05849469c77245a6c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:39:27 +0000 Subject: [PATCH 2/2] Trim local address property check in source resolver Co-authored-by: 3dgiordano <797057+3dgiordano@users.noreply.github.com> --- .../jmeter/http2/core/JMeterSourceAddressResolver.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/blazemeter/jmeter/http2/core/JMeterSourceAddressResolver.java b/src/main/java/com/blazemeter/jmeter/http2/core/JMeterSourceAddressResolver.java index 4e03cd6b..a4474db3 100644 --- a/src/main/java/com/blazemeter/jmeter/http2/core/JMeterSourceAddressResolver.java +++ b/src/main/java/com/blazemeter/jmeter/http2/core/JMeterSourceAddressResolver.java @@ -82,7 +82,7 @@ public static String cacheKeyFor(HTTPSamplerBase sampler) { public static boolean isConfigured(HTTPSamplerBase sampler) { String ipSource = sampler.getIpSource(); return (ipSource != null && !ipSource.trim().isEmpty()) - || !JMeterUtils.getPropDefault(LOCAL_ADDRESS_PROPERTY, "").isEmpty(); + || !JMeterUtils.getPropDefault(LOCAL_ADDRESS_PROPERTY, "").trim().isEmpty(); } private static InetAddress resolveIpSource(String ipSource, int ipSourceType) @@ -131,7 +131,7 @@ private static InetAddress firstInterfaceAddress(String device, * is allowed to fail a sample. */ private static InetAddress resolveLocalAddressProperty() { - String localHostOrIp = JMeterUtils.getPropDefault(LOCAL_ADDRESS_PROPERTY, ""); + String localHostOrIp = JMeterUtils.getPropDefault(LOCAL_ADDRESS_PROPERTY, "").trim(); if (localHostOrIp.isEmpty()) { return null; }