Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make more error codes fail fast during the rediscovery #536

Merged

Conversation

bigmontz
Copy link
Contributor

@bigmontz bigmontz commented Nov 23, 2022

(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: this happens on failed attempts of impersonation (user or permissions missing)
  • Neo.ClientError.Request.Invalid: this happens when trying things like impersonating an integer
  • Neo.ClientError.Statement.TypeError: this happens when trying things like set a database as an integer in bolt implementation which doesn't support ROUTE

(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 two 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
This error occurs when try to use a number as database name in protocol versions which doesn't suport ROUTE message.
@bigmontz bigmontz marked this pull request as ready for review November 24, 2022 09:19
@robsdedude robsdedude self-requested a review November 24, 2022 11:16
Copy link
Contributor

@robsdedude robsdedude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests/stub/routing/test_routing_v5x0.py Outdated Show resolved Hide resolved
tests/stub/routing/test_routing_v5x0.py Outdated Show resolved Hide resolved
tests/stub/routing/test_routing_v5x0.py Outdated Show resolved Hide resolved
Co-authored-by: Robsdedude <dev@rouvenbauer.de>
@robsdedude robsdedude merged commit 8c237fd into neo4j-drivers:5.0 Dec 6, 2022
@bigmontz bigmontz deleted the 5.x-fast-failing-in-more-errors branch December 6, 2022 17:05
injectives added a commit to injectives/neo4j-java-driver that referenced this pull request Dec 14, 2022
injectives added a commit to neo4j/neo4j-java-driver that referenced this pull request Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants