matprec / rust-font-loader

A font loading utility written in rust.
MIT License
51 stars 21 forks source link

Improve error handling #16

Open matprec opened 6 years ago

matprec commented 6 years ago

There are a lot of unwrap()s in the code. Proper error handling with Result's would be nice.

Happy to provide guidance! Just comment on this issue.

Edit:

soumya-ranjan7 commented 6 years ago

Interested

ashkitten commented 6 years ago

Just a personal recommendation - I really like using the error_chain crate

matprec commented 6 years ago

I haven't used error_chain before, but looks nice!

@soumya-ranjan7 Look for unwraps in the src/ folder. pub fns with an unwrap in their path should return a Result. The Error type would be an enum in src/lib.rs, with could look like this:

enum LoadingError {
    FontNotPresent,
    Platform(Error)
}

Like @ashkitten suggested, you can use the error_chain crate if you want to :)

The unwraps in win32 like this one should be converted into an expect instead of unwrap, since we're depending on correct handling by the os. Since there are three different platforms, please add which one you are working on and comment on this thread which public interfaces were changed.

If you have questions, post them here :)

soumya-ranjan7 commented 6 years ago

I will be working on Windows @MSleepyPanda

ashkitten commented 6 years ago

Isn't it better as a library especially to bubble the errors up by returning Result? That way any application using the library can deal with errors however it wants, without having to panic if something goes wrong. Generally, panicking in a library is a bad thing to do.

matprec commented 6 years ago

In general, i agree. But when the OS is returning non null terminated or non utf8 strings, we have a bigger problem than the library panicking. At some point, we have to trust the OS i think.

(Fight me :P )