gnembon / fabric-carpet

Fabric Carpet
MIT License
1.72k stars 275 forks source link

[Scarpet] Better messaging for `__config:'requires'` #1042

Closed altrisi closed 2 years ago

altrisi commented 3 years ago

When I was implementing that, there wasn't really any way to exit there without either returning false, which would say that "config is invalid", not the case, or throwing an IEE (the one used), which would create a full stacktrace at the `config` declaration.

Both of those options really break with an app trying to tell the user why the app failed to load, since those take a lot more space and drive more attention (plus it looks like the app is wrong and not the user/system has done something wrong).

I think requires should not throw an IEE but instead some custom exception that would break the loading flow relatively silently, maybe ending with the simple Failed to load <name> app (after whatever the user specified of course).

Currently, the "cleanest" (IMO) way of breaking the loading flow is to exit() before a function required for the app's command is added, given that the error is only two lines, but the message isn't really accurate.

My use case here is loading required app data from JSON, and cancelling app load if an io_exception is thrown (aka file corrupted/badly edited).

How should I implement this? My current thought is throwing a new exception that implements ResolvedException (so it's rethrown until high enough) and catching it near the actual loading code, maybe something like AppLoadingCancelledException (or just AppLoadingException?).

gnembon commented 3 years ago

using throw with a custom exception is something that makes the most sense to me (without thinking too much about it). Let me ponder on it for a moment.