Make more error codes fail fast during the rediscovery (#536)

bigmontz · robsdedude · web-flow · commit 8c237fd992c6 · 2022-12-06T12:37:05.000+01:00
(Most) drivers will try really hard to fetch a valid RT. This entails going to the next know router if the previous one failed. In this process many errors that are considered unrecoverable in the transaction context are considered retryable.
However, a hand full of errors should not be retried to improve user experience. Namely those that are caused by malformed user-input. Instead of asking all routers if they can make sense of it and ultimately failing with a generic routing update error, it's better to fail fast and give the raw error of the server to the user.

Currently, these errors are
  - `Neo.ClientError.Database.DatabaseNotFound`
 - `Neo.ClientError.Transaction.InvalidBookmark`
 - `Neo.ClientError.Transaction.InvalidBookmarkMixture`

Added three more:
 - `Neo.ClientError.Statement.ArgumentError`, which happens on failed attempts of impersonation (user or permissions missing)
 - `Neo.ClientError.Request.Invalid`, which happens when trying  things like impersonating an integer
 - `Neo.ClientError.Statement.TypeError`, which happens when try to use a number as database name in protocol versions which don't support the `ROUTE` message.

Co-authored-by: Rouven Bauer &lt;rouven.bauer@neo4j.com&gt;
diff --git a/tests/stub/routing/scripts/v3/router_yielding_argument_error.script b/tests/stub/routing/scripts/v3/router_yielding_argument_error.script
@@ -0,0 +1,13 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007"#EXTR_HELLO_ROUTING_PROPS#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": #ROUTINGCTX#} {"[mode]": "r"}
+S: FAILURE {"code": "Neo.ClientError.Statement.ArgumentError", "message": "Kaboooom."}
+{?
+    C: PULL_ALL
+    S: IGNORED
+?}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v3/router_yielding_invalid_request.script b/tests/stub/routing/scripts/v3/router_yielding_invalid_request.script
@@ -0,0 +1,13 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007"#EXTR_HELLO_ROUTING_PROPS#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": #ROUTINGCTX#} {"[mode]": "r"}
+S: FAILURE {"code": "Neo.ClientError.Request.Invalid", "message": "Kaboooom."}
+{?
+    C: PULL_ALL
+    S: IGNORED
+?}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v3/router_yielding_type_error.script b/tests/stub/routing/scripts/v3/router_yielding_type_error.script
@@ -0,0 +1,13 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007"#EXTR_HELLO_ROUTING_PROPS#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": #ROUTINGCTX#} {"[mode]": "r"}
+S: FAILURE {"code": "Neo.ClientError.Statement.TypeError", "message": "Kaboooom."}
+{?
+    C: PULL_ALL
+    S: IGNORED
+?}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x1/router_yielding_argument_error.script b/tests/stub/routing/scripts/v4x1/router_yielding_argument_error.script
@@ -0,0 +1,13 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: RUN "CALL dbms.routing.getRoutingTable($context, $database)" {"context": #ROUTINGCTX#, "[database]": "*"} {"[mode]": "r", "db": "system", "bookmarks": ["foobar"]}
+S: FAILURE {"code": "Neo.ClientError.Statement.ArgumentError", "message": "Kaboooom."}
+{?
+    C: PULL {"n": {"Z": "*"}}
+    S: IGNORED
+?}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x1/router_yielding_invalid_request.script b/tests/stub/routing/scripts/v4x1/router_yielding_invalid_request.script
@@ -0,0 +1,13 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: RUN "CALL dbms.routing.getRoutingTable($context, $database)" {"context": #ROUTINGCTX#, "[database]": "*"} {"[mode]": "r", "db": "system", "bookmarks": ["foobar"]}
+S: FAILURE {"code": "Neo.ClientError.Request.Invalid", "message": "Kaboooom."}
+{?
+    C: PULL {"n": {"Z": "*"}}
+    S: IGNORED
+?}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x1/router_yielding_type_error.script b/tests/stub/routing/scripts/v4x1/router_yielding_type_error.script
@@ -0,0 +1,13 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: RUN "CALL dbms.routing.getRoutingTable($context, $database)" {"context": #ROUTINGCTX#, "[database]": "*"} {"[mode]": "r", "db": "system", "bookmarks": ["foobar"]}
+S: FAILURE {"code": "Neo.ClientError.Statement.TypeError", "message": "Kaboooom."}
+{?
+    C: PULL {"n": {"Z": "*"}}
+    S: IGNORED
+?}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x3/router_yielding_argument_error.script b/tests/stub/routing/scripts/v4x3/router_yielding_argument_error.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX# , "[patch_bolt]": "*"}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] "*"
+S: FAILURE {"code": "Neo.ClientError.Statement.ArgumentError", "message": "Kaboooom!"}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x3/router_yielding_invalid_request.script b/tests/stub/routing/scripts/v4x3/router_yielding_invalid_request.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX# , "[patch_bolt]": "*"}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] "*"
+S: FAILURE {"code": "Neo.ClientError.Request.Invalid", "message": "Kaboooom!"}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x3/router_yielding_type_error.script b/tests/stub/routing/scripts/v4x3/router_yielding_type_error.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX# , "[patch_bolt]": "*"}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] "*"
+S: FAILURE {"code": "Neo.ClientError.Statement.TypeError", "message": "Kaboooom!"}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x4/router_yielding_argument_error.script b/tests/stub/routing/scripts/v4x4/router_yielding_argument_error.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#, "[patch_bolt]": "*"}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] {"[db]": "*"}
+S: FAILURE {"code": "Neo.ClientError.Statement.ArgumentError", "message": "Kaboooom."}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x4/router_yielding_invalid_request.script b/tests/stub/routing/scripts/v4x4/router_yielding_invalid_request.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#, "[patch_bolt]": "*"}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] {"[db]": "*"}
+S: FAILURE {"code": "Neo.ClientError.Request.Invalid", "message": "Kaboooom."}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v4x4/router_yielding_type_error.script b/tests/stub/routing/scripts/v4x4/router_yielding_type_error.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#, "[patch_bolt]": "*"}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] {"[db]": "*"}
+S: FAILURE {"code": "Neo.ClientError.Statement.TypeError", "message": "Kaboooom."}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v5x0/router_yielding_argument_error.script b/tests/stub/routing/scripts/v5x0/router_yielding_argument_error.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] {"[db]": "*"}
+S: FAILURE {"code": "Neo.ClientError.Statement.ArgumentError", "message": "Kaboooom."}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v5x0/router_yielding_invalid_request.script b/tests/stub/routing/scripts/v5x0/router_yielding_invalid_request.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] {"[db]": "*"}
+S: FAILURE {"code": "Neo.ClientError.Request.Invalid", "message": "Kaboooom."}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/scripts/v5x0/router_yielding_type_error.script b/tests/stub/routing/scripts/v5x0/router_yielding_type_error.script
@@ -0,0 +1,9 @@
+!: BOLT #VERSION#
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "007", "routing": #ROUTINGCTX#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+*: RESET
+C: ROUTE #ROUTINGCTX# ["foobar"] {"[db]": "*"}
+S: FAILURE {"code": "Neo.ClientError.Statement.TypeError", "message": "Kaboooom."}
+*: RESET
+?: GOODBYE
diff --git a/tests/stub/routing/test_routing_v5x0.py b/tests/stub/routing/test_routing_v5x0.py
@@ -2306,6 +2306,36 @@ def test_should_fail_with_routing_failure_on_any_security_discovery_failure(  #
             exc.exception.code
         )
 
+    def test_should_fail_with_invalid_route_statement_argument(self):
+        exc = self._test_fast_fail_discover(
+            "router_yielding_argument_error.script",
+        )
+
+        self.assertEqual(
+            "Neo.ClientError.Statement.ArgumentError",
+            exc.exception.code
+        )
+
+    def test_should_fail_with_invalid_routing_request(self):
+        exc = self._test_fast_fail_discover(
+            "router_yielding_invalid_request.script",
+        )
+
+        self.assertEqual(
+            "Neo.ClientError.Request.Invalid",
+            exc.exception.code
+        )
+
+    def test_should_fail_with_type_error_request(self):
+        exc = self._test_fast_fail_discover(
+            "router_yielding_type_error.script",
+        )
+
+        self.assertEqual(
+            "Neo.ClientError.Statement.TypeError",
+            exc.exception.code
+        )
+
     def test_should_read_successfully_from_reachable_db_after_trying_unreachable_db(  # noqa: E501
         self
     ):
