DerGut opened a new issue, #15713:
URL: https://github.com/apache/datafusion/issues/15713
### 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
