Is your feature request related to a problem or challenge?
Wrapping a DataFusionError in a DataFusionError::Context can break behavior for users if they match on specific error variants. E.g. code that handles a Result like
match do_datafusion_things() {
Ok(_) => Ok(())
Err(DataFusionError::Execution(_)) => {
// retry
}
Err(e) => Err(e)
}
will stop working as soon as the DataFusionError::Execution returned by DataFusion would be wrapped in context.
Describe the solution you'd like
The best I came up with so far is a macro like
/// Macro to check if an error matches a pattern, similar to `matches!` but for error checking.
/// This macro will traverse through wrapped errors to find a match.
///
/// It is preferable to use `error_matches` over matching on specific error types directly. This is because
/// matching code could otherwise change behavior after the addition of an indirection, e.g. by wrapping
/// an error in Context.
///
/// # Examples
///
/// For example, given the following error chain:
/// ```text
/// DataFusionError::ArrowError
/// ArrowError::ExternalError
/// Box(DataFusionError::Context)
/// DataFusionError::ResourcesExhausted
/// ```
/// `error_matches` will return true for `DataFusionError::ResourceExhausted`,
/// `DataFusionError::Context` and `DataFusionError::ArrowError`.
///
/// In code, this could look like:
/// ```
/// use datafusion_common::error_matches;
/// use datafusion_common::DataFusionError;
///
/// let err = DataFusionError::ArrowError(
/// ArrowError::ExternalError(Box::new(DataFusionError::Context(
/// "additional context".to_string(),
/// Box::new(DataFusionError::ResourcesExhausted("oom".to_string())),
/// ))),
/// None,
/// );
///
/// assert!(error_matches!(err, DataFusionError::ArrowError(..)));
/// assert!(error_matches!(err, DataFusionError::Context(..)));
/// assert!(error_matches!(err, DataFusionError::ResourcesExhausted(_)));
/// ```
#[macro_export]
macro_rules! error_matches {
...
}
While this seems nice to work with, it's not a drop-in replacement of the match syntax above.
I'm still ramping up in Rust, so any suggestions on how to improve this API are very much welcomed!
Describe alternatives you've considered
- Using
DataFusionError::find_root(): this function is the closest solution to the problem we have today. But it only allows to check against the very last error in a chain, any errors in between can't currently be handled.
- Extending
impl DataFusionError with different variants of fn wraps(&self, other: &DataFusionError) -> bool: even if a macro provides a nicer API to work with, it will ultimately be backed by such a function
Additional context
@rluvaton noticed this issue when I added context to an existing error in a separate PR. It started a discussion about whether or not adding context could be considered a breaking change with the lack of such an API.
Note, this proposal is heavily inspired by Go's errors.Is which is the most robust and idiomatic way of checking errors in Go.
Is your feature request related to a problem or challenge?
Wrapping a
DataFusionErrorin aDataFusionError::Contextcan break behavior for users if theymatchon specific error variants. E.g. code that handles aResultlikewill stop working as soon as the
DataFusionError::Executionreturned by DataFusion would be wrapped in context.Describe the solution you'd like
The best I came up with so far is a macro like
While this seems nice to work with, it's not a drop-in replacement of the
matchsyntax above.I'm still ramping up in Rust, so any suggestions on how to improve this API are very much welcomed!
Describe alternatives you've considered
DataFusionError::find_root(): this function is the closest solution to the problem we have today. But it only allows to check against the very last error in a chain, any errors in between can't currently be handled.impl DataFusionErrorwith different variants offn wraps(&self, other: &DataFusionError) -> bool: even if a macro provides a nicer API to work with, it will ultimately be backed by such a functionAdditional context
@rluvaton noticed this issue when I added context to an existing error in a separate PR. It started a discussion about whether or not adding context could be considered a breaking change with the lack of such an API.
Note, this proposal is heavily inspired by Go's
errors.Iswhich is the most robust and idiomatic way of checking errors in Go.