Add new exception types and change to explicitly indetifying fast fail errors (#668)

* Add new exception types and change to explicitly idetifying fast fail errors

* Add some missing fail fast types

* Added unknown security exceptions

* Fix TokenExpiredException reporting to testkit

* Address review notes

Author: RichardIrons-neo4j

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Exceptions/ExceptionManager.cs

@@ -29,29 +29,32 @@ internal static class ExceptionManager
     {
         private static Dictionary<Type, string> TypeMap { get; set; } = new Dictionary<Type, string>()
         {
-            { typeof(Neo4jException),                   "Neo4jError" },
-            { typeof(ClientException),                  "ClientError" },
-            { typeof(TransientException),               "DriverError" },        //Should maybe Transient error, talk to Peter or Martin
-            { typeof(DatabaseException),                "DatabaseError" },
-            { typeof(ServiceUnavailableException),      "ServiceUnavailableError" },
-            { typeof(SessionExpiredException),          "SessionExpiredError" },
-            { typeof(Driver.ProtocolException),         "ProtocolError" },
-            { typeof(SecurityException),                "SecurityError" },
-            { typeof(AuthenticationException),          "AuthenticationError" },
-            { typeof(AuthorizationException),           "AuthorizationExpired" },
-            { typeof(ValueTruncationException),         "ValueTruncationError" },
-            { typeof(ValueOverflowException),           "ValueOverflowError" },
-            { typeof(FatalDiscoveryException),          "FatalDiscoveryError" },
-            { typeof(ResultConsumedException),          "ResultConsumedError" },
-            { typeof(TransactionNestingException),      "TransactionNestingException" },
-            { typeof(TokenExpiredException),			"TokenExpiredError"  },
-            { typeof(ConnectionReadTimeoutException),   "ConnectionReadTimeoutError"},
-            { typeof(InvalidBookmarkException),			"InvalidBookmarkError"},
-            { typeof(TransactionClosedException),       "ClientError"},
-
-            { typeof(NotSupportedException),            "NotSupportedException" },
-
-            { typeof(ArgumentException),				"ArgumentError"}
+            { typeof(Neo4jException), "Neo4jError" },
+            { typeof(ClientException), "ClientError" },
+            { typeof(TransientException), "DriverError" }, //Should maybe Transient error, talk to Peter or Martin
+            { typeof(DatabaseException), "DatabaseError" },
+            { typeof(ServiceUnavailableException), "ServiceUnavailableError" },
+            { typeof(SessionExpiredException), "SessionExpiredError" },
+            { typeof(Driver.ProtocolException), "ProtocolError" },
+            { typeof(SecurityException), "SecurityError" },
+            { typeof(AuthenticationException), "AuthenticationError" },
+            { typeof(AuthorizationException), "AuthorizationExpired" },
+            { typeof(ValueTruncationException), "ValueTruncationError" },
+            { typeof(ValueOverflowException), "ValueOverflowError" },
+            { typeof(FatalDiscoveryException), "FatalDiscoveryError" },
+            { typeof(ResultConsumedException), "ResultConsumedError" },
+            { typeof(TransactionNestingException), "TransactionNestingException" },
+            { typeof(TokenExpiredException), "ClientError" },
+            { typeof(ConnectionReadTimeoutException), "ConnectionReadTimeoutError" },
+            { typeof(InvalidBookmarkException), "InvalidBookmarkError" },
+            { typeof(TransactionClosedException), "ClientError" },
+            { typeof(NotSupportedException), "NotSupportedException" },
+            { typeof(ArgumentException), "ArgumentError" },
+            { typeof(InvalidBookmarkMixtureException), "InvalidBookmarkMixtureError" },
+            { typeof(ArgumentErrorException), "ArgumentError" },
+            { typeof(TypeException), "TypeError" },
+            { typeof(ForbiddenException), "ForbiddenError" },
+            { typeof(UnknownSecurityException), "OtherSecurityException" }
         };
 
 

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/TestBlackList.cs

@@ -116,9 +116,7 @@ static class TestBlackList
             ("test_temporal_types.TestDataTypes.test_date_time_cypher_created_tz_id",
                 "No Antarctica/Troll mapping available."),
             ("test_temporal_types.TestDataTypes.test_should_echo_all_timezone_ids",
-                "EST/HST/MST not supported."),
-
-            (".test_should_fail_with_invalid_routing_request", "Needs to adjust fast failing discovery errors.")
+                "EST/HST/MST not supported.")
         };
 
 		public static bool FindTest(string testName, out string reason)

