errors: Make Rust errors conversion to `CassError` more appropriate #352

wprzytula · 2025-07-28T18:13:28Z

Fixes: #133

This PR:

Merges execution_error.rs into cass_error.rs, as both contain related functionality that should live together.
Adjusts conversion of multiple Rust Driver's error variants to CassError, by choosing a better-fitting CassError variant. I tried to provide clear explanation why I believe the new variant is a better fit.

Pre-review checklist

I have split my patch into logically separate commits.
All commit messages clearly explain what they change and why.
PR description sums up the changes and reasons why they should be introduced.
~~[ ] I have implemented Rust unit tests for the features/changes introduced.~~
~~[ ] I have enabled appropriate tests in Makefile in {SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER.~~
I added appropriate Fixes: annotations to PR description.

The `CassError`-related items were arbitrarily split between two modules: `cass_error.rs` and `execution_error.rs`. As this division is artificial, `execution_error.rs` contents have been moved to `cass_error.rs`.

`CASS_ERROR_LIB_INVALID_DATA` is returned in CPP Driver in cases where the CQL values to be serialized/deserialized are not valid. OTOH, `CASS_ERROR_LIB_UNEXPECTED_RESPONSE` is used when the message is valid in terms of binary protocol, but the driver does not expect it as a response in the current context. I decided that errors in frame deserialization warrant `INVALID_DATA` more then `UNEXPECTED_RESPONSE`.

`CassError` is split into categories, by the issuer of the error: - `CASS_ERROR_LIB_*` for errors from the driver, - `CASS_ERROR_SERVER_*` for errors from the server, - `CASS_ERROR_SSL_*` for errors from the SSL library. The `CASS_ERROR_SERVER_PROTOCOL_ERROR` was used for errors that are not from the server, but from the driver, which is not correct. Among other things, it confuses users of the library, who expect SERVER errors to have come from the server, but they are actually from the driver. To fix this, such usages of the error were replaced with `CASS_ERROR_LIB_INVALID_STATE`.

I believe that the `BadQuery` enum variants that were previously returning `CASS_ERROR_LAST_ENTRY` (as an obvious placeholder) should instead return `CASS_ERROR_LIB_MESSAGE_ENCODE`. This change is made to ensure that the error handling is more specific and meaningful, particularly for cases where the query is malformed or exceeds certain limits. I understand `CASS_ERROR_LIB_MESSAGE_ENCODE` as a general error for issues around serialization.

`MetadataFetchErrorKind::NextRowError` was previously always converted to `CassError::CASS_ERROR_LIB_UNEXPECTED_RESPONSE`, which is not appropriate for all cases. This commit refactors the conversion to handle different cases of `NextRowError` more appropriately, depending on the specific error encountered.

This seems to be the most natural mapping for the `BadKeyspaceName` error.

INVALID_DATA seems to be an appropriate error code for serialization errors.

Copilot

Pull Request Overview

This PR merges the functionality from execution_error.rs into cass_error.rs and improves the mapping of Rust driver error variants to more appropriate CassError variants. The refactoring consolidates related error handling code while providing better error categorization.

Key changes:

Consolidates error handling by merging execution_error.rs into cass_error.rs
Updates error mappings to use more semantically appropriate CassError variants
Adjusts import statements across affected files to reflect the module reorganization

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
scylla-rust-wrapper/src/execution_error.rs	File deleted - all content moved to cass_error.rs
scylla-rust-wrapper/src/cass_error.rs	Merged execution_error.rs content and updated error mappings
scylla-rust-wrapper/src/lib.rs	Removed execution_error module declaration
scylla-rust-wrapper/src/query_result.rs	Updated imports to use CassErrorResult from cass_error
scylla-rust-wrapper/src/future.rs	Consolidated cass_error imports
scylla-rust-wrapper/src/api.rs	Updated public API exports to use cass_error module

Copilot · 2025-07-28T18:14:11Z

scylla-rust-wrapper/src/cass_error.rs

-use scylla::errors::*;
+use crate::argconv::*;
+use crate::cass_types::CassConsistency;
+use crate::misc::CassWriteType;


The import references crate::misc::CassWriteType but the From<&WriteType> implementation uses CassWriteType variants like CASS_WRITE_TYPE_SIMPLE. This suggests CassWriteType should be imported from crate::cass_error_types (where other error types are defined) rather than crate::misc.

Suggested change

use crate::misc::CassWriteType;

use crate::cass_error_types::CassWriteType;

Copilot · 2025-07-28T18:14:12Z

scylla-rust-wrapper/src/cass_error.rs

@@ -181,8 +187,39 @@ impl ToCassError for MetadataError {
                }
                MetadataFetchErrorKind::PrepareError(e) => e.to_cass_error(),
                MetadataFetchErrorKind::SerializationError(e) => e.to_cass_error(),
-                MetadataFetchErrorKind::NextRowError(_) => {
-                    CassError::CASS_ERROR_LIB_UNEXPECTED_RESPONSE
+                MetadataFetchErrorKind::NextRowError(e) => {


[nitpick] The nested match expression for NextRowError is deeply nested (4 levels) and contains complex logic. Consider extracting this into a separate helper function to improve readability and maintainability.

Lorak-mmk · 2025-08-03T12:27:22Z

scylla-rust-wrapper/src/cass_error.rs

+            BadQuery::ValuesTooLongForKey(_usize, _usize2) => {
+                CassError::CASS_ERROR_LIB_MESSAGE_ENCODE
+            }
+            BadQuery::PartitionKeyExtraction => CassError::CASS_ERROR_LIB_MESSAGE_ENCODE,
            BadQuery::SerializationError(e) => e.to_cass_error(),
-            BadQuery::TooManyQueriesInBatchStatement(_) => CassError::CASS_ERROR_LAST_ENTRY,
+            BadQuery::TooManyQueriesInBatchStatement(_) => CassError::CASS_ERROR_LIB_MESSAGE_ENCODE,
            // BadQuery is non_exhaustive
-            // For now, since all other variants return LAST_ENTRY,
-            // let's do it here as well.
            _ => CassError::CASS_ERROR_LAST_ENTRY,


Here we need to handle non-exhaustiveness of error types from the driver.
Maybe we could utilize unstable non_exhaustive_omitted_patterns_lint from rustc?

If you're suggesting a switch to a nightly compiler, I'm against. From my experience with TockOS, working with nightly is a pain which I'd definitely prefer to avoid.

From my experience with TockOS, working with nightly is a pain

Isn't it more precise to say "working with specific old version of nightly is a pain"?

I don't see how working with newer versions of nightly is not a pain.

Could you explain why working with nightly is a pain?

Consider the following:

nightly versions are often (nightly) changing;

you need to keep CI in sync with your local version, as well as local version of all contributors;

Is your idea to pin a specific nightly version, like a specific night that all contributors and CI shall use, and bump it once upon a time?

Isn't nightly more prone to bugs and unstable behaviour? Can we afford this in a production-critical software?

Are nightly features documented well-enough to rely on them? Won't it be more pain than gain when they change and we need to adjust the code frequently?

Lorak-mmk · 2025-08-03T12:38:03Z

scylla-rust-wrapper/src/cass_error.rs

            // It means that our custom `UnknownNamedParameterError` was returned.
            CassError::CASS_ERROR_LIB_NAME_DOES_NOT_EXIST
        } else {
-            CassError::CASS_ERROR_LAST_ENTRY
+            CassError::CASS_ERROR_LIB_INVALID_DATA
        }
    }


Commit: "cass_error: SerializationError -> INVALID_DATA "

When is LIB_INVALID_DATA appropriate vs LIB_MESSAGE_ENCODE?

My vague understanding of it is that when the particular CQL value is of bad type or corrupted, then INVALID_DATA is appropriate. MESSAGE_ENCODE is more about CQL protocol limitations, such as too big number of values, too many statements in a batch etc.

wprzytula added 7 commits July 27, 2025 17:20

tree: merge execution_error.rs into cass_error.rs

17f662f

The `CassError`-related items were arbitrarily split between two modules: `cass_error.rs` and `execution_error.rs`. As this division is artificial, `execution_error.rs` contents have been moved to `cass_error.rs`.

cass_error: BadKeyspaceName -> UNABLE_TO_SET_KEYSPACE

fd0131a

This seems to be the most natural mapping for the `BadKeyspaceName` error.

cass_error: SerializationError -> INVALID_DATA

3ab3159

INVALID_DATA seems to be an appropriate error code for serialization errors.

wprzytula requested review from Lorak-mmk and Copilot and removed request for Lorak-mmk July 28, 2025 18:13

wprzytula self-assigned this Jul 28, 2025

wprzytula requested a review from Lorak-mmk July 28, 2025 18:13

wprzytula added this to the 1.0.0 milestone Jul 28, 2025

Copilot AI reviewed Jul 28, 2025

View reviewed changes

Lorak-mmk reviewed Aug 3, 2025

View reviewed changes

Lorak-mmk approved these changes Aug 4, 2025

View reviewed changes

wprzytula merged commit 59ba8a1 into scylladb:master Aug 4, 2025
9 checks passed

wprzytula deleted the cass_error-refactors branch August 4, 2025 13:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

errors: Make Rust errors conversion to `CassError` more appropriate #352

errors: Make Rust errors conversion to `CassError` more appropriate #352

Uh oh!

wprzytula commented Jul 28, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 28, 2025

Uh oh!

Copilot AI Jul 28, 2025

Uh oh!

Lorak-mmk Aug 3, 2025

Uh oh!

wprzytula Aug 3, 2025

Uh oh!

Lorak-mmk Aug 3, 2025

Uh oh!

wprzytula Aug 4, 2025

Uh oh!

Lorak-mmk Aug 4, 2025

Uh oh!

wprzytula Aug 4, 2025

Uh oh!

Lorak-mmk Aug 3, 2025

Uh oh!

wprzytula Aug 3, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

	use crate::misc::CassWriteType;
	use crate::cass_error_types::CassWriteType;

errors: Make Rust errors conversion to CassError more appropriate #352

errors: Make Rust errors conversion to CassError more appropriate #352

Uh oh!

Conversation

wprzytula commented Jul 28, 2025

Pre-review checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Lorak-mmk Aug 3, 2025

Choose a reason for hiding this comment

Uh oh!

wprzytula Aug 3, 2025

Choose a reason for hiding this comment

Uh oh!

Lorak-mmk Aug 3, 2025

Choose a reason for hiding this comment

Uh oh!

wprzytula Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

Lorak-mmk Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

wprzytula Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

Lorak-mmk Aug 3, 2025

Choose a reason for hiding this comment

Uh oh!

wprzytula Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

errors: Make Rust errors conversion to `CassError` more appropriate #352

errors: Make Rust errors conversion to `CassError` more appropriate #352

wprzytula Aug 3, 2025 •

edited

Loading