Skip to content

API to match against any error in chain #15713

@DerGut

Description

@DerGut

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

  1. 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.
  2. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions