Merge pull request davidfowl#8 from canndrew/tcp-timeout-fix

Make tcp connections timeout after 1sec
knocte · Jun 29, 2019 · c89ea5c · c89ea5c
2 parents cf7c569 + d1f6706
commit c89ea5c
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 33 deletions.
diff --git a/src/Client.Tests/ClientTests.cs b/src/Client.Tests/ClientTests.cs
@@ -1,8 +1,10 @@
 using System;
 using System.Threading.Tasks;
+using System.Diagnostics;
 using NUnit.Framework;
 
 namespace Client.Tests {
+
     [TestFixture]
     public class ClientTest {
         public static readonly string[] servers = {
@@ -33,46 +35,73 @@ public class ClientTest {
             "y4td57fxytoo5ki7.onion",
         };
 
-        [Test]
-        public async Task ConnectWithElectrumServersTransactionGet () {
-            var hasAtLeastOneSuccessful = false;
+        private async Task LoopThroughElectrumServers (Func<TcpEcho.StratumClient,Task> action) {
+            var successfulCount = 0;
+            Console.WriteLine();
             for (int i = 0; i < servers.Length; i++) {
+                Console.Write($"Trying to query '{servers[i]}'... ");
                 try {
                     var client = new TcpEcho.StratumClient (servers[i], 50001);
-                    var result = await client.BlockchainTransactionGet (
-                        17,
-                        "2f309ef555110ab4e9c920faa2d43e64f195aa027e80ec28e1d243bd8929a2fc"
-                    );
-                    Console.Error.WriteLine (result.Result); // Using stderr to show into the console
-                    hasAtLeastOneSuccessful = true;
-                    break;
-                } catch (TcpEcho.ConnectionUnsuccessfulException error) {
-                    Console.Error.WriteLine ($"Couldn't request {servers[i]}: {error}");
+                    await action(client);
+                    Console.WriteLine("success");
+                    successfulCount++;
+                } catch (TcpEcho.CommunicationUnsuccessfulException error) {
+                    Console.Error.WriteLine ("failure");
                 }
                 catch (AggregateException aggEx)
                 {
-                    if (!(aggEx.InnerException is TcpEcho.ConnectionUnsuccessfulException))
+                    if (!(aggEx.InnerException is TcpEcho.CommunicationUnsuccessfulException))
                         throw;
+                    Console.Error.WriteLine ("failure");
                 }
             }
-            Assert.AreEqual (hasAtLeastOneSuccessful, true);
+            Assert.That (successfulCount, Is.GreaterThan(1));
+        }
+
+        [Test]
+        public async Task ConnectWithElectrumServersTransactionGet () {
+            await LoopThroughElectrumServers(async client => {
+                var result = await client.BlockchainTransactionGet (
+                    17,
+                    "2f309ef555110ab4e9c920faa2d43e64f195aa027e80ec28e1d243bd8929a2fc"
+                );
+                Assert.That(result, Is.Not.Null);
+            });
         }
 
         [Test]
         public async Task ConnectWithElectrumServersEstimateFee () {
-            var hasAtLeastOneSuccessful = false;
-            for (int i = 0; i < servers.Length; i++) {
-                try {
-                    var client = new TcpEcho.StratumClient (servers[i], 50001);
-                    var result = await client.BlockchainEstimateFee (17, 6);
-                    Console.Error.WriteLine (result.Result); // Using stderr to show into the console
-                    hasAtLeastOneSuccessful = true;
-                    break;
-                } catch (Exception error) {
-                    Console.Error.WriteLine ($"Couldn't request {servers[i]}: {error}");
-                }
+            await LoopThroughElectrumServers(async client => {
+                var result = await client.BlockchainEstimateFee (17, 6);
+                Assert.That(result, Is.Not.Null);
+            });
+        }
+
+        [Test]
+        public async Task ProperNonEternalTimeout()
+        {
+            var someRandomIP = "52.1.57.181";
+            bool? succesful = null;
+
+            var stopWatch = new Stopwatch();
+            stopWatch.Start();
+            try
+            {
+                var client = new TcpEcho.StratumClient(someRandomIP, 50001);
+                var result = await client.BlockchainTransactionGet(
+                    17,
+                    "2f309ef555110ab4e9c920faa2d43e64f195aa027e80ec28e1d243bd8929a2fc"
+                );
+                succesful = true;
+            }
+            catch
+            {
+                succesful = false;
+                stopWatch.Stop();
             }
-            Assert.AreEqual (hasAtLeastOneSuccessful, true);
+            Assert.That(succesful.HasValue, Is.EqualTo(true), "test is broken?");
+            Assert.That(succesful.Value, Is.EqualTo(false), "IP is not too random? port was open actually!");
+            Assert.That(stopWatch.Elapsed, Is.LessThan(TimeSpan.FromSeconds(2)));
         }
     }
 }
diff --git a/src/Client/Client.cs b/src/Client/Client.cs
@@ -10,15 +10,16 @@
 
 namespace TcpEcho {
 
-    public class ConnectionUnsuccessfulException : Exception
+    public class CommunicationUnsuccessfulException : Exception
     {
-        internal ConnectionUnsuccessfulException(string msg, Exception innerException) : base(msg, innerException)
+        internal CommunicationUnsuccessfulException(string msg, Exception innerException) : base(msg, innerException)
         {
         }
     }
 
     public abstract class Client {
         private const int minimumBufferSize = 1024;
+        private const int tcpTimeout = 1000;
         private string endpoint = "";
         private int port = 0;
 
@@ -29,9 +30,13 @@ public Client (string _endpoint, int _port) {
 
         private async Task<string> CallImpl (string json) {
             using (Socket socket = new Socket (SocketType.Stream, ProtocolType.Tcp)) {
-                socket.ReceiveTimeout = 500;
+                socket.ReceiveTimeout = tcpTimeout;
 
-                await socket.ConnectAsync (endpoint, port);
+                var connectTimeOut = Task.Delay (tcpTimeout);
+                var completedConnTask = await Task.WhenAny (connectTimeOut, socket.ConnectAsync (endpoint, port));
+                if (completedConnTask == connectTimeOut) {
+                    throw new TimeoutException("connect timed out");
+                }
 
                 byte[] bytesToSend = UTF8Encoding.UTF8.GetBytes (json + Environment.NewLine);
                 socket.Send (bytesToSend);
@@ -40,7 +45,12 @@ private async Task<string> CallImpl (string json) {
                 var writing = WriteToPipeAsync (socket, pipe.Writer);
                 var reading = ReadFromPipeAsync (pipe.Reader);
 
-                await Task.WhenAll (reading, writing);
+                var readAndWriteTask = Task.WhenAll (reading, writing);
+                var readTimeOut = Task.Delay (tcpTimeout);
+                var completedReadTask = await Task.WhenAny (readTimeOut, readAndWriteTask);
+                if (completedReadTask == readTimeOut) {
+                    throw new TimeoutException("reading/writing from socket timed out");
+                }
 
                 return await reading;
             }
@@ -54,7 +64,11 @@ protected async Task<string> Call(string json)
             }
             catch (SocketException ex)
             {
-                throw new ConnectionUnsuccessfulException(ex.Message, ex);
+                throw new CommunicationUnsuccessfulException(ex.Message, ex);
+            }
+            catch (TimeoutException ex)
+            {
+                throw new CommunicationUnsuccessfulException(ex.Message, ex);
             }
         }
 
@@ -146,4 +160,4 @@ private static ArraySegment<byte> GetArray(ReadOnlyMemory<byte> memory)
     }
 }
 
-#endif
+#endif