Winit: Don't panic :)

[felixrabe]

I think there is too much unwrap in winit and it could benefit from a healty dose of Results. Thus I'm opening this issue to spark discussion on:

• [ ] Is such a change desirable? (I vote "yes".)
• [ ] To people knowing more about winit internals: Is it feasible? (If we can only get rid of panics in half the places, it might be more confusing to users than just a "catch the panic if you care" policy.)
• [ ] If yes, start a PR converting a handful of currently non-Result-returning functions to return Result. (/me volunteers.)
• [ ] Decide on when to release this. Seems to make sense to not include this change in 0.20, but maybe 0.21?
• [ ] Pretty please use the codename "Don't panic :)" for that release. :)

Original discussion.

[felixrabe]

Results can always be converted to panics, but not the other way round.

[vberger]

I'll vote against blindly trying to remove all panics in winit, for the following reason.

There are imo two kinds of panics:

• "something bad occurred but it might be feasible to recover", these are absolutely worth converting to errors (examples would be "the current platform does not support the requested actions")
• "something was fundamentally broken in the internal state of winit", these panics reveal a bug in winit, and the downstream crate cannot really do anything about it. I believe it makes sense to keep these panics (for example the wayland backend contains a large number of unwraps that fall in this category).

So I'd prefer a guiding rule like: "Don't panic if this error occurring is not a bug and can be handled by the downstream crate".

[Osspial]

I largely agree with @vberger here. There is one caveat I'd like to make here, though: In the "fundamentally broken" case, we should be using expect instead of unwrap, so that people seeing the panic can know what's causing it and where to report it.

Still, I like what this issue is trying to accomplish. In terms of concrete steps to take, I'd suggest banning unwraps in favor of expect in future PRs, and then going over Winit's codebase to determine what panics are the result of bugs and which should be removed.

[felixrabe]

I think we actually agree. I propose sensible conversions of unwrap to Result where someone originally prototyped the API and it was just not changed since, because it would break the API. I don't mean to change unrecoverable error conditions that are best left to panic. (That's what I meant by "healthy dose", and I'm currently also working my way through "Programming Rust" that contains a very nice section on the semantics of panic vs. Result.)

winit was started before Rust was even 1.0, and the ? operator that helps making Results much easier to use has only been added recently, so it makes sense that in the spirit of "let's ship it", many places where Result would (now) be preferable were just left to panic instead.