Common midir error trait

[nettoyeurny]

Hi! I'd like to handle midir errors in a uniform way, and so I added a common midir error trait.

[Boddlnagg]

Thanks for the PR! I have a few questions:

• What exactly does this allow you to do that you can't do without it?
• Is the same pattern applied in other Rust libraries that I could look at?
• Wouldn't it be sufficient to increment the version to 0.7.1? It's not an incompatible change in any way, right?

[nettoyeurny]

1. My immediate motivation is that I have a project that uses midir under the hood, but I'd like to hide the implementation details from client code. My library introduces its own error trait, and that's the only kind of error that client code has to deal with. It looks something like this:

#[derive(Debug)]
pub enum MyFancyMidiDeviceError {
Midi(String),
Device(String),
}

In other words, client code can tell whether the problem was on the MIDI side or on the device side, and that's all I want to know. In particular, I don't want to expose the nature of the MIDI implementation. The first draft used rtmidi, but then I decided that I like midir better, and I'd like to be able to replace the MIDI implementation without changing client code.

If you accept my pull request, I'll be able to consume all midir errors with a single conversion:

impl<E: MidirError + Display> From for MyFancyMidiDeviceError {
fn from(err: E) -> MyFancyMidiDeviceError {
MyFancyMidiDeviceError::Midi(err.to_string())
}
}

Without my pull request, I have to implement the From trait for all midir error types. (Or I could introduce MidirError in my library and add implementations for all midir error types.)

2. I'm just getting started with rust, so I don't yet have a good sense of best practices. If my approach isn't idiomatic and there's a better way, then I'd like to learn about it :) Coming from an object-oriented mindset, however, I do believe that it makes a lot of sense to funnel errors through a common base trait.

3. I don't feel strongly about the version number, but incrementing the minor version number seems appropriate here. As you point out, my change is backwards compatible, but it does add functionality, i.e., code that depends on my change wouldn't work with the previous version, and that's what the minor version communicates.

[nettoyeurny]

Here's another way to accomplish what I have in mind: Replace multiple error types with a single error enum whose variants are the current error types. That would also allow me to uniformly absorb midir errors, but it would be much more intrusive and wouldn't be backwards compatible, i.e., it would require a major version number change.

[kangalio]

What exactly does this allow you to do that you can't do without it?

Not a whole lot. The code added could go into user crates equally well. If anything, this PR is not about opening up some entirely new use case; it's more about reducing boilerplate in dependent crates

Is the same pattern applied in other Rust libraries that I could look at?

Tons of crates have a blanket error enum that encompasses all possible errors within the library. However, this is mostly for the crate's own convenience of not having to return fine-grained error types (like midir honorably does).

This PR is non-standard in that it introduces an error trait instead of an enum. Since you can't store a trait directly, users are expected users to store an opaque object like Box<dyn midir::Error> or String.

Not sure if an error trait or enum is more idiomatic here. Both would suit @nettoyeurny's use case equally well since you could call .to_string() on both. I'd say neither should get added to midir since dependent crates can add an error trait easily themselves (literally 4 lines of code; error enum is slightly more) and upstreaming them to midir is unneeded.