API to match against any error in chain
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### 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
```rs
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
```rs
/// 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()`](https://github.com/apache/datafusion/blob/63f37a34404391f19114407c2a3965213306bb8e/datafusion/common/src/error.rs#L423): 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](https://github.com/apache/datafusion/pull/15692#discussion_r2041239380) 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`](https://pkg.go.dev/errors#Is) which is the most robust and idiomatic way of checking errors in Go.
Contributor guide
Research direction
Start with datafusion/common/src/error.rs and the DataFusionError::find_root entry point, then compare the proposed error_matches! API with the documented error-chain examples. Done means agreeing on a public way to match any error in the chain and its behavior for the shown variants.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100