#802 added additional checks if hard exceptions occur in IConnection (not only faulted tasks, synchronous version was ok but async version did not failover properly

Author: Mpdreamz

Diff for: src/Elasticsearch.Net/Connection/Transport.cs

@@ -6,6 +6,7 @@
 using System.Net;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
+using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 using Elasticsearch.Net.Connection.Configuration;
@@ -89,15 +90,24 @@ private Task<bool> PingAsync(ITransportRequestState requestState)
 				RequestTimeout = pingTimeout
 			};
 			var rq = requestState.InitiateRequest(RequestType.Ping);
-			return this.Connection.Head(requestState.CreatePathOnCurrentNode(""), requestOverrides)
-				.ContinueWith(t =>
-				{
-					rq.Finish(t.Result.Success, t.Result.HttpStatusCode);
-					rq.Dispose();
-					var response = t.Result;
-					using (response.Response)
-						return response.Success;
-				});
+			try
+			{
+				return this.Connection.Head(requestState.CreatePathOnCurrentNode(""), requestOverrides)
+					.ContinueWith(t =>
+					{
+						rq.Finish(t.Result.Success, t.Result.HttpStatusCode);
+						rq.Dispose();
+						var response = t.Result;
+						using (response.Response)
+							return response.Success;
+					});
+			}
+			catch (Exception e)
+			{
+				var tcs = new TaskCompletionSource<bool>();
+				tcs.SetException(e);
+				return tcs.Task;
+			}
 		}
 
 		private IList<Uri> Sniff(ITransportRequestState ownerState = null)
@@ -403,14 +413,14 @@ private Task<ElasticsearchResponse<T>> DoRequestAsync<T>(TransportRequestState<T
 				return this.PingAsync(requestState)
 					.ContinueWith(t =>
 					{
+						if (t.IsFaulted)
+							return this.RetryRequestAsync(requestState, t.Exception);
 						if (t.IsCompleted)
 						{
 							if (!t.Result)
 								return this.RetryRequestAsync(requestState, t.Exception);
 							return this.FinishOrRetryRequestAsync(requestState);
 						}
-						if (t.IsFaulted)
-							return this.RetryRequestAsync(requestState, t.Exception);
 						return null;
 					}).Unwrap();
 			}

Diff for: src/Tests/Elasticsearch.Net.Tests.Unit/Connection/RetryTests.cs

@@ -22,7 +22,7 @@ public class RetryTests
 			.MaximumRetries(_retries);
 
 		[Test]
-		public void ThrowsOutOfNodesException_AndRetriesTheSpecifiedTimes()
+		public void ThrowsMaxRetryException_AndRetriesTheSpecifiedTimes()
 		{
 			using (var fake = new AutoFake(callsDoNothing: true))
 			{
@@ -43,7 +43,7 @@ public void ThrowsOutOfNodesException_AndRetriesTheSpecifiedTimes()
 		}
 
 		[Test]
-		public void ThrowsOutOfNodesException_AndRetriesTheSpecifiedTimes_Async()
+		public void ThrowsMaxRetryException_AndRetriesTheSpecifiedTimes_Async()
 		{
 			using (var fake = new AutoFake(callsDoNothing: true))
 			{
@@ -179,6 +179,6 @@ public void ShouldRetryOn503_Async()
 				getCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
 			}
 		}
-
+		
 	}
 }

Diff for: src/Tests/Elasticsearch.Net.Tests.Unit/Connection/StaticConnectionPoolRetryTests.cs

@@ -2,6 +2,8 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Runtime.InteropServices;
+using System.Threading.Tasks;
 using Autofac;
 using Autofac.Extras.FakeItEasy;
 using Elasticsearch.Net.Connection;
@@ -372,5 +374,77 @@ public void IfAllButOneConnectionDiesSubsequentRequestsMustUseTheOneAliveConnect
 				markLastAlive.MustHaveHappened(Repeated.Exactly.Times(4));
 			}
 		}
+
+		[Test]
+		public void ShouldRetryOnPingConnectionException_Async()
+		{
+			using (var fake = new AutoFake(callsDoNothing: true))
+			{	
+				var connectionPool = new StaticConnectionPool(_uris, randomizeOnStartup: false);
+				var config = new ConnectionConfiguration(connectionPool);
+				
+				fake.Provide<IConnectionConfigurationValues>(config);
+				FakeCalls.ProvideDefaultTransport(fake);
+
+				var pingCall = FakeCalls.PingAtConnectionLevelAsync(fake);
+				var seenPorts = new List<int>();
+				pingCall.ReturnsLazily((Uri u, IRequestConfiguration c) =>
+				{
+					seenPorts.Add(u.Port);
+					throw new Exception("Something bad happened");
+				});
+
+				var getCall = FakeCalls.GetCall(fake);
+				getCall.Returns(FakeResponse.OkAsync(config));
+
+				var client = fake.Resolve<ElasticsearchClient>();
+
+				var e = Assert.Throws<MaxRetryException>(async () => await client.InfoAsync());
+				pingCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
+				getCall.MustNotHaveHappened();
+				
+				//make sure that if a ping throws an exception it wont
+				//keep retrying to ping the same node but failover to the next
+				seenPorts.ShouldAllBeEquivalentTo(_uris.Select(u=>u.Port));
+				var ae = e.InnerException as AggregateException;
+				ae = ae.Flatten();
+				ae.InnerException.Message.Should().Be("Something bad happened");
+			}
+		}
+		
+		[Test]
+		public void ShouldRetryOnPingConnectionException()
+		{
+			using (var fake = new AutoFake(callsDoNothing: true))
+			{	
+				var connectionPool = new StaticConnectionPool(_uris, randomizeOnStartup: false);
+				var config = new ConnectionConfiguration(connectionPool);
+				
+				fake.Provide<IConnectionConfigurationValues>(config);
+				FakeCalls.ProvideDefaultTransport(fake);
+
+				var pingCall = FakeCalls.PingAtConnectionLevel(fake);
+				var seenPorts = new List<int>();
+				pingCall.ReturnsLazily((Uri u, IRequestConfiguration c) =>
+				{
+					seenPorts.Add(u.Port);
+					throw new Exception("Something bad happened");
+				});
+
+				var getCall = FakeCalls.GetSyncCall(fake);
+				getCall.Returns(FakeResponse.Ok(config));
+
+				var client = fake.Resolve<ElasticsearchClient>();
+
+				var e = Assert.Throws<MaxRetryException>(() => client.Info());
+				pingCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
+				getCall.MustNotHaveHappened();
+				
+				//make sure that if a ping throws an exception it wont
+				//keep retrying to ping the same node but failover to the next
+				seenPorts.ShouldAllBeEquivalentTo(_uris.Select(u=>u.Port));
+				e.InnerException.Message.Should().Be("Something bad happened");
+			}
+		}
 	}
 }