Open simonsan opened 1 year ago
We don't use ErrorKind
except std::io::ErrorKind
.
We don't use
ErrorKind
exceptstd::io::ErrorKind
.
Please reopen, ErrorKind was paraphrasing different ErrorKinds all over the place like https://github.com/containers/youki/blob/main/crates/libcgroups/src/stats.rs#L200
Other examples: https://github.com/containers/youki/blob/dfe6ee80bc780449fffcf22ecf6618c7c30791ae/crates/libcgroups/src/v2/util.rs#L12
I see. I was confused with the ErrorKind
mention. Apologize.
Would you like to take a shot and draft a PR for this?
I see. I was confused with the
ErrorKind
mention. Apologize.Would you like to take a shot and draft a PR for this?
Yes, can do. I think it should be decided beforehand though, if an opaque error type would be needed or if we just move all the errors into error.rs
?
@simonsan Honestly, we don't have a good idea on this yet. We just recently migrated our error definitions from anyhow to structured errors using thiserror
. We have not thought through all implications of proper error interface yet.
Can you point me to some readings or examples on opaque error type? Especially, if you can help us understand the trade offs between different approaches? Thanks.
Something like this could be helpful: https://web.archive.org/web/20230128000646/http://www.tglman.com/posts/rust_lib_error_management.html
I think as a second step to the refactoring to thiserror
we should collect the available error kinds in a central place within each crate (error.rs
). So we get an overview.
From there on we can check what is available and which approaches would make sense. It's hard to say at this point to be honest, as I don't have an overview.
@yihuaf is this issue still open? if yes, can i be assigned?
i do think having a error.rs
in each crate does improve discoverability.
but then again, defining errors closer to where they are actually used instead of a single error.rs
file is also a good idea.
@yihuaf friendly remind ;) If you find time to address, feel free to let me know.
Currently
ErrorKind
s are all over the place, defined where they are needed. This isn't great for visibility and also importing can get quite tedious, as it's not clear on first sight what error types a crate exposes. Which usually (AFAIK) is being done by looking intoerror.rs
.Another thing to think about is to use one opaque error type and implement something like an ErrorMarker to be able to convert into that opaque error type: