From d5cc66b982361d25f8f72e7ab09f1f6743363e33 Mon Sep 17 00:00:00 2001 From: Andrew Cann Date: Fri, 28 Jun 2019 13:59:57 +0800 Subject: [PATCH 1/8] Make tcp connections timeout after 1sec Fixes https://github.com/nblockchain/TcpEchoSharp/issues/7 --- src/Client.Tests/ClientTests.cs | 28 ++++++++++++++++++++++++++++ src/Client/Client.cs | 6 ++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Client.Tests/ClientTests.cs b/src/Client.Tests/ClientTests.cs index 075723f..a474fab 100644 --- a/src/Client.Tests/ClientTests.cs +++ b/src/Client.Tests/ClientTests.cs @@ -1,5 +1,6 @@ using System; using System.Threading.Tasks; +using System.Diagnostics; using NUnit.Framework; namespace Client.Tests { @@ -74,5 +75,32 @@ public async Task ConnectWithElectrumServersEstimateFee () { } Assert.AreEqual (hasAtLeastOneSuccessful, true); } + + [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.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 index 5576f12..f60e81a 100644 --- a/src/Client/Client.cs +++ b/src/Client/Client.cs @@ -31,7 +31,9 @@ private async Task CallImpl (string json) { using (Socket socket = new Socket (SocketType.Stream, ProtocolType.Tcp)) { socket.ReceiveTimeout = 500; - await socket.ConnectAsync (endpoint, port); + if (!socket.ConnectAsync (endpoint, port).Wait (1000)) { + throw new TimeoutException("connect timed out"); + } byte[] bytesToSend = UTF8Encoding.UTF8.GetBytes (json + Environment.NewLine); socket.Send (bytesToSend); @@ -146,4 +148,4 @@ private static ArraySegment GetArray(ReadOnlyMemory memory) } } -#endif \ No newline at end of file +#endif From 1ef94a89ea67d85704c0e6db8dc06cb7ec5db3d7 Mon Sep 17 00:00:00 2001 From: Andrew Cann Date: Fri, 28 Jun 2019 14:14:37 +0800 Subject: [PATCH 2/8] Add tcp timeout constant Put tcp timeout in a constant and use it for the receive timeout aswell. --- src/Client/Client.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Client/Client.cs b/src/Client/Client.cs index f60e81a..96f599a 100644 --- a/src/Client/Client.cs +++ b/src/Client/Client.cs @@ -19,6 +19,7 @@ internal ConnectionUnsuccessfulException(string msg, Exception 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,9 @@ public Client (string _endpoint, int _port) { private async Task CallImpl (string json) { using (Socket socket = new Socket (SocketType.Stream, ProtocolType.Tcp)) { - socket.ReceiveTimeout = 500; + socket.ReceiveTimeout = tcpTimeout; - if (!socket.ConnectAsync (endpoint, port).Wait (1000)) { + if (!socket.ConnectAsync (endpoint, port).Wait (tcpTimeout)) { throw new TimeoutException("connect timed out"); } From 08be1aff8d4baed049eb2280dc204236b13cc578 Mon Sep 17 00:00:00 2001 From: Andrew Cann Date: Fri, 28 Jun 2019 15:08:02 +0800 Subject: [PATCH 3/8] make tcp timeout asynchronous --- src/Client/Client.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Client/Client.cs b/src/Client/Client.cs index 96f599a..d53bf1c 100644 --- a/src/Client/Client.cs +++ b/src/Client/Client.cs @@ -32,7 +32,9 @@ private async Task CallImpl (string json) { using (Socket socket = new Socket (SocketType.Stream, ProtocolType.Tcp)) { socket.ReceiveTimeout = tcpTimeout; - if (!socket.ConnectAsync (endpoint, port).Wait (tcpTimeout)) { + var timeOut = Task.Delay (tcpTimeout); + var completedTask = await Task.WhenAny (timeOut, socket.ConnectAsync (endpoint, port)); + if (completedTask == timeOut) { throw new TimeoutException("connect timed out"); } From 353e9d6bb5243b531b9066db1bd53a088826952a Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Fri, 28 Jun 2019 15:51:34 +0800 Subject: [PATCH 4/8] Catch timeout exception to be wrapped in ConnectionUnsuccessfulException --- src/Client/Client.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Client/Client.cs b/src/Client/Client.cs index d53bf1c..31d35fd 100644 --- a/src/Client/Client.cs +++ b/src/Client/Client.cs @@ -61,6 +61,10 @@ protected async Task Call(string json) { throw new ConnectionUnsuccessfulException(ex.Message, ex); } + catch (TimeoutException ex) + { + throw new ConnectionUnsuccessfulException(ex.Message, ex); + } } private static async Task WriteToPipeAsync (Socket socket, PipeWriter writer) { From a0c919563845724f0cbe96359af4f7a9c2324fc3 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 29 Jun 2019 17:40:20 +0800 Subject: [PATCH 5/8] ClientTests: better logging --- src/Client.Tests/ClientTests.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Client.Tests/ClientTests.cs b/src/Client.Tests/ClientTests.cs index a474fab..717bde2 100644 --- a/src/Client.Tests/ClientTests.cs +++ b/src/Client.Tests/ClientTests.cs @@ -37,23 +37,26 @@ public class ClientTest { [Test] public async Task ConnectWithElectrumServersTransactionGet () { var hasAtLeastOneSuccessful = false; + 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 + Console.WriteLine("success"); hasAtLeastOneSuccessful = true; break; } catch (TcpEcho.ConnectionUnsuccessfulException error) { - Console.Error.WriteLine ($"Couldn't request {servers[i]}: {error}"); + Console.Error.WriteLine ("failure"); } catch (AggregateException aggEx) { if (!(aggEx.InnerException is TcpEcho.ConnectionUnsuccessfulException)) throw; + Console.Error.WriteLine ("failure"); } } Assert.AreEqual (hasAtLeastOneSuccessful, true); From 1aa58f940ecc895fd78b71bf67550f25d55ee969 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 29 Jun 2019 17:48:33 +0800 Subject: [PATCH 6/8] Also handle timeouts after conn has been established --- src/Client.Tests/ClientTests.cs | 4 ++-- src/Client/Client.cs | 21 +++++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Client.Tests/ClientTests.cs b/src/Client.Tests/ClientTests.cs index 717bde2..27c5144 100644 --- a/src/Client.Tests/ClientTests.cs +++ b/src/Client.Tests/ClientTests.cs @@ -49,12 +49,12 @@ public async Task ConnectWithElectrumServersTransactionGet () { Console.WriteLine("success"); hasAtLeastOneSuccessful = true; break; - } catch (TcpEcho.ConnectionUnsuccessfulException error) { + } 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"); } diff --git a/src/Client/Client.cs b/src/Client/Client.cs index 31d35fd..f95f0f3 100644 --- a/src/Client/Client.cs +++ b/src/Client/Client.cs @@ -10,9 +10,9 @@ 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) { } } @@ -32,9 +32,9 @@ private async Task CallImpl (string json) { using (Socket socket = new Socket (SocketType.Stream, ProtocolType.Tcp)) { socket.ReceiveTimeout = tcpTimeout; - var timeOut = Task.Delay (tcpTimeout); - var completedTask = await Task.WhenAny (timeOut, socket.ConnectAsync (endpoint, port)); - if (completedTask == timeOut) { + var connectTimeOut = Task.Delay (tcpTimeout); + var completedConnTask = await Task.WhenAny (connectTimeOut, socket.ConnectAsync (endpoint, port)); + if (completedConnTask == connectTimeOut) { throw new TimeoutException("connect timed out"); } @@ -45,7 +45,12 @@ private async Task 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; } @@ -59,11 +64,11 @@ protected async Task Call(string json) } catch (SocketException ex) { - throw new ConnectionUnsuccessfulException(ex.Message, ex); + throw new CommunicationUnsuccessfulException(ex.Message, ex); } catch (TimeoutException ex) { - throw new ConnectionUnsuccessfulException(ex.Message, ex); + throw new CommunicationUnsuccessfulException(ex.Message, ex); } } From 523b8b2fd6015c03ca70defa5af6e43d0cbd12cd Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 29 Jun 2019 17:57:40 +0800 Subject: [PATCH 7/8] Make test reach all servers, and count, not stop at 1st successful We can do this now that we have proper timeout handling. This way it will be easier to catch connection/transmission bugs. --- src/Client.Tests/ClientTests.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Client.Tests/ClientTests.cs b/src/Client.Tests/ClientTests.cs index 27c5144..e8f960f 100644 --- a/src/Client.Tests/ClientTests.cs +++ b/src/Client.Tests/ClientTests.cs @@ -36,7 +36,7 @@ public class ClientTest { [Test] public async Task ConnectWithElectrumServersTransactionGet () { - var hasAtLeastOneSuccessful = false; + var successfulCount = 0; Console.WriteLine(); for (int i = 0; i < servers.Length; i++) { Console.Write($"Trying to query '{servers[i]}'... "); @@ -47,8 +47,7 @@ public async Task ConnectWithElectrumServersTransactionGet () { "2f309ef555110ab4e9c920faa2d43e64f195aa027e80ec28e1d243bd8929a2fc" ); Console.WriteLine("success"); - hasAtLeastOneSuccessful = true; - break; + successfulCount++; } catch (TcpEcho.CommunicationUnsuccessfulException error) { Console.Error.WriteLine ("failure"); } @@ -59,7 +58,7 @@ public async Task ConnectWithElectrumServersTransactionGet () { Console.Error.WriteLine ("failure"); } } - Assert.AreEqual (hasAtLeastOneSuccessful, true); + Assert.That (successfulCount, Is.GreaterThan(1)); } [Test] From d1f6706cfe1252d3af503376d7058bcf7980a5f5 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Sat, 29 Jun 2019 18:18:16 +0800 Subject: [PATCH 8/8] Refactor tests to follow same structure --- src/Client.Tests/ClientTests.cs | 37 ++++++++++++++++----------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/Client.Tests/ClientTests.cs b/src/Client.Tests/ClientTests.cs index e8f960f..5693831 100644 --- a/src/Client.Tests/ClientTests.cs +++ b/src/Client.Tests/ClientTests.cs @@ -4,6 +4,7 @@ using NUnit.Framework; namespace Client.Tests { + [TestFixture] public class ClientTest { public static readonly string[] servers = { @@ -34,18 +35,14 @@ public class ClientTest { "y4td57fxytoo5ki7.onion", }; - [Test] - public async Task ConnectWithElectrumServersTransactionGet () { + private async Task LoopThroughElectrumServers (Func 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" - ); + await action(client); Console.WriteLine("success"); successfulCount++; } catch (TcpEcho.CommunicationUnsuccessfulException error) { @@ -61,21 +58,23 @@ public async Task ConnectWithElectrumServersTransactionGet () { 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}"); - } - } - Assert.AreEqual (hasAtLeastOneSuccessful, true); + await LoopThroughElectrumServers(async client => { + var result = await client.BlockchainEstimateFee (17, 6); + Assert.That(result, Is.Not.Null); + }); } [Test]