HASH
error-stack support for multiple related errors
#737
Replies: 14 comments
-
|
For the "source" of the error (with compatibility to |
Beta Was this translation helpful? Give feedback.
-
|
Just as a reference, the topological sorting using DFS of the graph shown above would result in the following: flowchart BT;
a{{std::io::Error}} --> b{{crate1::Error}}
b --> c[crate2::Context2]
c --> d[crate3::Context2]
d --> e{{std::io::Error}}
e --> f[crate2::Context]
f --> g[crate3::Context]
g --> h[crate4::Context]
|
Beta Was this translation helpful? Give feedback.
-
|
Hi @indietyp and thanks for filing this! Generally, Pull Request are highly appreciated! This is a pretty good scenario and we haven't thought about this yet. However, it's possible to attach any data to a let report_a = Report::new(...);
let report_b = Report::new(...);
report_a.attach(report_b);Later it's then possible to call However, there is one issue: |
Beta Was this translation helpful? Give feedback.
-
|
My suggestion would be to have a new trait: The thing that would need to be changed is that Traversing the "graph" via
if |
Beta Was this translation helpful? Give feedback.
-
|
but to your earlier point, the provider API would kind of make sense, but I would need to know the context type in advance, then traverse through, which is doable, but a lot of pain |
Beta Was this translation helpful? Give feedback.
-
|
I had a thought when traversing frames, we could, in theory, use BFS or DFS (or the topological sorting), but this would mean that we wouldn't be able to guarantee that every frame before a parent of the previous one. |
Beta Was this translation helpful? Give feedback.
-
|
I thought about this for a long time and I think I came up with a possible solution. Now, a
I'd prefer the second option, especially as it's not special casing What do you think about that? |
Beta Was this translation helpful? Give feedback.
-
|
I like the 2nd approach of using This would mean that in my aforementioned example of I feel like this could be pretty confusing for users because in the whole library, you "layer" things on top of each other - but you then break that methodology, which could confuse if that makes sense. (Just trying to think aloud) I like the output you proposed. It looks apparent and concise. In my own projects (where I'd really like to switch to error-context) I am currently making use of a modified version of the let group = ReportGroup::new(context);
group.push(err);
group.push(err2);
group.check()?;What do you think? |
Beta Was this translation helpful? Give feedback.
-
|
Great that we are on the same page regarding the 2nd approach! Yes, logically, this would be more like I experimented around a little bit locally and I think I was able to resolve this. I'll use a bit of pseudo-code to express the approaches: struct Report {
// Each report can hold multiple frames
frames: Vec<Frame>, // Could be changed to `SmallVec<[Frame; 1]>`
}
struct Frame {
// When attaching or changing context the `report.frames` are used as the sources for the new frame
// so we don't need to add
sources: Box<[Self]>,
...
}
// Combining reports with same context
impl<C> Extend<Report<C>> for Report<C> {
...
}
// Probably, this could also be extended by implementing `FromIterator` to use `collect` but this also
// introduces weird edge cases like passing an empty collection
Thank you! Actually, it would be grand if we could have a similar output like
I'd prefer not to add new structs to the crate. I think my idea would result in the same behavior? My approach however has two downside:
|
Beta Was this translation helpful? Give feedback.
-
Maybe using this - in another PR - might be helpful to implement multiple possible output formats (with a hook system of some sorts) - I can imagine output to JSON might be beneficial as well.
Yes, it does. I just found this approach a bit better, especially the panic on the drop, as sometimes you forget to return the error. After thinking about it, this isn't something that should be in the main crate I think the benefits for (1) outweigh the downsides. If the next version already introduces breaking changes, it might be good to have those in one swoop and then keep them to a minimum - if that makes sense. Regarding (2), Report and Frame would be larger, but this might be negligible with |
Beta Was this translation helpful? Give feedback.
-
We already support that. It's possible to set a debug hook or a display hook. A JSON output example is also provided.
Couldn't this be solved by marking
I agree. Having breaking changes in only one release is far better than having dozens of breaking releases, even though it's technically allowed.
This should mostly be fine. The fast path is most likely the happy path, not the error path, so allocating one more Do you want to open a PR for this feature? 🚀 |
Beta Was this translation helpful? Give feedback.
-
O yea I totally forgot about that 🤦
sounds good to me! Yea, I think I am going to give implementing this functionality a try :D |
Beta Was this translation helpful? Give feedback.
-
|
Great! Please feel free to ping me when having any problems! 🙂 |
Beta Was this translation helpful? Give feedback.
-
|
This feature was implemented in #747 and will be available with |
Beta Was this translation helpful? Give feedback.
-
Pull Request:
error-stack#747I have been checking out
error-stacklately, and it seems very promising, great work! For my specific usecase I would want report multiple errors at once, while not natively supported by thestd::error::Errortrait, crates like miette have the concept of related errors.From my investigation into the library
error-stackdoesn't currently support this feature, is this correct? Are there any plans for supporting such a thing in the future? or would you be open for a PR implementing something like this? I think something like this would be a great addition to the library!What I'd love to have is a DAG like the diagram below:
flowchart BT err1{{std::io::Error}} err2{{std::io::Error}} err3{{crate1::Error}} ctx1[crate2::Context] ctx2[crate3::Context] ctx3[crate4::Context] ctx4[crate2::Context2] ctx5[crate3::Context2] err1 --> ctx1 ctx1 --> ctx2 err2 --> ctx2 ctx2 --> ctx3 err3 --> ctx4 ctx4 --> ctx5 ctx5 --> ctx3Beta Was this translation helpful? Give feedback.
All reactions