Implement std::error::Error::source() for DataFusionError, make DataFusionError::find_root more generic
#4992
Conversation
|
|
||
| let mut last_datafusion_error = self; | ||
| let mut root_error: &dyn Error = self; | ||
| while let Some(source) = find_source(root_error) { |
There was a problem hiding this comment.
I am quite pleased with how this works (and it will work for other intervening wrapping error types if they implement source() as well)
| } | ||
| // some errors are wrapped into `Arc`s to share them with multiple |
There was a problem hiding this comment.
Is this necessary? The source implementation of Arc should already call this?
There was a problem hiding this comment.
It is necessary, but I don't know why. The tests fail without it. If you could help figure out how to avoid it it would be great.
|
Benchmark runs are scheduled for baseline = 350cb47 and contender = e713547. e713547 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #4991
Rationale for this change
In IOx (and in DataFusion) we often want to know what the root cause of an error is (e.g was it a bug or was it a resources exhausted). ArrowError can wrap other errors with Arrow::External (and DataFusion has something similar) but there is no easy way to get at the source of the error to walk the chain without knowing all the possible types that may be present
I want a generic way to walk up the error stack
What changes are included in this PR?
std::error::Error::source()forDataFusionErrorDataFusionError::find_rootless specific (uses source() rather than so much checking for arrow)Are these changes tested?
Yes, covered by (some quite gnarly) existing tests
Are there any user-facing changes?
Better standard support