Skip to content
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

Better global error handling #3491

Closed
stefanhaller opened this issue Apr 11, 2024 · 0 comments · Fixed by #3502
Closed

Better global error handling #3491

stefanhaller opened this issue Apr 11, 2024 · 0 comments · Fixed by #3502

Comments

@stefanhaller
Copy link
Collaborator

Right now, all entry points into the codebase (e.g. event handlers) need to catch errors and present them in an error panel, so we have tons of calls to self.c.Error(err) in the controller and helper classes. It's easy to forget this, however, and simply return the error instead, which will cause a panic in gocui (see #3490 for an example).

It would be much nicer if we could simply return errors everywhere, and gocui would take care of showing an error box after handling an event that returned an error. To do this gocui.Gui just needs an error callback, this seems very easy (see also the last paragraph of this long comment, starting with "As for surfacing the error..."). It seems we could then hopefully get rid of IPopupHandler.Error.

stefanhaller added a commit that referenced this issue Apr 18, 2024
- **PR Description**

Simplify and canonicalize error handling across the code base.

Previously it was important to make sure that errors don't bubble up
into gocui, because it would panic there; so we had to show errors in
error panels in the right places (in controller code, usually). This is
error-prone because it's easy to forget (see #3490 for an example);
also, it's not always obvious where in the call chain the error panel
needs to be shown. Most of the time it's in controller code, but we had
plenty of calls to `Error()` in helpers, and I remember that I found
this very confusing when I was new to the code base.

Change this so that you can simply return errors everywhere. The
functions to show an error message manually have been removed for
clarity.

I tried to structure the commits so that you can review them one by one.

Closes #3491.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant