Consider hiding Error internals

[jonas-schievink]

Currently, adding a variant to Error (or changing an existing one) is a breaking change. This happens pretty frequently during development. We could prevent that by making Error a struct with a few helper methods to create a specific error, and make the internal enum private so it's not part of the API.

This is probably not that important right now, but when rlua gets closer to a 1.0 release it could allow more freedom in expanding rlua's internals (better error messages, more distinguished error cases without a breaking change). I'm just opening this issue to get a bit of feedback.

[Timidger]

If you do this, it would make it harder to remember to match on specific error cases, especially when there's a change and e.g an error is made more specific.

Though if you're thinking of changing the error system, might I suggest the new failure crate?

[jonas-schievink]

If you do this, it would make it harder to remember to match on specific error cases, especially when there's a change and e.g an error is made more specific.

Do you have an example where this is useful? I've mostly returned elaborate error messages from within callbacks (ie. where the error actually happens), not after catching an error that happened elsewhere.

Though if you're thinking of changing the error system, might I suggest the new failure crate?

That's probably a good idea, but it does pull in an additional dependency.

[Timidger]

Do you have an example where this is useful? I've mostly returned elaborate error messages from within callbacks (ie. where the error actually happens), not after catching an error that happened elsewhere.

I'm not sure what type of changes your are thinking of making to the errors, but there are cases in Way Cooler where I explicitly match on Runtime or Type conversion errors in order to either swallow the error or perform a different action. Obviously this is still do-able with what you suggested, but if the error is opaque and changes when those errors happen it would make the error handling more difficult.

For my 2 cents: if most of the error messages are either caught (e.g with an expect), raised (e.g try! or ?), or suppressed (e.g matching on any error and suppressing it), which is the majority of my particular use case (obviously there are others, this is just my opinion) I don't see the gain in trying to reduce breakage when a new error condition is possible. That is a breaking change and trying to paper over it will just hurt downstream consumers of the library.

That's probably a good idea, but it does pull in an additional dependency.

AFAIK the plan is to eventually incorporate this into the STD. Most people (including me) are waiting for that to actually happen though, so if an additional dependency is too much then feel free to wait as well.

[torkleyy]

The way I avoid this problem usually is to add another variant, __NonExhaustive with #[doc(hidden)] and a doc comment saying that you shall never match against it, so you need to have a _ pattern. This would make adding errors a non-breaking change. Changing an error obviously still is a breaking change.

[jugglerchris]

Closing as this is out of date (Error has changed to use failure and then back again in that time).