Neo4j.Driver/Neo4j.Driver/Internal/Extensions/ErrorExtensions.cs

@@ -55,6 +55,27 @@ public static Neo4jException ParseServerException(string code, string message)
 					{
 						error = new InvalidBookmarkException(message);
 					}
+                    else if (InvalidBookmarkMixtureException.IsInvalidBookmarkMixtureException(code))
+                    {
+                        error = new InvalidBookmarkMixtureException(message);
+                    }
+                    else if (ArgumentErrorException.IsArgumentErrorException(code))
+                    {
+                        error = new ArgumentErrorException(message);
+                    }
+                    else if (TypeException.IsTypeException(code))
+                    {
+                        error = new TypeException(message);
+                    }
+                    else if (ForbiddenException.IsForbiddenException(code))
+                    {
+                        error = new ForbiddenException(message);
+                    }
+                    // this one needs to come after it has checked all other possibilities
+                    else if (UnknownSecurityException.IsUnknownSecurityException(code))
+                    {
+                        return new UnknownSecurityException(message, code);
+                    }
                     else
                     {
                         error = new ClientException(code, message);
@@ -127,4 +148,4 @@ public static ResultConsumedException NewResultConsumedException()
                 "or the query runner where the result is created has already been closed.");
         }
     }
-}
+}

Neo4j.Driver/Neo4j.Driver/Internal/Routing/RoutingTableManager.cs

@@ -291,31 +291,49 @@ await _discovery.DiscoverAsync(conn, database, impersonatedUser, bookmarks)
                             router, database);
                     }
                 }
-                catch (AuthorizationException e)
+                catch (Exception ex)
                 {
-                    _logger?.Warn(e, "Failed to update routing table from server '{0}' for database '{1}'.", router, database);
-                }
-                catch (SecurityException e)
-                {
-                    _logger?.Error(e,
-                        "Failed to update routing table from server '{0}' for database '{1}' because of a security exception.",
-                        router, database);
-                    throw;
-                }
-                catch (ClientException e)
-                {
-                    _logger?.Error(e,
-                        "Failed to update routing table from server '{0}' for database '{1}' because of a client exception.",
-                        router, database);
-                    throw;
-                }
-                catch (Exception e)
-                {
-                    _logger?.Warn(e, "Failed to update routing table from server '{0}' for database '{1}'.", router, database);
+                    var failfast = IsFailFastException(ex);
+                    var logMsg = "Failed to update routing table from server '{0}' for database '{1}'.";
+                    if (ex is Neo4jException ne)
+                    {
+                        logMsg += " Error code: '{2}'";
+                        var code = ne.Code;
+                        if (failfast)
+                        {
+                            _logger?.Error(ex, logMsg, router, database, code);
+                        }
+                        else
+                        {
+                            _logger?.Warn(ex, logMsg, router, database, code);
+                        }
+                    }
+                    else
+                    {
+                        _logger?.Warn(ex, logMsg, router, database);
+                    }
+
+                    if (failfast)
+                    {
+                        throw;
+                    }
                 }
             }
 
             return null;
         }
+
+        private static bool IsFailFastException(Exception ex)
+        {
+            return ex
+                is FatalDiscoveryException // Neo.ClientError.Database.DatabaseNotFound
+                or InvalidBookmarkException // Neo.ClientError.Transaction.InvalidBookmark
+                or InvalidBookmarkMixtureException // Neo.ClientError.Transaction.InvalidBookmarkMixture
+                or ArgumentErrorException // Neo.ClientError.Statement.ArgumentError
+                or ProtocolException // Neo.ClientError.Request.Invalid and (special to .NET driver) Neo.ClientError.Request.InvalidFormat
+                or TypeException // Neo.ClientError.Statement.TypeError
+                or SecurityException // Neo.ClientError.Security.*
+                and not AuthorizationException; // except Neo.ClientError.Security.AuthorizationExpired
+        }
     }
