FreeLanguageTools / vocabsieve

Simple sentence mining tool for language learning
GNU General Public License v3.0
374 stars 29 forks source link

Introduce a general error handler #129

Closed RasmusRendal closed 8 months ago

RasmusRendal commented 8 months ago

Uncaught exceptions will be caught by this handler, and it will present a dialog with a stack trace, but also still print the error to the console.

Also, make issues fetching forvo clips throw exceptions, which is a great way to test it.

Being able to see when something goes wrong is really a great usability improvement imho

1over137 commented 8 months ago

Looks good, but we should probably use the logger rather than printing so it can be seen more easily by people using it without a console. I notice that a lot of exceptions are getting caught at earlier levels, maybe some of that should be removed (especially by the QTimers it seems)

RasmusRendal commented 8 months ago

I notice that a lot of exceptions are getting caught at earlier levels, maybe some of that should be removed (especially by the QTimers it seems)

I still think that catching exceptions other places is ideal. In many cases I think the following pattern'd be good:

try:
   get_forvo_audio():
except NetworkException:
    logger.exception("Error fetching audio")
    QMessageBox.Critical("Error fetching audio", "Could not connect to forvo.com. Please check that your network connection is up, and forvo.com is reachable")

Giving the error messages locally allows us to do something that's a bit more userfriendly, than long stacktraces.