Skip to content

Commit 960abea

Browse files
hyperxproclaude
andcommitted
Retry a refused connect on every JDK and transport
NettyConnectListener applies maxRequestRetry to a failure on a NEW channel only when StackTraceInspector deems it recoverable, and that check searched the cause chain for the frame sun.nio.ch.SocketChannelImpl.checkConnect. That frame exists up to JDK 12 only: JDK 13 moved the completion of a non-blocking connect to sun.nio.ch.Net.pollConnect, and the native transports report the failure as a ConnectException from io.netty.channel.unix.Errors with no sun.nio.ch frame at all. A refused TCP connect was therefore retried on JDK 11 with NIO and silently not retried on JDK 13, 17, 21 and 25, nor on epoll/kqueue/io_uring, which became the default where the library is present in #2216. Match the refusal by type as well, and keep scanning from the cause rather than from the throwable itself: what the listener receives is an annotating wrapper, and NettyChannelConnector wraps anything that is not already a ConnectException in one, so the wrapper's own type carries no information. Netty's connect timeout is a ConnectException too and is excluded explicitly, so a blackholed host still fails after one connectTimeout rather than maxRequestRetry of them. That exclusion, like the whole predicate, only governs a request's first attempt: retry() sets ChannelState.RECONNECTED, and from then on the gate in NettyConnectListener.onFailure short-circuits on the state and never consults the predicate. That is long-standing behaviour and is left alone here. Behaviour change: on JDK 13+ and on the native transports a refused connect is retried up to maxRequestRetry again, immediately and with DNS re-resolved per attempt, so onTcpConnectAttempt and onTcpConnectFailure fire up to (1 + maxRequestRetry) times the number of resolved addresses, and the time to fail against a dead port rises accordingly. Known residual, pre-existing and not introduced here: the native transports map ENETUNREACH and EHOSTUNREACH to a bare NoRouteToHostException, a sibling of ConnectException with no sun.nio.ch frame, so an unreachable peer is retried on NIO and not on a native transport. No connect backoff is added. Claude Code on behalf of Aayush Atharva Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 684c5df commit 960abea

6 files changed

Lines changed: 400 additions & 1 deletion

File tree

