New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: Add WIP panic handler code #579
base: main
Are you sure you want to change the base?
Conversation
We sort of discussed this in that issue and in the Matrix room, but what's the benefit of this over just capturing panics and aborting on a crate-wide level? |
With the approach in this PR you can specify individual behavior for each component. You can choose what should happen and whether the entire app should be aborted or not. Also, it's just a method on the |
That was leading me to think about when you would ever not want the entire app to be aborted (or at least start unwinding, which isn't a current possibility). When a component panics you can't really use it much anymore, and there doesn't seem to be a good way to handle that kind of state from parent components. I feel like the lack of handling in that regard from parent components would just make things bound to break, as components don't expect that their child components are just going to panic. I know at least in the app that I'm working on that I'm not expecting things to work at all once a panic occurs, and the pre-Relm4 code where panics weren't captured behaved in a similar manner. |
Yes, in most cases you want to stop the entire app, so that's the default behavior introduced by this PR. You can still return |
I'm curious if there's ever really a case where you'd want to recover from the panic though @AaronErhardt. I know just imagining about it that there could be a possibility for it, but I can't really think of a concrete use case where you would want to do such (at least on a singular component level like in here). |
Let's say you use some crate which has a bad API that panics instead of returning an error (plotters would be a prominent example). You could a worker or component that manages the export of the plot to a file and if a panics happens you can catch it and continue the execution. Or you want to execute some additional code, for example for recording the application state and only then you want to actually shut down the app. |
I'm still a bit questionable on how needed this should be, I feel like there isn't really a case where it provides much use.
I feel like this kind of stuff should be handled in application code itself, e.g. via the use of enum AppOutput {
...
PanicErr(Err)
}
struct AppModel;
impl Component for AppModel {
...
fn update(...) {
if let Err(err) = catch_unwind(|| { ... }) {
sender.output(AppOutput::PanicErr(err));
}
} And then any panics outside of that case should just crash the app in my opinion. I can't really see a useful case outside of that. |
bae4fe5
to
119b664
Compare
I think you're right, catching a panic should be handled by the user manually. I've updated the PR now accordingly and extended the functionality to factories. |
Summary
Fixes #578
@hwittenborn
Checklist