Error types everywhere instead of String

[fluffysquirrels]

Some of the errors returned by the library implement the std::error::Error trait and some are just Strings. I propose all errors returned by the library implement std::error::Error to simplify error handling for consumers.

In the cases where we only have a String to return, this can be wrapped in an enum that implements Error.

This would of course be a breaking change.

Would you be interested in a PR for this?

[Cobrand]

I would definitely be interested in a PR for this. The problem is that it's mostly a trial and error process. The strings don't look as nice as "GPU_ERROR_56", but rather look like "Error when opening keyboard state: 0x5786 driver error" (so yeah, there would be some parsing to do as well).

Basically what I mean is that most of the errors SDL2 returns are really hard to parse errors, aside for a few ones that are static... The documentation on the matter is also rather sparse (and sometimes non existant), you have to dig through the source code to know exactly what some error might mean. That, and even if you were to cover all string cases, there might be a case where SDL2 adds a new string error without our knowledge, so assuming we use enums instead of string, there would always be the need to have a Unknown(String) variant.

However, as cumbersome as it may look, it would be a definitely huge feature to have. Just know that it may not be as trivial as you might think at first glance!

[fluffysquirrels]

Parsing all the possible strings does sound like a lot of work. I was thinking of just wrapping everything that is currently a String in an Unknown(String) variant.

The benefit would be for the consumer being able to cast the library's errors into their own error type with a blanket impl of From<E: std::error::Error>; this is what the failure crate supports. Currently in my code I need to do things like do_it().map_err(|s| Error::String(s))? everywhere to map the error variants of Results.

[fluffysquirrels]

I've done some private work on this and have adjusted all the functions that return Result<T, String> to Result<T, Error> with a new Error enum. There remain other error types sprinkled throughout the library e.g. sdl2::controller::AddMappingError, so you can't simply write a function of Result<T, sdl2::Error>. The libraries I've seen have a single error enum with lots of variants, is that the approach we want to take?

[Cobrand]

Mmmh no, it doesn't make sense in my opinion to have a ControllerMappingError variant or other inside an enum returned in a "render" function... You could create error kinds per category at most (render, controller, keyboard, ...), but don't include everything inside a single enum.

[fluffysquirrels]

OK.

In that case to make the examples neat I've tried using the failure crate to convert all the different Error types into failure::Error. Here's the diff for the first example, what do you think?

[fluffysquirrels]

Ah wait, we don't need the failure crate, a return type of Box<dyn std::error::Error> works fine. I'll fix up the rest of the examples shortly.