client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,10 @@ default long getMaxDecompressedResponseSize() {
335335
/**
336336
* Return the number of time the library will retry when an {@link IOException} is throw by the remote server
337337
*
338+
* <p>A TCP connect the peer refused is retried as well, since no request byte reached it. On a
339+
* request's first attempt a connect that timed out is not; from its first retry onwards any failure
340+
* is. Retries are immediate, with no backoff, and the host is re-resolved for each attempt.
341+
*
338342
* @return the number of time the library will retry when an {@link IOException} is throw by the remote server
339343
*/
340344
int getMaxRequestRetry();

client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,6 +1277,13 @@ public Builder setRealm(Realm.Builder realmBuilder) {
12771277
return this;
12781278
}
12791279

1280+
/**
1281+
* Sets how many times a request is replayed, as described by
1282+
* {@link AsyncHttpClientConfig#getMaxRequestRetry()}.
1283+
*
1284+
* @param maxRequestRetry the number of retries, {@code 0} to disable them
1285+
* @return this builder
1286+
*/
12801287
public Builder setMaxRequestRetry(int maxRequestRetry) {
12811288
this.maxRequestRetry = maxRequestRetry;
12821289
return this;

client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,12 @@ private void registerHttp2AndManageSemaphore(Channel channel, ConnectionSemaphor
328328
}
329329
}
330330

331+
/**
332+
* Invariant every caller must keep: this is only reached before {@link #writeRequest}, never after. It
333+
* may replay the exchange, so a call site added after the request had been written could silently
334+
* resend it. The channel is published to the handler inside writeRequest, so a later failure is
335+
* reported through {@code AsyncHttpClientHandler.channelInactive} instead.
336+
*/
331337
public void onFailure(Channel channel, Throwable cause) {
332338

333339
// beware, channel can be null

client/src/main/java/org/asynchttpclient/netty/future/StackTraceInspector.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
*/
1616
package org.asynchttpclient.netty.future;
1717

18+
import io.netty.channel.ConnectTimeoutException;
19+
1820
import java.io.IOException;
21+
import java.net.ConnectException;
1922
import java.nio.channels.ClosedChannelException;
2023

2124
public final class StackTraceInspector {
@@ -38,7 +41,19 @@ private static boolean exceptionInMethod(Throwable t, String className, String m
3841

3942
private static boolean recoverOnConnectCloseException(Throwable t) {
4043
while (true) {
41-
if (exceptionInMethod(t, "sun.nio.ch.SocketChannelImpl", "checkConnect")) {
44+
// A connect timeout is a ConnectException as well, but replaying it would multiply the time
45+
// to fail by maxRequestRetry; only a connect the peer actually refused is replayed.
46+
if (t instanceof ConnectTimeoutException) {
47+
return false;
48+
}
49+
// Match the refusal by type as well as by frame: NIO moved the frame from
50+
// SocketChannelImpl.checkConnect (JDK <= 12) to Net.pollConnect (JDK 13+), and the native
51+
// transports report it with no sun.nio.ch frame at all. The frame probes are not redundant
52+
// with the type check: they are what keeps an unreachable peer, reported as a
53+
// NoRouteToHostException rather than a ConnectException, recoverable on NIO.
54+
if (t instanceof ConnectException
55+
|| exceptionInMethod(t, "sun.nio.ch.SocketChannelImpl", "checkConnect")
56+
|| exceptionInMethod(t, "sun.nio.ch.Net", "pollConnect")) {
4257
return true;
4358
}
4459
if (t.getCause() == null) {
@@ -49,6 +64,9 @@ private static boolean recoverOnConnectCloseException(Throwable t) {
4964
}
5065

5166
public static boolean recoverOnNettyDisconnectException(Throwable t) {
67+
// Deliberately scanned from the cause: the throwable handed to the connect listener is an
68+
// annotating wrapper, and NettyChannelConnector wraps anything that is not already a
69+
// ConnectException in one, so its own type carries no information.
5270
return t instanceof ClosedChannelException
5371
|| exceptionInMethod(t, "io.netty.handler.ssl.SslHandler", "disconnect")
5472
|| t.getCause() != null && recoverOnConnectCloseException(t.getCause());
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
/*
2+
* Copyright (c) 2026 AsyncHttpClient Project. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.asynchttpclient.netty.channel;
17+
18+
import io.netty.channel.Channel;
19+
import io.netty.resolver.AbstractAddressResolver;
20+
import io.netty.resolver.AddressResolver;
21+
import io.netty.resolver.AddressResolverGroup;
22+
import io.netty.util.concurrent.EventExecutor;
23+
import io.netty.util.concurrent.Promise;
24+
import jakarta.servlet.ServletException;
25+
import jakarta.servlet.http.HttpServletRequest;
26+
import jakarta.servlet.http.HttpServletResponse;
27+
import org.apache.commons.io.IOUtils;
28+
import org.asynchttpclient.AbstractBasicTest;
29+
import org.asynchttpclient.AsyncCompletionHandler;
30+
import org.asynchttpclient.AsyncHttpClient;
31+
import org.asynchttpclient.Response;
32+
import org.eclipse.jetty.server.Request;
33+
import org.eclipse.jetty.server.handler.AbstractHandler;
34+
import org.junit.jupiter.api.Test;
35+
36+
import java.io.IOException;
37+
import java.net.InetAddress;
38+
import java.net.InetSocketAddress;
39+
import java.nio.charset.StandardCharsets;
40+
import java.util.Collections;
41+
import java.util.List;
42+
import java.util.concurrent.TimeUnit;
43+
import java.util.concurrent.atomic.AtomicInteger;
44+
45+
import static org.asynchttpclient.Dsl.asyncHttpClient;
46+
import static org.asynchttpclient.Dsl.config;
47+
import static org.asynchttpclient.test.TestUtils.findFreePort;
48+
import static org.junit.jupiter.api.Assertions.assertEquals;
49+
50+
/**
51+
* A TCP connect the peer refused fails before the request is written, so {@code maxRequestRetry} applies to
52+
* it, on every JDK and every transport. Nothing reached the server, so the replay is safe whatever the
53+
* method, and it must send the entity exactly once.
54+
*/
55+
public class ConnectFailureRetryTest extends AbstractBasicTest {
56+
57+
private static final String BODY = "connect-retry-body";
58+
59+
private final AtomicInteger requestsReceived = new AtomicInteger();
60+
private final AtomicInteger bodiesReceived = new AtomicInteger();
61+
62+
@Override
63+
public AbstractHandler configureHandler() {
64+
return new CountingEchoHandler();
65+
}
66+
67+
@Test
68+
public void refusedConnectIsRetriedOnTheNextResolution() throws Exception {
69+
requestsReceived.set(0);
70+
// The first resolution points at a closed port, so the connect is refused before any byte is
71+
// written; every later resolution points at the live server. Only a request retry can succeed.
72+
SwitchingResolverGroup resolverGroup = new SwitchingResolverGroup(findFreePort(), port1);
73+
try {
74+
try (AsyncHttpClient client = asyncHttpClient(config()
75+
.setAddressResolverGroup(resolverGroup)
76+
.setMaxRequestRetry(1))) {
77+
Response response = client.prepareGet(getTargetUrl()).execute().get(TIMEOUT, TimeUnit.SECONDS);
78+
assertEquals(200, response.getStatusCode());
79+
assertEquals(2, resolverGroup.resolutions(), "the refused connect was not retried");
80+
assertEquals(1, requestsReceived.get(), "the refused attempt must not have reached the server");
81+
}
82+
} finally {
83+
resolverGroup.close();
84+
}
85+
}
86+
87+
/**
88+
* The replay of a refused connect has to resend the entity, and has to send it once: the first attempt
89+
* never reached the server, so a duplicate here would be a duplicate POST on the wire. The connect
90+
* count pins the invariant stated on {@code NettyConnectListener.onFailure} - a connect that succeeded
91+
* is a request that was written, so a second success would mean a written request had been replayed.
92+
*/
93+
@Test
94+
public void refusedConnectReplaysThePostBodyExactlyOnce() throws Exception {
95+
requestsReceived.set(0);
96+
bodiesReceived.set(0);
97+
SwitchingResolverGroup resolverGroup = new SwitchingResolverGroup(findFreePort(), port1);
98+
try {
99+
try (AsyncHttpClient client = asyncHttpClient(config()
100+
.setAddressResolverGroup(resolverGroup)
101+
.setMaxRequestRetry(1))) {
102+
ConnectCountingHandler handler = new ConnectCountingHandler();
103+
Response response = client.preparePost(getTargetUrl())
104+
.setBody(BODY)
105+
.execute(handler)
106+
.get(TIMEOUT, TimeUnit.SECONDS);
107+
assertEquals(200, response.getStatusCode());
108+
assertEquals(BODY, response.getResponseBody());
109+
assertEquals(1, requestsReceived.get(), "the request was sent more than once");
110+
assertEquals(1, bodiesReceived.get(), "the body was sent more than once");
111+
assertEquals(1, handler.connectSuccesses.get(), "a written request was replayed");
112+
assertEquals(1, handler.connectFailures.get(), "the refused attempt was not counted");
113+
}
114+
} finally {
115+
resolverGroup.close();
116+
}
117+
}
118+
119+
private static final class ConnectCountingHandler extends AsyncCompletionHandler<Response> {
120+
121+
private final AtomicInteger connectSuccesses = new AtomicInteger();
122+
private final AtomicInteger connectFailures = new AtomicInteger();
123+
124+
@Override
125+
public void onTcpConnectSuccess(InetSocketAddress remoteAddress, Channel connection) {
126+
connectSuccesses.incrementAndGet();
127+
}
128+
129+
@Override
130+
public void onTcpConnectFailure(InetSocketAddress remoteAddress, Throwable cause) {
131+
connectFailures.incrementAndGet();
132+
}
133+
134+
@Override
135+
public Response onCompleted(Response response) {
136+
return response;
137+
}
138+
}
139+
140+
private final class CountingEchoHandler extends AbstractHandler {
141+
142+
@Override
143+
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response)
144+
throws IOException, ServletException {
145+
requestsReceived.incrementAndGet();
146+
String body = IOUtils.toString(request.getInputStream(), StandardCharsets.UTF_8);
147+
if (!body.isEmpty()) {
148+
bodiesReceived.incrementAndGet();
149+
}
150+
response.setStatus(200);
151+
response.getOutputStream().write(body.getBytes(StandardCharsets.UTF_8));
152+
response.getOutputStream().flush();
153+
baseRequest.setHandled(true);
154+
}
155+
}
156+
157+
// Hands out the refusing address once, then the live one, so the retry is driven by the resolution
158+
// count rather than by timing.
159+
private static final class SwitchingResolverGroup extends AddressResolverGroup<InetSocketAddress> {
160+
161+
private final AtomicInteger resolutions = new AtomicInteger();
162+
private final int firstPort;
163+
private final int remainingPort;
164+
165+
SwitchingResolverGroup(int firstPort, int remainingPort) {
166+
this.firstPort = firstPort;
167+
this.remainingPort = remainingPort;
168+
}
169+
170+
int resolutions() {
171+
return resolutions.get();
172+
}
173+
174+
@Override
175+
protected AddressResolver<InetSocketAddress> newResolver(EventExecutor executor) {
176+
return new AbstractAddressResolver<InetSocketAddress>(executor, InetSocketAddress.class) {
177+
178+
@Override
179+
protected boolean doIsResolved(InetSocketAddress address) {
180+
return !address.isUnresolved();
181+
}
182+
183+
@Override
184+
protected void doResolve(InetSocketAddress unresolvedAddress, Promise<InetSocketAddress> promise) {
185+
promise.setSuccess(next());
186+
}
187+
188+
@Override
189+
protected void doResolveAll(InetSocketAddress unresolvedAddress, Promise<List<InetSocketAddress>> promise) {
190+
promise.setSuccess(Collections.singletonList(next()));
191+
}
192+
};
193+
}
194+
195+
private InetSocketAddress next() {
196+
int port = resolutions.getAndIncrement() == 0 ? firstPort : remainingPort;
197+
return new InetSocketAddress(InetAddress.getLoopbackAddress(), port);
198+
}
199+
}
200+
}

0 commit comments

Comments
 (0)