4 ms·
I'm not sure I like how they're trying to dynamically cast to an error type. Err(report) => { // For machines: find and handle the structured error
by bccdee 9mo ago
I'm not sure I like how they're trying to dynamically cast to an error type.
Err(report) => {
// For machines: find and handle the structured error
if let Some(err) = find_error::<StorageError>(&report) {
if err.status == ErrorStatus::Temporary {
return queue_for_retry(report);
}
return Err(map_to_http_status(err.kind));
}
They get it right elsewhere when they describe errors for machines as being "flat and actionable." `StorageError` is that, but the outer `Err(report)` is not. You shouldn't be guessing which types of error you might run into; you should be exhaustively enumerating them.
I'd rather have something like this:
struct Exn<T> {
trace: Trace,
err: T,
}
impl<T> Exn<T> {
#[track_caller]
fn wrap<U: From<T>>(self, msg: String) -> Exn<U> {
Exn {
trace: self.trace.add_context(Location::caller(), msg),
err: self.err.into(),
}
}
}
That way your `err` field is always a structured error, but you still get a context trace. With a bit more tweaking, you can make the trace tree-shaped rather than linear, too, if you want.
I think actionable error types need to be exhaustively matchable, at least for any Rust error that you expect a machine to be handling. Details a human is interested in can be preserved at each layer by the trace, while details the machine cares about will be pruned and reinterpreted at every layer, so the machine-readable info is kept flat, relevant, and matchable.
- andylokandy 9mo ago`Exn<T>` preserves the outmost error type and `Exn::<T>::as_error()` will give you the error just the way you want. Traversing though the error tree is the worst case where the structured error has been bubbled up through layers until the one who are able to recover from it.
- bccdee 9mo agoThat worst case shouldn't happen. When you do something like this: let data = serialize(&doc) .or_raise(|| StorageError::permanent("serialization failed"))?; you're burying the error thrown by `serialize(&doc)` in such a way that you have to dig for it dynamically to recover it. The closure in `or_raise` should take the error from `serialize(&doc)` as an argument and save the actionable details. It makes sense to have an "outermost error" when you're talking about a context trace that provides information to humans, but an error which a machine responds to should be flat & statically matchable. Something like this: let data = serialize(&doc) .or_raise(|e| Exn::with_message( "serialization failed", StorageError::fromInner(e)))?; where `StorageError::fromInner` decides whether the error should be permanent or not based on the contents of `e`, and saves any details that would be relevant for automatically recovering from the error.