Merge #1626 into 1.0.7

violetagg · violetagg · commit fb9d6837ed5d · 2021-05-07T15:18:07.000+03:00
diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java
@@ -344,7 +344,7 @@ public void onUncaughtException(Connection connection, Throwable error) {
 			}
 			else if (handler.shouldRetry && AbortedException.isConnectionReset(error)) {
 				HttpClientOperations ops = connection.as(HttpClientOperations.class);
-				if (ops != null) {
+				if (ops != null && ops.hasSentHeaders()) {
 					// In some cases the channel close event may be delayed and thus the connection to be
 					// returned to the pool and later the eviction functionality to remote it from the pool.
 					// In some rare cases the connection might be acquired immediately, before the channel close
@@ -353,11 +353,30 @@ else if (handler.shouldRetry && AbortedException.isConnectionReset(error)) {
 					// Mark the connection as non-persistent here so that it never be returned to the pool and leave
 					// the channel close event to invalidate it.
 					ops.markPersistent(false);
-					ops.retrying = true;
+					// Disable retry if the headers or/and the body were sent
+					handler.shouldRetry = false;
+					if (log.isWarnEnabled()) {
+						log.warn(format(connection.channel(),
+								"The connection observed an error, the request cannot be " +
+										"retried as the headers/body were sent"), error);
+					}
 				}
-				if (log.isDebugEnabled()) {
-					log.debug(format(connection.channel(),
-							"The connection observed an error, the request will be retried"), error);
+				else {
+					if (ops != null) {
+						// In some cases the channel close event may be delayed and thus the connection to be
+						// returned to the pool and later the eviction functionality to remote it from the pool.
+						// In some rare cases the connection might be acquired immediately, before the channel close
+						// event and the eviction functionality be able to remove it from the pool, this may lead to I/O
+						// errors.
+						// Mark the connection as non-persistent here so that it never be returned to the pool and leave
+						// the channel close event to invalidate it.
+						ops.markPersistent(false);
+						ops.retrying = true;
+					}
+					if (log.isDebugEnabled()) {
+						log.debug(format(connection.channel(),
+								"The connection observed an error, the request will be retried"), error);
+					}
 				}
 			}
 			else if (log.isWarnEnabled()) {
diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java
@@ -1264,15 +1264,20 @@ void testExplicitSendMonoErrorOnGet() {
 
 	@Test
 	void testRetryNotEndlessIssue587() throws Exception {
-		doTestRetry(false);
+		doTestRetry(false, true);
+	}
+
+	@Test
+	void testRetryDisabledWhenHeadersSent() throws Exception {
+		doTestRetry(false, false);
 	}
 
 	@Test
 	void testRetryDisabledIssue995() throws Exception {
-		doTestRetry(true);
+		doTestRetry(true, false);
 	}
 
-	private void doTestRetry(boolean retryDisabled) throws Exception {
+	private void doTestRetry(boolean retryDisabled, boolean expectRetry) throws Exception {
 		ExecutorService threadPool = Executors.newCachedThreadPool();
 		int serverPort = SocketUtils.findAvailableTcpPort();
 		ConnectionResetByPeerServer server = new ConnectionResetByPeerServer(serverPort);
@@ -1295,8 +1300,14 @@ private void doTestRetry(boolean retryDisabled) throws Exception {
 		}
 
 		AtomicReference<Throwable> error = new AtomicReference<>();
-		StepVerifier.create(client.get()
+		StepVerifier.create(client.request(HttpMethod.GET)
 		                          .uri("/")
+		                          .send((req, out) -> {
+		                              if (expectRetry) {
+		                                  return Mono.error(new IOException("Connection reset by peer"));
+		                              }
+		                              return out;
+		                          })
 		                          .responseContent())
 		            .expectErrorMatches(t -> {
 		                error.set(t);
@@ -1308,7 +1319,7 @@ private void doTestRetry(boolean retryDisabled) throws Exception {
 
 		int requestCount = 1;
 		int requestErrorCount = 1;
-		if (!retryDisabled && !(error.get() instanceof PrematureCloseException)) {
+		if (expectRetry && !(error.get() instanceof PrematureCloseException)) {
 			requestCount = 2;
 			requestErrorCount = 2;
 		}
