rust-font-loader icon indicating copy to clipboard operation
rust-font-loader copied to clipboard

Improve error handling

Open matprec opened this issue 6 years ago • 6 comments

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:

  • [ ] Windows
  • [ ] Fontconfig (*nix)
  • [ ] Mac

matprec avatar Oct 14 '17 09:10 matprec

Interested

soumya-ranjan7 avatar Oct 14 '17 10:10 soumya-ranjan7

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

ashkitten avatar Oct 14 '17 12:10 ashkitten

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 :)

matprec avatar Oct 14 '17 21:10 matprec

I will be working on Windows @MSleepyPanda

soumya-ranjan7 avatar Oct 15 '17 05:10 soumya-ranjan7

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.

ashkitten avatar Oct 15 '17 12:10 ashkitten

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 )

matprec avatar Oct 15 '17 13:10 matprec