Display implementation of `signature::Error` can lead to errors being printed twice

[casey]

It's a somewhat common pattern, when an error has a underlying source, to print the error itself, and then print the source error, and so on, recursively, in error messages.

Here's an example of what this looks like in snafu:

Error: Unable to frobnicate the mumbletypeg

Caused by these errors (recent errors listed first):
  1: Could not contact the mumbletypeg API
  2: The configuration has no password

Because signature::Error both contains a source error, and includes that source error in its display implementation, this can lead to the source error being printed twice, leading to something like this:

Error: signature error: Verification equation was not satisfied

Caused by these errors (recent errors listed first):
  1: Verification equation was not satisfied

I think that given the semantics of signature::Error, it probably shouldn't have a source error. A source error is an underlying error which caused the higher level error. In the case of signature::Error, if I'm understanding it correctly, the source error really is the top level error, and not an underlying error which caused it.

[tarcieri]

We can consider removing printing the source in the Display impl, however note:

I think that given the semantics of signature::Error, it probably shouldn't have a source error.

signature::Error can contain an inner boxed error as its source: https://docs.rs/crate/signature/2.2.0/source/src/error.rs#29

The primary intended use case is communicating errors from remote signing operations on e.g. HSM, TPM, SEP, or KMS for handling communication errors with these devices/services, while also erasing the inner type, which may be specific to a particular device driver library / service client.

These sorts of applications are, IMO, exactly what Error::source is intended for.

[casey]

Sorry, let me be more precise.

Let's call the error that signature::Error contains the "underlying" error, since it's confusing to call it the "source" error, since that's also the name of the std::error::Error::source() method.

I think there are two options, both reasonable, which don't lead to double-printing the error:

1. The underlying error is not included in the Display message, and std::error::Error::source() returns the underlying error
2. The underlying error is included in the Display message, and std::error::Error::source() returns underlying.source()

[tarcieri]

The second option would break downcasting Error::source to obtain the concrete error, which is something I use today in running code.

Changing the Display impl is comparatively benign.