DavidoTek / ProtonUp-Qt

Install and manage GE-Proton, Luxtorpeda & more for Steam and Wine-GE & more for Lutris with this graphical user interface.
https://davidotek.github.io/protonup-qt
GNU General Public License v3.0
1.18k stars 39 forks source link

Add GUI dialog for startup errors #247

Closed JohanAR closed 7 months ago

JohanAR commented 1 year ago

Is your feature request related to a problem? Please describe. I'm pre-installing some useful stuff on my wife's new computer, and it seemed like nothing happened when I tried to run ProtonUp-Qt (appimage started from file manager). When I instead start it from a terminal, I see that it crashed due to non-existing libraryfolders.vdf. I'm guessing the file doesn't exist because she hasn't logged in to Steam yet. However it would've been nice with some graphical feedback.

Describe the solution you'd like If an error/exception happens during startup, a popup dialog window is shown with the error message + stack trace.

Describe alternatives you've considered A popup window saying "An unexpected error occurred, check program logs", preferably with a link to the log file location, would also be ok.

Additional context N/A

sonic2kk commented 1 year ago

Some error dialogs are already in place for ctmods failing to load (can't find the PR atm) but it would be useful to have more, I agree.

The specific crash you're referring to may be fixed in the next release, but cases where files can't be loaded etc should probably be shown in a dialog to the user. Even cases where ctmods fail to install could have their own dialogs (right now the errors are caught iirc but not shown visually to the user).

DavidoTek commented 1 year ago

If an error/exception happens during startup, a popup dialog window is shown with the error message + stack trace.

I thought about that. It is possible to add some sort of wrapper script that executes the Python code and if it fails, opens a message box with the error.

Some error dialogs are already in place for ctmods failing to load (can't find the PR atm)

We mostly cover internal errors. Problems causing Python/Qt to completely crash are not covered.

but it would be useful to have more, I agree.

I agree, having a message box for everything that could obviously fail could make sense. Though we need to be clear about whether a message box tries to inform the user to do something different (like "oh, your internet connection doesn't work" or "disk is full") or whether it is an internal error (e.g. "something went wrong there. If you can't fix it, please report a bug on GitHub")

loathingKernel commented 7 months ago

I happened to come by this issue, and I would like to offer the example on how we do generic exception handling in Rare, which is also a python Qt application. I hope this helps.

https://github.com/Dummerle/Rare/blob/main/rare/widgets/rare_app.py#L19-L41

DavidoTek commented 7 months ago

I wasn't aware of this method, it seems way cleaner than a wrapper script. I will take a look and implement it. Thanks for letting me know!

loathingKernel commented 7 months ago

Glad to be of help. If you would like to ask me anything feel free to do so. But it should be fairly straight-forward. Some information you might find useful.

The meaty part is obviously monkey-patching sys.excepthook. The class will do that on instantiation. In Rare we keep around the instance in RareApp._hook because later on we replace it with a subclassed instance specific to the different applications in Rare (we have two different applications bundled in one), so that part might not be useful or relevant to you. This works for us in PyQt5 but I expect it to work in PySide6 too, although I haven't tested it. RareAppException._handler is there for our subclasses but it should be useful in case the user wanted to ignore the exception and the application could keep on running. Also IIRC it does work with exceptions coming from threads and runnables.