Merge pull request #143 from zhenlineo/1.1-error-improvement

Better error handling

Neo4j.Driver/Neo4j.Driver.IntegrationTests/DirectDriver/AuthenticationIT.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using FluentAssertions;
 using Neo4j.Driver.Internal;
 using Neo4j.Driver.V1;
@@ -9,10 +10,22 @@ namespace Neo4j.Driver.IntegrationTests
 {
     public class AuthenticationIT : DirectDriverIT
     {
-        public AuthenticationIT(ITestOutputHelper output, IntegrationTestFixture fixture)
+        public AuthenticationIT(ITestOutputHelper output, StandAloneIntegrationTestFixture fixture)
             : base(output, fixture)
         { }
 
+        [Fact]
+        public void AuthenticationErrorIfWrongAuthToken()
+        {
+            Exception exception;
+            using (var driver = GraphDatabase.Driver(ServerEndPoint, AuthTokens.Basic("fake", "fake")))
+            {
+                exception = Record.Exception(()=>driver.Session());
+            }
+            exception.Should().BeOfType<AuthenticationException>();
+            exception.Message.Should().Contain("The client is unauthorized due to authentication failure.");
+        }
+
         [Fact]
         public void ShouldProvideRealmWithBasicAuthToken()
         {

Neo4j.Driver/Neo4j.Driver.IntegrationTests/DirectDriver/DirectDriverIT.cs

@@ -23,16 +23,16 @@
 
 namespace Neo4j.Driver.IntegrationTests
 {
-    [Collection(IntegrationCollection.CollectionName)]
-    public class DirectDriverIT : IDisposable
+    [Collection(SAIntegrationCollection.CollectionName)]
+    public abstract class DirectDriverIT : IDisposable
     {
         public static readonly Config DebugConfig = Config.Builder.WithLogger(new DebugLogger {Level = LogLevel.Debug}).ToConfig();
         protected ITestOutputHelper Output { get; }
         protected StandAlone Server { get; }
         protected string ServerEndPoint { get; }
         protected IAuthToken AuthToken { get; }
 
-        public DirectDriverIT(ITestOutputHelper output, IntegrationTestFixture fixture)
+        protected DirectDriverIT(ITestOutputHelper output, StandAloneIntegrationTestFixture fixture)
         {
             Output = output;
             Server = fixture.StandAlone;

Neo4j.Driver/Neo4j.Driver.IntegrationTests/DirectDriver/ResultIT.cs

@@ -12,7 +12,7 @@ public class ResultIT : DirectDriverIT
     {
         private IDriver Driver => Server.Driver;
 
-        public ResultIT(ITestOutputHelper output, IntegrationTestFixture fixture) : base(output, fixture)
+        public ResultIT(ITestOutputHelper output, StandAloneIntegrationTestFixture fixture) : base(output, fixture)
         {}
 
         [Fact]

Neo4j.Driver/Neo4j.Driver.IntegrationTests/DirectDriver/SessionIT.cs

@@ -4,17 +4,32 @@
 using Xunit;
 using Xunit.Abstractions;
 using System.Linq;
+using System.Net.Sockets;
 
 namespace Neo4j.Driver.IntegrationTests
 {
     public class SessionIT : DirectDriverIT
     {
         private IDriver Driver => Server.Driver;
 
-        public SessionIT(ITestOutputHelper output, IntegrationTestFixture fixture) : base(output, fixture)
+        public SessionIT(ITestOutputHelper output, StandAloneIntegrationTestFixture fixture) : base(output, fixture)
         {
         }
 
+        [Fact]
+        public void ServiceUnavailableErrorWhenFailedToConn()
+        {
+            Exception exception;
+            using (var driver = GraphDatabase.Driver("bolt://localhost:123"))
+            {
+               exception = Record.Exception(()=>driver.Session());
+            }
+            exception.Should().BeOfType<ServiceUnavailableException>();
+            exception.Message.Should().Be("Connection with the server breaks due to AggregateException: One or more errors occurred.");
+            exception.GetBaseException().Should().BeOfType<SocketException>();
+            exception.GetBaseException().Message.Should().Contain("No connection could be made because the target machine actively refused it");
+        }
+
         [Fact]
         public void DisallowNewSessionAfterDriverDispose()
         {

Neo4j.Driver/Neo4j.Driver.IntegrationTests/DirectDriver/SessionResetIT.cs

@@ -15,7 +15,7 @@ public class SessionResetIT : DirectDriverIT
     {
         private IDriver Driver => Server.Driver;
 
-        public SessionResetIT(ITestOutputHelper output, IntegrationTestFixture fixture)
+        public SessionResetIT(ITestOutputHelper output, StandAloneIntegrationTestFixture fixture)
             : base(output, fixture)
         {
             Server.RestartServerWithProcedures(new DirectoryInfo("../../Resources/longRunningStatement.jar").FullName);

Neo4j.Driver/Neo4j.Driver.IntegrationTests/Examples.cs

@@ -27,13 +27,13 @@
 
 namespace Neo4j.Driver.Examples
 {
-    [Collection(IntegrationCollection.CollectionName)]
+    [Collection(SAIntegrationCollection.CollectionName)]
     public class Examples : IDisposable
     {
         private ITestOutputHelper Output { get; }
         private IDriver Driver { get; }
 
-        public Examples(ITestOutputHelper output, IntegrationTestFixture fixture)
+        public Examples(ITestOutputHelper output, StandAloneIntegrationTestFixture fixture)
         {
             Output = output;
             Driver = fixture.StandAlone.Driver;

Neo4j.Driver/Neo4j.Driver.IntegrationTests/Internals/IntegrationTestFixture.cs

@@ -20,37 +20,64 @@
 
 namespace Neo4j.Driver.IntegrationTests
 {
-    public class IntegrationTestFixture : IDisposable
+    public class StandAloneIntegrationTestFixture : IDisposable
     {
         public StandAlone StandAlone { get; }
-        public CausalCluster Cluster { get; }
 
-        public IntegrationTestFixture()
+        public StandAloneIntegrationTestFixture()
         {
             try
             {
                 StandAlone = new StandAlone();
-                Cluster = new CausalCluster();
             }
             catch (Exception)
             {
                 Dispose();
                 throw;
             }
-
         }
 
         public void Dispose()
         {
             StandAlone?.Dispose();
+        }
+    }
+
+    public class CausalClusterIntegrationTestFixture : IDisposable
+    {
+        public CausalCluster Cluster { get; }
+
+        public CausalClusterIntegrationTestFixture()
+        {
+            try
+            {
+                Cluster = new CausalCluster();
+            }
+            catch (Exception)
+            {
+                Dispose();
+                throw;
+            }
+        }
+        public void Dispose()
+        {
             Cluster?.Dispose();
         }
     }
 
     [CollectionDefinition(CollectionName)]
-    public class IntegrationCollection : ICollectionFixture<IntegrationTestFixture>
+    public class SAIntegrationCollection : ICollectionFixture<StandAloneIntegrationTestFixture>
+    {
+        public const string CollectionName = "StandAloneIntegration";
+        // This class has no code, and is never created. Its purpose is simply
+        // to be the place to apply [CollectionDefinition] and all the
+        // ICollectionFixture<> interfaces.
+    }
+
+    [CollectionDefinition(CollectionName)]
+    public class CCIntegrationCollection : ICollectionFixture<CausalClusterIntegrationTestFixture>
     {
-        public const string CollectionName = "Integration";
+        public const string CollectionName = "CausalClusterIntegration";
         // This class has no code, and is never created. Its purpose is simply
         // to be the place to apply [CollectionDefinition] and all the
         // ICollectionFixture<> interfaces.

Neo4j.Driver/Neo4j.Driver.IntegrationTests/RoutingDriver/RoutingDriverIT.cs

@@ -10,8 +10,7 @@
 
 namespace Neo4j.Driver.IntegrationTests
 {
-    // If I have a cluster, then I should be able to do the following tests
-    [Collection(IntegrationCollection.CollectionName)]
+    [Collection(CCIntegrationCollection.CollectionName)]
     public class RoutingDriverIT : IDisposable
     {
         public static readonly Config DebugConfig = Config.Builder.WithLogger(new DebugLogger { Level = LogLevel.Debug }).ToConfig();
@@ -22,7 +21,7 @@ public class RoutingDriverIT : IDisposable
         private string RoutingServer => Cluster.AnyCore().BoltRoutingUri.ToString();
         private string WrongServer => "bolt+routing://localhost:1234";
 
-        public RoutingDriverIT(ITestOutputHelper output, IntegrationTestFixture fixture)
+        public RoutingDriverIT(ITestOutputHelper output, CausalClusterIntegrationTestFixture fixture)
         {
             Output = output;
             Cluster = fixture.Cluster;

Neo4j.Driver/Neo4j.Driver.Tck.Tests/Tck/ErrorReportingSteps.cs

@@ -77,7 +77,7 @@ public void WhenISetUpADriverToAnIncorrectPort()
             using (var driver = GraphDatabase.Driver("bolt://localhost:1234"))
             {
                 var ex = Xunit.Record.Exception(() => driver.Session());
-                ex.Should().BeOfType<AggregateException>();
+                ex.Should().BeOfType<ServiceUnavailableException>();
                 ex = ex.GetBaseException();
                 ex.Should().BeOfType<SocketException>();
                 ex.Message.Should().Be("No connection could be made because the target machine actively refused it 127.0.0.1:1234");

Neo4j.Driver/Neo4j.Driver.Tests/ConnectionPoolTests.cs

@@ -57,11 +57,11 @@ public void ShouldAddExternalConnectionHandlerIfNotNull()
                 var mockedHandler = new Mock<IConnectionErrorHandler>();
                 var connectionPool = new ConnectionPool(mock.Object, exteralErrorHandler:mockedHandler.Object);
                 // When
-                connectionPool.Acquire();
+                var conn = connectionPool.Acquire();
 
                 //Then
-                mock.Verify(x=>x.AddConnectionErrorHander(mockedHandler.Object), Times.Once);
-                mock.Verify(x => x.Init(), Times.Once);
+                mock.Verify(x => x.ExternalConnectionErrorHander(It.IsAny<IConnectionErrorHandler>()), Times.Once);
+                ((PooledConnection)conn).ExternalConnectionErrorHandler().Should().Be(mockedHandler.Object);
             }
 
             [Fact]

Neo4j.Driver/Neo4j.Driver.Tests/Connector/MessageResponseHandlerTests.cs

@@ -219,6 +219,30 @@ public void ShouldCreateClientExceptionWhenClassificationContainsClientError(str
                 mrh.Error.Should().BeOfType<ClientException>();
             }
 
+            [Theory, MemberData("ProtocolErrors")]
+            public void ShouldCreateProtocolExceptionWhenClassificationContainsProtocolError(string code)
+            {
+                var mockResultBuilder = new Mock<IMessageResponseCollector>();
+                var mrh = new MessageResponseHandler();
+                mrh.EnqueueMessage(new PullAllMessage(), mockResultBuilder.Object);
+
+                mrh.HandleFailureMessage(code, "message");
+                mrh.HasError.Should().BeTrue();
+                mrh.Error.Should().BeOfType<ProtocolException>();
+            }
+
+            [Theory, MemberData("AuthErrors")]
+            public void ShouldCreateAuthExceptionWhenClassificationContainsAuthError(string code)
+            {
+                var mockResultBuilder = new Mock<IMessageResponseCollector>();
+                var mrh = new MessageResponseHandler();
+                mrh.EnqueueMessage(new PullAllMessage(), mockResultBuilder.Object);
+
+                mrh.HandleFailureMessage(code, "message");
+                mrh.HasError.Should().BeTrue();
+                mrh.Error.Should().BeOfType<AuthenticationException>();
+            }
+
             [Theory, MemberData("TransientErrors")]
             public void ShouldCreateTransientExceptionWhenClassificationContainsTransientError(string code)
             {
@@ -291,12 +315,22 @@ public void LogsTheMessageToDebug()
             }
 
             #region Test Data
+
+            public static IEnumerable<object[]> ProtocolErrors => new[]
+            {
+                new object[] {"Neo.ClientError.Request.Invalid"},
+                new object[] {"Neo.ClientError.Request.InvalidFormat"}
+            };
+
+            public static IEnumerable<object[]> AuthErrors => new[]
+            {
+                new object[] {"Neo.ClientError.Security.Unauthorized"}
+            };
+
             public static IEnumerable<object[]> ClientErrors => new[]
             {
                 new object[] {"Neo.ClientError.General.ReadOnly"},
                 new object[] {"Neo.ClientError.LegacyIndex.NoSuchIndex"},
-                new object[] {"Neo.ClientError.Request.Invalid"},
-                new object[] {"Neo.ClientError.Request.InvalidFormat"},
                 new object[] {"Neo.ClientError.Schema.ConstraintAlreadyExists"},
                 new object[] {"Neo.ClientError.Schema.ConstraintVerificationFailure"},
                 new object[] {"Neo.ClientError.Schema.ConstraintViolation"},

Neo4j.Driver/Neo4j.Driver.Tests/Connector/PooledConnectionTests.cs

@@ -33,7 +33,7 @@ public void ShouldAddPooledConnectionErrorHandler()
             {
                 var mockedSocketConn = new Mock<IConnection>();
                 var conn = new PooledConnection(mockedSocketConn.Object);
-                mockedSocketConn.Verify(x=>x.AddConnectionErrorHander(It.IsAny<PooledConnection.PooledConnectionErrorHandler>()), Times.Once);
+                mockedSocketConn.Verify(x=>x.ExternalConnectionErrorHander(It.IsAny<PooledConnection.PooledConnectionErrorHandler>()), Times.Once);
             }
         }
 

Neo4j.Driver/Neo4j.Driver.Tests/Connector/SocketClientTests.cs

@@ -61,7 +61,7 @@ public async Task ShouldThrowExceptionIfVersionIsNotSupported(byte[] response, s
             }
         }
 
-        public class SendMethod
+        public class SendReceiveMethod
         {
             [Fact]
             public async Task ShouldSendMessagesAsExpected()
@@ -204,12 +204,12 @@ public async Task ShouldStopClientAndThrowExceptionWhenProtocolErrorOccurs()
                     await harness.Client.Start();
 
                     // force to recive an error
-                    messageHandler.Error = new ClientException("Neo.ClientError.Request.Invalid", "Test Message");
+                    messageHandler.Error = new ProtocolException("Neo.ClientError.Request.Invalid", "Test Message");
 
                     // When
                     harness.Client.Send(messages);
                     var ex = Record.Exception(() => harness.Client.Receive(messageHandler));
-                    ex.Should().BeOfType<ClientException>();
+                    ex.Should().BeOfType<ProtocolException>();
 
                     harness.MockTcpSocketClient.Verify(x => x.DisconnectAsync(), Times.Once);
                     harness.MockTcpSocketClient.Verify(x => x.Dispose(), Times.Once);

Neo4j.Driver/Neo4j.Driver.Tests/Connector/SocketConnectionTests.cs

@@ -16,6 +16,7 @@
 // limitations under the License.
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Threading.Tasks;
 using FluentAssertions;
 using Moq;
@@ -118,7 +119,7 @@ public void ShouldThrowClientErrorIfFailedToConnectToServerWithinTimeout()
                 // When
                 var error = Exception(()=>conn.Init());
                 // Then
-                error.Should().BeOfType<ClientException>();
+                error.Should().BeOfType<IOException>();
                 error.Message.Should().Be("Failed to connect to the server neo4j.com:80 within connection timeout 5000ms");
             }
         }

Neo4j.Driver/Neo4j.Driver.Tests/PackStream/PackerTests.cs

@@ -22,6 +22,7 @@
 using Neo4j.Driver.Internal.Connector;
 using Neo4j.Driver.Internal.Packstream;
 using Neo4j.Driver.Internal;
+using Neo4j.Driver.V1;
 using Xunit;
 
 namespace Neo4j.Driver.Tests
@@ -440,7 +441,7 @@ public void ShouldThrowExceptionIfTypeUnknown()
                 {
                     var packer = new PackStream.Packer(null);
                     var ex = Xunit.Record.Exception(() => packer.Pack(new {Name = "Test"}));
-                    ex.Should().BeOfType<ArgumentOutOfRangeException>();
+                    ex.Should().BeOfType<ProtocolException>();
                 }
             }
 
@@ -564,7 +565,7 @@ public void ShouldThrowExceptionIfSizeIsGreaterThanShortMax()
                 {
                     var packer = new PackStream.Packer(null);
                     var ex = Xunit.Record.Exception(() => packer.PackStructHeader(short.MaxValue +1, 0x1));
-                    ex.Should().BeOfType<ArgumentOutOfRangeException>();
+                    ex.Should().BeOfType<ProtocolException>();
                 }
             }