Considering displaying underyling error on Context Display

[withoutboats]

Context's Display impl currently displays only the Context's display message. The idea was that the context would be relevant to end users, but the underlying error would likely not be. Also the idea was that you could "just" use the debug output to get the whole thing, but they debug output of most errors is atrocious.

I think just doing the context's display colon the error's display is probably a strict improvement.

cc @aturon

[shepmaster]

I'm going to 👍 the general gist here. I definitely wrote:

let mut f = File::open(fname).with_context(|_| fname.clone())?;

Since the concepts of "open" and "filename" are the context of the error to me. This doesn't print the actual error though, so I ended up at

let mut f = File::open(fname)
    .with_context(|e| format!("Can't open {}: {}", fname, e))?;

---

Of interest is that it doesn't really make my existing code feel much better:

before

f.read_to_string(&mut s)
    .unwrap_or_else(|e| panic!("Can't read {}: {}", fname, e));

after

f.read_to_string(&mut s)
    .with_context(|e| format!("Can't read {}: {}", fname, e))?;

[L-as]

I've realised that there is an issue with @shepmaster's code: it Displays the error even if the error never needs to be displayed. This is probably not that common, but it's something to keep in mind when considering if Context should also show the cause.

[shepmaster]

it Displays the error even if the error never needs to be displayed

@Laaas could you expand on that? with_context is never called unless an error has occurred. I don't understand what else might you mean by using Display as a verb here.

[L-as]

@shepmaster an error occurring doesn't mean you have to display it.

[L-as]

It's not very relevant when you're using strings as errors, but when you have an actual error type, you can't take ownership of the error or refer to it, so you need to display it and store the resulting string.

[shepmaster]

Do you mean something like this, where the Result is created, the context is added, but then the Result is thrown away?

extern crate failure;

use std::fs;
use failure::{Context, ResultExt};

fn do_something(name: &str) -> Result<String, Context<String>> {
    fs::read_to_string(name).with_context(|_| {
        println!("Adding context...");
        format!("Woah! {} was not a good file", name)
    })
}

fn main() {
    let r = do_something("does not exist");
    drop(r);
}

If so, I think you are right, but I don't see any alternative. Likewise, the "regular" error (io::Error in this case) is also created when it never needs to be. Of course, it's possible that the optimizer removes cases where it can (not in my code because the println is a side effect).

but when you have an actual error type, you can't take ownership of the error

Ah, you mean like this

extern crate failure;

use std::{io, fs, fmt};
use failure::{Context, ResultExt};

struct MyFileCtx<'a> {
    name: &'a str,
    err: io::Error,
}

impl<'a> fmt::Display for MyFileCtx<'a> {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "Could not read {}", self.name)
    }
}

fn do_something<'a>(name: &'a str) -> Result<String, Context<MyFileCtx<'a>>> {
    fs::read_to_string(name)
        .with_context(|err| MyFileCtx { name, err })
}

fn main() {
    let r = do_something("does not exist");
    drop(r);
}

Note that writing a bit of code often expresses the idea we are trying to communicate more clearly.