shepmaster
Generate `From<source>` implementations for opaque error
What a fantastic library! Thank you to all who maintain it. 🤝
I've recently came across this and am curious about replacing our current error handling based on thiserror in favor of snafu in our open-source project. Specifically the context and backtrace features on Rust stable have really got some attention in my team.
While evaluating, one question came up after reviewing the docs and examples. Not sure if this is a bug, feature request, or simply user error in how best to achieve it with snafu. Hoping someone can point me in the right direction since I didn't know exactly where I should post this.
Question
For an opaque error setup, is there a way to instruct snafu to automatically generate the From implementations on the public error such that the question mark operator (?) cleanly resolves throughout the codebase i.e. without manually using map_err(...)? I was hoping to reduce boilerplate as much as possible and simply use ? in the rest of our library.
Minimal Example
mod package {
use snafu::prelude::*;
use std::{fs, io, result};
pub type Result<T> = result::Result<T, CustomError>;
#[derive(Debug, Snafu)]
pub struct CustomError(Kind);
#[derive(Debug, Snafu)]
enum Kind {
#[snafu(transparent)]
Io { source: io::Error },
}
pub fn custom_read(path: &str) -> Result<String> {
Ok(fs::read_to_string(path).map_err(Kind::from)?) // works
// Ok(fs::read_to_string(path)?) // was hoping this would work but needed to add impl myself
}
// impl From<io::Error> for CustomError {
// fn from(error: io::Error) -> Self {
// Self(Kind::Io { source: error })
// }
// }
}
use package::{Result, custom_read};
fn main() -> Result<()> {
let _content = custom_read("this_does_not_exist.txt")?;
Ok(())
}
I also want a #[snafu(from)] attribute, makes
#[derive(Debug, Snafu)]
pub enum Error {
Io {
#[snafu(from)]
source: std::io::Error,
backtrace: Backtrace,
},
}
equivalent to
#[derive(Debug, Snafu)]
pub enum Error {
Io {
source: std::io::Error,
backtrace: Backtrace,
},
}
impl From<std::io::Error> for Error {
#[inline(always)]
fn from(value: std::io::Error) -> Self {
IoSnafu.into_error(value)
}
}
This also meets the requirements of those who do not want the From trait to be automatically implemented.
So the quirk is not that there isn't a From conversion from the inner error type to the external error type (it is already provided), but rather that the code needs to convert the error value twice: first from io::Error to Kind and then from Kind to Error. To the best of my knowledge, Snafu cannot handle two conversion steps at the moment, even if one of them emerged from a transparent or context-free error.
In other scenarios, such as the example of an opaque error in the Snafu guide, a function returning Result<_, InnerError> can be called at a function returning Result<_, Error>, and the ? operator will take care of adapting the outcome. This code would work, for example:
mod package {
use snafu::prelude::*;
use std::{fs, io, result};
pub type Result<T, E = CustomError> = result::Result<T, E>;
#[derive(Debug, Snafu)]
pub struct CustomError(Kind);
#[derive(Debug, Snafu)]
enum Kind {
#[snafu(transparent)]
Io {
source: io::Error
},
}
pub fn custom_read(path: &str) -> Result<String> {
Ok(custom_read_inner(path)?)
}
fn custom_read_inner(path: &str) -> Result<String, Kind> {
Ok(fs::read_to_string(path)?)
}
}
use package::{Result, custom_read};
fn main() -> Result<()> {
let _content = custom_read("this_does_not_exist.txt")?;
Ok(())
}
Perhaps this calls for an exhaustive construction of From at multiple depths, though we would need to understand the feasibility of this.
@Enet4 Thanks for the quick response!
I really wanted your workaround to be suitable for our needs but additional method wrapping or replacing all ? with .map_err(Kind::from)? would add more 'noise' to our codebase than I'm comfortable with; compared to just writing the From impl blocks manually for now... bummer.
The potential feature you mentioned about somehow specifying how to construct additional From at multiple depths (minimally for an opaque error type of case) sounds like exactly what I was looking for.
@Itsusinn Now has me thinking of a syntax too. Here's my take:
mod package {
use snafu::prelude::*;
use std::{fs, io, result};
pub type Result<T, E = CustomError> = result::Result<T, E>;
#[derive(Debug, Snafu)]
pub struct CustomError(Kind);
#[derive(Debug, Snafu)]
#[snafu(from(CustomError, |inner| CustomError(inner)))] // proposed syntax
enum Kind {
#[snafu(transparent)]
Io { source: io::Error },
}
// `from(type, transform)` - In additional to existing `From` impl's, this could add
// one for each enum variant for a type using the transform function
// e.g. opaque error
//
// .
// .
// impl From<io::Error> for CustomError {
// fn from(error: io::Error) -> Self {
// let error: io::Error = (|v| v)(error);
// let original = Kind::Io { source: error };
// (|inner| CustomError(inner))(original)
// }
// }
// .
// .
pub fn custom_read(path: &str) -> Result<String> {
Ok(fs::read_to_string(path)?) // yay! (one day)
}
}
use package::{Result, custom_read};
fn main() -> Result<()> {
let _content = custom_read("this_does_not_exist.txt")?;
Ok(())
}
Thoughts?
In my use case, I need to implement impl From<T> for <Enum> where T: Into<Source> for the error enum.
Snafu’s automatic implementation causes a conflict.
For now, it would be great to provide a way to disable the automatic implementation, such as:
#[derive(Debug, Snafu)]
enum Error {
#[snafu(transparent)]
Foo {
#[snafu(source(from(false)))]
source: foo::Error
},
}
I need to implement
impl From<T> for <Enum> where T: Into<Source>
(I'm interpreting this as impl<T> From<T> for OpaqueError where T: Into<WrappedError>)
I was originally planning on writing a paragraph about how this could never work because it'd be breaking the orphan rules, but thankfully for my sake I tried it first — and it worked! I then hacked up the procedural macro to generate equivalent code, and all my tests still passed! It also allows @guzman-raphael's original request (Ok(fs::read_to_string(path)?)) to compile. This formulation seems like an overall win.
There is a downside: this change is semver-incompatible. If someone had already implemented From for their opaque error and then upgraded to this proposed version, there'd be a conflict between the new blanket implementation and their hand-written implementation.
I think the right path forward is to add this ability as an option on the opaque error definition, then flip the polarity of the option's default in the next semver release. That is, something like
#[derive(Debug, Snafu)]
#[snafu(something(true))] // You'd write this in 0.8.x
struct Opaque(Inner);
#[derive(Debug, Snafu)]
// And remove it in 0.9.x
struct Opaque(Inner);
While people who are relying on hand-written From impls would add #[snafu(something(false))] in 0.9.x.
All that being said, would people affected by this test the fix to ensure it works in your case? It should be (untested)
[patch.crates-io]
snafu = { git = 'https://github.com/shepmaster/snafu.git', branch = 'opaque-use-generics' }
The new attribute isn't created yet, so this just makes all opaque errors into the generic implementation.
I checked the code on the opaque-use-generics branch, and it seems to only support newtypes.
What I’m looking for is similar to the forward attribute in derive_more::From. The attribute marks a specific enum variant to generate a forward From implementation instead of the regular From.
and it seems to only support newtypes.
Yes, this issue is indeed focused on opaque error types, as suggested by the issue title and the branch name.
I'd encourage you to provide a minimal but complete example of the code you wish to work. As bonus points, showing what code SNAFU should generate would be informative.
Notably, I'm not understanding why you'd want such functionality for an enum variant — under what cases do you have multiple underlying error types that you want to collapse into a single error variant?
I think the right path forward is to add this ability as an option on the opaque error definition, then flip the polarity of the option's default in the next semver release
I ran mini-crater on the ~300 crates that depend on SNAFU on crates.io. Of that, a handful had build errors, but only one had a real error caused by the change. That suggests that I might skip the 0.8.x release and just skip to the 0.9.x release as there will be no impact for the vast majority of people.
I also want a
#[snafu(from)]attribute, makes
@Itsusinn I think you are looking to disable the context selector:
#[derive(Debug, Snafu)]
pub enum Error {
#[snafu(context(false))]
Io {
source: std::io::Error,
backtrace: snafu::Backtrace,
},
}
which generates this From impl:
impl ::core::convert::From<std::io::Error> for Error {
#[track_caller]
fn from(error: std::io::Error) -> Self {
let error: std::io::Error = (|v| v)(error);
Error::Io {
backtrace: {
use ::snafu::AsErrorSource;
let error = error.as_error_source();
::snafu::GenerateImplicitData::generate_with_source(error)
},
source: error,
}
}
}
This may also be what you want, @Huliiiiii ?
#[derive(Snafu)]
enum Error {
#[snafu(transparent)]
#[snafu(source(forward))]
DomainA { source: a::Error },
#[snafu(transparent)]
DomainB { source: b::Error },
}
Generated:
impl<T> From<T> for Error
where T: Into<a::Error>
{
fn from(val: T) -> Self {
Self::DomainA { source: val.into() }
}
}
impl From<b::Error> for Error {
fn from(val: b::Error) -> Self {
Self::DomainB { source: val }
}
}
Since Rust does not yet support specialization, I still need to call map_err for T: Into<b::error>. However, a forward From implementation for the most frequently used variant can significantly reduces boilerplate.
I'm going to pull the not-directly-related-to-opaque-error discussion out to #518; let's continue over there.