Returning closures for map_err ergonomics

[drmason13]

Just a small idea here. I've read this great article by Sabrina Jewson a few times and I've always thought "there must be a nicer way to use these errors without the wrapping closure thing".

I had a play round recently and came up with this. drmason13/modular-errors-in-rust.

The crux of it comes down to returning a closure from an impl on the error that matches the signature of map_err. We have such a closure for each error kind (more boilerplate, admittedly), but then it just fits. This pleases me greatly!

Some code here for convenience, check out the github repo linked above for more context and the full implementation:

Before:

pub fn from_file<P: AsRef<Path>>(path: P) -> Result<Self, FromFileError> {
	let path = path.as_ref();
	(|| {
		Self::from_str(&fs::read_to_string(path).map_err(FromFileErrorKind::ReadFile)?)
			.map_err(FromFileErrorKind::Parse)
	})()
	.map_err(|kind| FromFileError {
		path: path.into(),
		kind,
	})
}

After:

pub fn from_file<P: AsRef<Path>>(path: P) -> Result<Self, FromFileError> {
    let path = path.as_ref();
    let data = fs::read_to_string(path).map_err(FromFileError::read_file(path))?;

    Self::from_str(&data).map_err(FromFileError::parse(path))
}

You can see how the FromFileError method names (read_file, parse) map to FromFileErrorKind variants (ReadFile, Parse), but I do worry it looks a bit confusing at first. Why is my error type reading a file??.

I am curious if there is any significant performance penalty to the approach of returning closures from methods vs just inlining closures manually.

I'm also curious if people had thoughts how they would go about some sort of proc macro perhaps to create all the methods like read_file, parse, and so on by snake_case-ing variant names and selecting parameters from the error struct fields other than kind, and the closure types from the Errors wrapped in the ErrorKind enum.
It's the kind of thing where I just know I'd end up spending far too long trying to make a proc macro work where I could just write the impls by hand in a fraction of the time/effort

Thanks for reading!

[quinedot]

I also avoid IIFEs, but by implementing From<(SrcErr, Context)> or the like.

@drmason13

I am curious if there is any significant performance penalty to the approach of returning closures from methods vs just inlining closures manually.

It's so short it's probably just inlined, but I also would probably not care unless measurements told me I had to. The error path is rarely your hot spot anyway, in my experience.

@drmason13

check out the github repo linked above for more context and the full implementation:

Nit: the implementations on DownloadError don't need to use the -> impl FnOnce pattern since they take no extra context to capture; you could return Self and just use .map_err(DownloadError::read_body) (using the function item; no call with ()). And due to how ? works, if you had impl From<DownloadErrorKind> for DownloadError, you could even just use .map_err(DownloadErrorKind::ReadBody)?.

[drmason13]

@quinedot

return Self and just use .map_err(DownloadError::read_body)

Ah that's a really good point thank you. You are right I was only returning the closure for the sake of "fitting" into map_err, but returning the function itself is the obvious way to do that in hindsight

impl From<DownloadErrorKind> for DownloadError is an interesting one.
I like how terse that is!
But it is worth also bearing in mind (I didn't make this clear in my post at all) that this might not be desirable if the errors are part of your public API
and you might want to later on add context to the error struct.

Implementing From<ErrorKind> "closes the door" so to speak on adding context later.
Speaking of which, all these convenience methods should probably not be pub for the same reason.

Thank you for you sharing your thoughts, and that IRLO post - going to have a good read through of that now. Insightful as ever

I will be experimenting with using this impl From<(ErrorKind, Context)> pattern. It reduces the boilerplate to one method impl per ErrorKind/Context pairing, so I am a fan

[quinedot]

@drmason13

But it is worth also bearing in mind (I didn't make this clear in my post at all) that this might not be desirable if the errors are part of your public API
and you might want to later on add context to the error struct.

Implementing From<ErrorKind> "closes the door" so to speak on adding context later.

Fair enough. But In your gist, DownloadErrorKind is public and DownloadError has kind as a public field. So consumers can already do the same construction as the From implementation manually. There's still an argument of what you want to encourage, I suppose. But probably those should just be private, too, if you don't want to close the door.^[1][2]

(In the example I linked, FileReadErrorKind and the corresponding kind field are private.)

---

1. At which point, From doesn't close any doors either. ↩︎

2. As a bonus you may be able to excise specific dependencies like ureq from your public API. ↩︎