-}
+}

Neo4j.Driver/Neo4j.Driver/Neo4jException.cs

@@ -426,7 +426,7 @@ public AuthorizationException(string message) : base(ErrorCode, message)
     /// </summary>
     public class TokenExpiredException : SecurityException 
     {
-        private const string ErrorCode = "Neo.ClientError.Security.TokenExpiredException";
+        private const string ErrorCode = "Neo.ClientError.Security.TokenExpired";
 
         internal static bool IsTokenExpiredError(string code)
         {
@@ -464,6 +464,117 @@ public InvalidBookmarkException(string message) : base(ErrorCode, message)
         }
     }
 
+    /// <summary>
+    /// The provided bookmark is invalid. To recover from this a new session needs to be created.
+    /// </summary>
+    public class InvalidBookmarkMixtureException : ClientException
+    {
+        private const string ErrorCode = "Neo.ClientError.Transaction.InvalidBookmarkMixture";
+
+        /// <summary>
+        /// Create a new <see cref="InvalidBookmarkMixtureException" /> with an error message.
+        /// </summary>
+        /// <param name="message">The error message.</param>
+        public InvalidBookmarkMixtureException(string message) : base(ErrorCode, message)
+        {
+        }
+
+        internal static bool IsInvalidBookmarkMixtureException(string code)
+        {
+            return string.Equals(code, ErrorCode);
+        }
+    }
+
+    /// <summary>
+    /// A generic argument error has occurred. To recover from this a new session needs to be created.
+    /// </summary>
+    [DataContract]
+    public class ArgumentErrorException : ClientException
+    {
+        private const string ErrorCode = "Neo.ClientError.Statement.ArgumentError";
+
+        /// <summary>
+        /// Create a new <see cref="ArgumentErrorException" /> with an error message.
+        /// </summary>
+        /// <param name="message">The error message.</param>
+        public ArgumentErrorException(string message) : base(ErrorCode, message)
+        {
+        }
+
+        internal static bool IsArgumentErrorException(string code)
+        {
+            return string.Equals(code, ErrorCode);
+        }
+    }
+
+    /// <summary>
+    /// An error occurred related to data typing.
+    /// </summary>
+    [DataContract]
+    public class TypeException : ClientException
+    {
+        private const string ErrorCode = "Neo.ClientError.Statement.TypeError";
+
+        /// <summary>
+        /// Create a new <see cref="TypeException" /> with an error message.
+        /// </summary>
+        /// <param name="message">The error message.</param>
+        public TypeException(string message) : base(ErrorCode, message)
+        {
+        }
+
+        internal static bool IsTypeException(string code)
+        {
+            return string.Equals(code, ErrorCode);
+        }
+    }
+
+    /// <summary>
+    /// This operation is forbidden.
+    /// </summary>
+    [DataContract]
+    public class ForbiddenException : SecurityException
+    {
+        private const string ErrorCode = "Neo.ClientError.Security.Forbidden";
+
+        /// <summary>
+        /// Create a new <see cref="ForbiddenException" /> with an error message.
+        /// </summary>
+        /// <param name="message">The error message.</param>
+        public ForbiddenException(string message) : base(ErrorCode, message)
+        {
+        }
+
+        internal static bool IsForbiddenException(string code)
+        {
+            return string.Equals(code, ErrorCode);
+        }
+    }
+
+    /// <summary>
+    /// An unknown security error occurred.
+    /// </summary>
+    [DataContract]
+    public class UnknownSecurityException : SecurityException
+    {
+        private const string ErrorCodePrefix= "Neo.ClientError.Security.";
+
+        /// <summary>
+        /// Create a new <see cref="UnknownSecurityException" /> with an error message.
+        /// </summary>
+        /// <param name="message">The error message.</param>
+        /// <param name="code">The error code.</param>
+        public UnknownSecurityException(string message, string code) : base($"{ErrorCodePrefix}*", message)
+        {
+            Code = code;
+        }
+
+        internal static bool IsUnknownSecurityException(string code)
+        {
+            return code.StartsWith(ErrorCodePrefix);
+        }
+    }
+
     /// <summary>
     /// A value retrieved from the database needs to be truncated for this conversion to work, and will
     /// cause working with a modified data.