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

Better names and higher granularity for errors #519

Open
wprzytula opened this issue Aug 18, 2022 · 4 comments
Open

Better names and higher granularity for errors #519

wprzytula opened this issue Aug 18, 2022 · 4 comments
Assignees
Labels
API-breaking This might introduce incompatible API changes API-stability Part of the effort to stabilize the API cpp-rust-driver-p0 Functionality required by cpp-rust-driver
Milestone

Comments

@wprzytula
Copy link
Collaborator

Problem statement

There are quite a few places in driver's code where the errors' names and/or messages are inappropriate. Unjustified ProtocolError's usage is prevalent. We lack an error for faults triggered by user's logic, such as unwise custom RetryPolicy being used.

Proposed solution

Add more error variants (mainly for QueryError), fix the usaged to make names and messages appropriate. Stop abusing ProtocolError when no guilt is on received messages.

@piodul piodul added the API-breaking This might introduce incompatible API changes label Sep 22, 2022
@piodul piodul added this to the 0.9.0 milestone Mar 24, 2023
@wprzytula wprzytula self-assigned this Apr 12, 2023
@roydahan roydahan modified the milestones: 1.0.0, 0.12.0 Nov 12, 2023
@avelanarius avelanarius modified the milestones: 0.12.0, 0.13.0 Jan 15, 2024
@avelanarius avelanarius modified the milestones: 0.13.0, 0.14.0 Apr 30, 2024
@roydahan
Copy link
Collaborator

roydahan commented Jun 3, 2024

What other drivers do for it and the question is if we need to conform into a practice that all drivers agreed upon?

@wprzytula
Copy link
Collaborator Author

What other drivers do for it and the question is if we need to conform into a practice that all drivers agreed upon?

With drivers, error handling depends strongly on the language used.

  • Java driver uses exceptions (I suppose Python too);
  • gocql uses err type together with various textual error messages;
  • Cpp driver returns error code without any additional context;
  • Rust driver has a hierarchy of enums that builds up a tree of error types; this could be described in some document (as I asked @muzarski for) and discussed about. In Rust driver, we want strongly typed errors to be able to pattern match on them.

What this issue is about is not making error handling in Rust driver "conformant" to other drivers (because there's no such convention whatsoever), yet it is about changes to the error types hierarchy to make it more reasonable.

@roydahan
Copy link
Collaborator

roydahan commented Jul 1, 2024

@muzarski / @wprzytula what are the minimal set of errors we would like to handle for this for the 0.14.0 milestone or in general as the "API breaking" item.
I doubt we need to handle all cases right now.

In any case, if easy let's divide them to issues under an umbrella issue and target them to milestones.

@wprzytula
Copy link
Collaborator Author

In the Rust world, all alterations to current errors must be, unfortunately, considered API-breaking. Only after the introduced changes will the error framework be well-suited for changes in a non-API-breaking way. That means that we must do that before 1.0.
But we can, of course, divide the changes into smaller parts and attach to multiple milestones.

@wprzytula wprzytula added this to the 0.16.0 milestone Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-breaking This might introduce incompatible API changes API-stability Part of the effort to stabilize the API cpp-rust-driver-p0 Functionality required by cpp-rust-driver
Projects
None yet
Development

No branches or pull requests

5 participants