-
Notifications
You must be signed in to change notification settings - Fork 144
RUST-1406 Add type-specific errors to standard error type #555
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
Changes from all commits
6380979
a10b680
33a6a6f
950965c
d7e05d4
d368d4c
ec54947
a74e72e
3d8e887
54c7dbb
2176ad6
5b0057c
1ee3a41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,27 @@ | ||
mod decimal128; | ||
mod oid; | ||
mod uuid; | ||
mod value_access; | ||
|
||
use thiserror::Error; | ||
|
||
use crate::spec::ElementType; | ||
pub use decimal128::Decimal128ErrorKind; | ||
pub use oid::ObjectIdErrorKind; | ||
pub use uuid::UuidErrorKind; | ||
pub use value_access::ValueAccessErrorKind; | ||
|
||
pub type Result<T> = std::result::Result<T, Error>; | ||
|
||
/// An error that can occur in the `bson` crate. | ||
#[derive(Debug, Error)] | ||
#[derive(Clone, Debug, Error)] | ||
#[non_exhaustive] | ||
pub struct Error { | ||
/// The kind of error that occurred. | ||
pub kind: ErrorKind, | ||
|
||
/// An optional message describing the error. | ||
pub message: Option<String>, | ||
|
||
/// The document key associated with the error, if any. | ||
pub key: Option<String>, | ||
|
||
|
@@ -20,28 +31,69 @@ pub struct Error { | |
|
||
impl std::fmt::Display for Error { | ||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
write!(f, "BSON error")?; | ||
|
||
if let Some(key) = self.key.as_deref() { | ||
write!(f, "Error at key \"{key}\": ")?; | ||
write!(f, " at key \"{key}\"")?; | ||
} else if let Some(index) = self.index { | ||
write!(f, "Error at array index {index}: ")?; | ||
write!(f, " at array index {index}")?; | ||
} | ||
|
||
write!(f, "{}", self.kind) | ||
write!(f, ". Kind: {}", self.kind)?; | ||
if let Some(ref message) = self.message { | ||
write!(f, ". Message: {}", message)?; | ||
} | ||
write!(f, ".") | ||
} | ||
} | ||
|
||
/// The types of errors that can occur in the `bson` crate. | ||
#[derive(Debug, Error)] | ||
#[derive(Clone, Debug, Error)] | ||
#[non_exhaustive] | ||
pub enum ErrorKind { | ||
/// An error related to the [`Binary`](crate::Binary) type occurred. | ||
#[error("A Binary-related error occurred")] | ||
#[non_exhaustive] | ||
Binary {}, | ||
|
||
/// An error related to the [`DateTime`](crate::DateTime) type occurred. | ||
#[error("A DateTime-related error occurred")] | ||
#[non_exhaustive] | ||
DateTime {}, | ||
|
||
/// An error related to the [`Decimal128`](crate::Decimal128) type occurred. | ||
#[error("A Decimal128-related error occurred: {kind}")] | ||
#[non_exhaustive] | ||
Decimal128 { | ||
/// The kind of error that occurred. | ||
kind: Decimal128ErrorKind, | ||
}, | ||
|
||
/// Malformed BSON bytes were encountered. | ||
#[error("Malformed BSON: {message}")] | ||
#[error("Malformed BSON bytes")] | ||
#[non_exhaustive] | ||
MalformedValue { message: String }, | ||
MalformedBytes {}, | ||
|
||
/// An error related to the [`ObjectId`](crate::oid::ObjectId) type occurred. | ||
#[error("An ObjectId-related error occurred: {kind}")] | ||
#[non_exhaustive] | ||
ObjectId { | ||
/// The kind of error that occurred. | ||
kind: ObjectIdErrorKind, | ||
}, | ||
|
||
/// Invalid UTF-8 bytes were encountered. | ||
#[error("Invalid UTF-8")] | ||
Utf8Encoding, | ||
#[non_exhaustive] | ||
Utf8Encoding {}, | ||
|
||
/// An error related to the [`Uuid`](crate::uuid::Uuid) type occurred. | ||
#[error("A UUID-related error occurred: {kind}")] | ||
#[non_exhaustive] | ||
Uuid { | ||
/// The kind of error that occurred. | ||
kind: UuidErrorKind, | ||
}, | ||
|
||
/// An error occurred when attempting to access a value in a document. | ||
#[error("An error occurred when attempting to access a document value: {kind}")] | ||
|
@@ -52,8 +104,9 @@ pub enum ErrorKind { | |
}, | ||
|
||
/// A [`std::io::Error`] occurred. | ||
#[error("An IO error occurred: {0}")] | ||
Io(std::io::Error), | ||
#[error("An IO error occurred")] | ||
#[non_exhaustive] | ||
Io {}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. switched this over to a stringified version of the error - There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might want to carry the |
||
|
||
/// A wrapped deserialization error. | ||
/// TODO RUST-1406: collapse this | ||
|
@@ -68,13 +121,14 @@ impl From<ErrorKind> for Error { | |
kind, | ||
key: None, | ||
index: None, | ||
message: None, | ||
} | ||
} | ||
} | ||
|
||
impl From<std::io::Error> for Error { | ||
fn from(value: std::io::Error) -> Self { | ||
ErrorKind::Io(value).into() | ||
Error::from(ErrorKind::Io {}).with_message(value) | ||
} | ||
} | ||
|
||
|
@@ -85,31 +139,6 @@ impl From<crate::de::Error> for Error { | |
} | ||
} | ||
|
||
/// The types of errors that can occur when attempting to access a value in a document. | ||
#[derive(Debug, Error)] | ||
#[non_exhaustive] | ||
pub enum ValueAccessErrorKind { | ||
/// No value for the specified key was present in the document. | ||
#[error("The key was not present in the document")] | ||
NotPresent, | ||
|
||
/// The type of the value in the document did not match the requested type. | ||
#[error("Expected type {expected:?}, got type {actual:?}")] | ||
#[non_exhaustive] | ||
UnexpectedType { | ||
/// The actual type of the value. | ||
actual: ElementType, | ||
|
||
/// The expected type of the value. | ||
expected: ElementType, | ||
}, | ||
|
||
/// An error occurred when attempting to parse the document's BSON bytes. | ||
#[error("{message}")] | ||
#[non_exhaustive] | ||
InvalidBson { message: String }, | ||
} | ||
|
||
impl Error { | ||
pub(crate) fn with_key(mut self, key: impl Into<String>) -> Self { | ||
self.key = Some(key.into()); | ||
|
@@ -121,58 +150,25 @@ impl Error { | |
self | ||
} | ||
|
||
pub(crate) fn value_access_not_present() -> Self { | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::NotPresent, | ||
} | ||
.into() | ||
} | ||
|
||
pub(crate) fn value_access_unexpected_type(actual: ElementType, expected: ElementType) -> Self { | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::UnexpectedType { actual, expected }, | ||
} | ||
.into() | ||
} | ||
|
||
pub(crate) fn value_access_invalid_bson(message: String) -> Self { | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::InvalidBson { message }, | ||
} | ||
.into() | ||
pub(crate) fn with_message(mut self, message: impl ToString) -> Self { | ||
self.message = Some(message.to_string()); | ||
self | ||
} | ||
|
||
pub(crate) fn malformed_value(message: impl ToString) -> Self { | ||
ErrorKind::MalformedValue { | ||
message: message.to_string(), | ||
} | ||
.into() | ||
pub(crate) fn binary(message: impl ToString) -> Self { | ||
Self::from(ErrorKind::Binary {}).with_message(message) | ||
} | ||
|
||
#[cfg(test)] | ||
pub(crate) fn is_value_access_not_present(&self) -> bool { | ||
matches!( | ||
self.kind, | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::NotPresent, | ||
.. | ||
} | ||
) | ||
pub(crate) fn datetime(message: impl ToString) -> Self { | ||
Self::from(ErrorKind::DateTime {}).with_message(message) | ||
} | ||
|
||
#[cfg(test)] | ||
pub(crate) fn is_value_access_unexpected_type(&self) -> bool { | ||
matches!( | ||
self.kind, | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::UnexpectedType { .. }, | ||
.. | ||
} | ||
) | ||
pub(crate) fn malformed_bytes(message: impl ToString) -> Self { | ||
Self::from(ErrorKind::MalformedBytes {}).with_message(message) | ||
} | ||
|
||
#[cfg(all(test, feature = "serde"))] | ||
pub(crate) fn is_malformed_value(&self) -> bool { | ||
matches!(self.kind, ErrorKind::MalformedValue { .. },) | ||
pub(crate) fn is_malformed_bytes(&self) -> bool { | ||
matches!(self.kind, ErrorKind::MalformedBytes { .. },) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
use thiserror::Error as ThisError; | ||
|
||
use crate::error::{Error, ErrorKind}; | ||
|
||
/// The kinds of errors that can occur when working with the [`Decimal128`](crate::Decimal128) type. | ||
#[derive(Clone, Debug, ThisError)] | ||
#[non_exhaustive] | ||
pub enum Decimal128ErrorKind { | ||
/// Empty exponent. | ||
#[error("empty exponent")] | ||
#[non_exhaustive] | ||
EmptyExponent {}, | ||
|
||
/// Invalid exponent. | ||
#[error("invalid exponent")] | ||
#[non_exhaustive] | ||
InvalidExponent {}, | ||
|
||
/// Invalid coefficient. | ||
#[error("invalid coefficient")] | ||
#[non_exhaustive] | ||
InvalidCoefficient {}, | ||
|
||
/// Overflow. | ||
#[error("overflow")] | ||
#[non_exhaustive] | ||
Overflow {}, | ||
|
||
/// Underflow. | ||
#[error("underflow")] | ||
#[non_exhaustive] | ||
Underflow {}, | ||
|
||
/// Inexact rounding. | ||
#[error("inexact rounding")] | ||
#[non_exhaustive] | ||
InexactRounding {}, | ||
|
||
/// Unparseable. | ||
#[error("unparseable")] | ||
#[non_exhaustive] | ||
Unparseable {}, | ||
} | ||
|
||
impl Error { | ||
pub(crate) fn decimal128(kind: Decimal128ErrorKind) -> Self { | ||
ErrorKind::Decimal128 { kind }.into() | ||
} | ||
|
||
#[cfg(test)] | ||
pub(crate) fn is_decimal128_unparseable(&self) -> bool { | ||
matches!( | ||
self.kind, | ||
ErrorKind::Decimal128 { | ||
kind: Decimal128ErrorKind::Unparseable {}, | ||
} | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
use hex::FromHexError; | ||
use thiserror::Error as ThisError; | ||
|
||
use crate::error::{Error, ErrorKind}; | ||
|
||
/// The kinds of errors that can occur when working with the [`ObjectId`](crate::oid::ObjectId) | ||
/// type. | ||
#[derive(Clone, Debug, ThisError)] | ||
#[non_exhaustive] | ||
pub enum ObjectIdErrorKind { | ||
/// An invalid character was found in the provided hex string. Valid characters are: `0...9`, | ||
/// `a...f`, or `A...F`. | ||
#[error("invalid character '{c}' encountered at index {index}")] | ||
#[non_exhaustive] | ||
InvalidHexStringCharacter { | ||
/// The invalid character. | ||
c: char, | ||
|
||
/// The index at which the invalid character was encountered. | ||
index: usize, | ||
}, | ||
|
||
/// An `ObjectId` with an invalid length was encountered. | ||
#[error("invalid hex string length {length}")] | ||
#[non_exhaustive] | ||
InvalidHexStringLength { | ||
/// The length of the invalid hex string. | ||
length: usize, | ||
}, | ||
} | ||
|
||
impl Error { | ||
// This method is not a From implementation so that it is not part of the public API. | ||
pub(crate) fn from_hex_error(error: FromHexError, length: usize) -> Self { | ||
let kind = match error { | ||
FromHexError::InvalidHexCharacter { c, index } => { | ||
ObjectIdErrorKind::InvalidHexStringCharacter { c, index } | ||
} | ||
FromHexError::InvalidStringLength | FromHexError::OddLength => { | ||
ObjectIdErrorKind::InvalidHexStringLength { length } | ||
} | ||
}; | ||
ErrorKind::ObjectId { kind }.into() | ||
} | ||
|
||
pub(crate) fn oid_invalid_length(length: usize) -> Self { | ||
ErrorKind::ObjectId { | ||
kind: ObjectIdErrorKind::InvalidHexStringLength { length }, | ||
} | ||
.into() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
use thiserror::Error as ThisError; | ||
|
||
use crate::{ | ||
error::{Error, ErrorKind}, | ||
spec::BinarySubtype, | ||
UuidRepresentation, | ||
}; | ||
|
||
/// The kinds of errors that can occur when working with the [`Uuid`](crate::uuid::Uuid) type. | ||
#[derive(Clone, Debug, ThisError)] | ||
#[non_exhaustive] | ||
pub enum UuidErrorKind { | ||
/// An invalid string was used to construct a UUID. | ||
#[error("invalid UUID string")] | ||
#[non_exhaustive] | ||
InvalidString {}, | ||
|
||
/// The requested `UuidRepresentation` does not match the binary subtype of a `Binary` | ||
/// value. | ||
#[error( | ||
"expected binary subtype {expected_binary_subtype:?} for representation \ | ||
{requested_representation:?}, got {actual_binary_subtype:?}" | ||
)] | ||
#[non_exhaustive] | ||
RepresentationMismatch { | ||
/// The subtype that was expected given the requested representation. | ||
expected_binary_subtype: BinarySubtype, | ||
|
||
/// The actual subtype of the binary value. | ||
actual_binary_subtype: BinarySubtype, | ||
|
||
/// The requested representation. | ||
requested_representation: UuidRepresentation, | ||
}, | ||
|
||
/// An invalid length of bytes was used to construct a UUID value. | ||
#[error("expected length of 16 bytes, got {length}")] | ||
#[non_exhaustive] | ||
InvalidLength { | ||
/// The actual length of the data. | ||
length: usize, | ||
}, | ||
} | ||
|
||
impl Error { | ||
pub(crate) fn invalid_uuid_string(message: impl ToString) -> Self { | ||
Self::from(ErrorKind::Uuid { | ||
kind: UuidErrorKind::InvalidString {}, | ||
}) | ||
.with_message(message) | ||
} | ||
|
||
pub(crate) fn uuid_representation_mismatch( | ||
requested_representation: UuidRepresentation, | ||
actual_binary_subtype: BinarySubtype, | ||
expected_binary_subtype: BinarySubtype, | ||
) -> Self { | ||
ErrorKind::Uuid { | ||
kind: UuidErrorKind::RepresentationMismatch { | ||
expected_binary_subtype, | ||
actual_binary_subtype, | ||
requested_representation, | ||
}, | ||
} | ||
.into() | ||
} | ||
|
||
pub(crate) fn invalid_uuid_length(length: usize) -> Self { | ||
ErrorKind::Uuid { | ||
kind: UuidErrorKind::InvalidLength { length }, | ||
} | ||
.into() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
use thiserror::Error as ThisError; | ||
|
||
use crate::{ | ||
error::{Error, ErrorKind}, | ||
spec::ElementType, | ||
}; | ||
|
||
/// The types of errors that can occur when attempting to access a value in a document. | ||
#[derive(Clone, Debug, ThisError)] | ||
#[non_exhaustive] | ||
pub enum ValueAccessErrorKind { | ||
/// No value for the specified key was present in the document. | ||
#[error("the key was not present in the document")] | ||
#[non_exhaustive] | ||
NotPresent {}, | ||
|
||
/// The type of the value in the document did not match the requested type. | ||
#[error("expected type {expected:?}, got type {actual:?}")] | ||
#[non_exhaustive] | ||
UnexpectedType { | ||
/// The actual type of the value. | ||
actual: ElementType, | ||
|
||
/// The expected type of the value. | ||
expected: ElementType, | ||
}, | ||
|
||
/// An error occurred when attempting to parse the document's BSON bytes. | ||
#[error("invalid BSON bytes")] | ||
#[non_exhaustive] | ||
InvalidBson {}, | ||
} | ||
|
||
impl Error { | ||
pub(crate) fn value_access_not_present() -> Self { | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::NotPresent {}, | ||
} | ||
.into() | ||
} | ||
|
||
pub(crate) fn value_access_unexpected_type(actual: ElementType, expected: ElementType) -> Self { | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::UnexpectedType { actual, expected }, | ||
} | ||
.into() | ||
} | ||
|
||
pub(crate) fn value_access_invalid_bson(message: String) -> Self { | ||
Self::from(ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::InvalidBson {}, | ||
}) | ||
.with_message(message) | ||
} | ||
|
||
#[cfg(test)] | ||
pub(crate) fn is_value_access_not_present(&self) -> bool { | ||
matches!( | ||
self.kind, | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::NotPresent {}, | ||
.. | ||
} | ||
) | ||
} | ||
|
||
#[cfg(test)] | ||
pub(crate) fn is_value_access_unexpected_type(&self) -> bool { | ||
matches!( | ||
self.kind, | ||
ErrorKind::ValueAccess { | ||
kind: ValueAccessErrorKind::UnexpectedType { .. }, | ||
.. | ||
} | ||
) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is exposing how arbitrary it is which of our errors carry a string and which carry a subkind enum; that's got a bit of a code smell to it but I can't decide if it's actually a problem worth addressing. Do you have thoughts here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't love this design; I agree that it's arbitrary and inconsistent. My concern, however, is that users who do have some kind of programmatic use for the values in these errors will be less inclined to migrate to the new version, so my instinct here is to err on the side of portability for lower-priority changes. Certainly open to other designs here if you had something else in mind though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sketch of a proposal:
message
to an optional field ofError
, with corresponding code in theDisplay
impl to append it if it's present.ErrorKind
are required to be#[non_exhaustive]
struct variants, even if they're empty (which a lot now will be withmessage
stripped out)Kind
types follow the same pattern (sub-Kinds
also don't need to havemessage
since they're ultimately attached to anError
!)This makes the error space a lot more uniform, gives us a lot of flexibility for future error evolution, and doesn't introduce substantial migration burden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot more - updated with your suggestions!