Display does not include source

[brooksprumo]

When printing out the errors generated by fs-err with Display, the message does not contain the source. This often means we know what happened, but not why an error happened.

For example, I encountered this when a call to fs_err::hard_link() failed. I have an error hierarchy that wraps the error from the hard link call. Printing the error with Display shows my custom text, then the text from fs-err, but not the text from the source/underlying std::io::Error.

Here's an example in the Playground (please pardon the unpolished nature of the code):
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a5e0844e243ec0807372b01227419b50

The Display1 shows wrapping a normal std::io::Error. We can see the output does not include the path. So this has the why, but no what.

In Display2, which is what fs-err does, we have the path, but not the underlying source io::Error included in the output. This is having the what without the why

And Display3 is what I was hoping that fs-err did: it prints the what followed by the why.

By adding the source at the end of the Display, this automatically keeps printing more and more inner source errors at the end; providing more and more information/context.

For fs-err, this can be accomplished by changing the implementation of Display from here:

fs-err/src/errors.rs

Lines 65 to 101 in ba98887

	fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
	use ErrorKind::*;
	let path = self.path.display();
	match self.kind {
	OpenFile => write!(formatter, "failed to open file `{}`", path),
	CreateFile => write!(formatter, "failed to create file `{}`", path),
	CreateDir => write!(formatter, "failed to create directory `{}`", path),
	SyncFile => write!(formatter, "failed to sync file `{}`", path),
	SetLen => write!(formatter, "failed to set length of file `{}`", path),
	Metadata => write!(formatter, "failed to query metadata of file `{}`", path),
	Clone => write!(formatter, "failed to clone handle for file `{}`", path),
	SetPermissions => write!(formatter, "failed to set permissions for file `{}`", path),
	Read => write!(formatter, "failed to read from file `{}`", path),
	Seek => write!(formatter, "failed to seek in file `{}`", path),
	Write => write!(formatter, "failed to write to file `{}`", path),
	Flush => write!(formatter, "failed to flush file `{}`", path),
	ReadDir => write!(formatter, "failed to read directory `{}`", path),
	RemoveFile => write!(formatter, "failed to remove file `{}`", path),
	RemoveDir => write!(formatter, "failed to remove directory `{}`", path),
	Canonicalize => write!(formatter, "failed to canonicalize path `{}`", path),
	ReadLink => write!(formatter, "failed to read symbolic link `{}`", path),
	SymlinkMetadata => write!(formatter, "failed to query metadata of symlink `{}`", path),
	FileExists => write!(formatter, "failed to check file existance `{}`", path),
	#[cfg(windows)]
	SeekRead => write!(formatter, "failed to seek and read from `{}`", path),
	#[cfg(windows)]
	SeekWrite => write!(formatter, "failed to seek and write to `{}`", path),
	#[cfg(unix)]
	ReadAt => write!(formatter, "failed to read with offset from `{}`", path),
	#[cfg(unix)]
	WriteAt => write!(formatter, "failed to write with offset to `{}`", path),
	}
	}

To something like:

    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        use ErrorKind::*;

        let path = self.path.display();

        match self.kind {
-            OpenFile => write!(formatter, "failed to open file `{}`", path),
+            OpenFile => write!(formatter, "failed to open file `{}`: {}", path, self.source),
            // and the same thing for the subsequent arms

I can create a PR for this; just wanted to check if this would be well-received first. Please also let me know if there is more information that would be helpful.

Edit: I created a PR since it was quite quick: #53

[andrewhickman]

There's no clear consensus on which way is better in rust-lang/project-error-handling#23, however I tend to agree with this guideline

For now, the recommendation is to always do one or the other. If you're going to print your sources, you should not return them via the source function and vice versa.

I think its useful for this crate to expose the original error through source(), in case people want to get things like raw_os_error, so it shouldn't duplicate the error message in the Display impl.

[brooksprumo]

There's no clear consensus on which way is better in rust-lang/project-error-handling#23, however I tend to agree with this guideline [...]

Thank you for the link and information. This was helpful for me to read, and now I understand the impl in fs-err.