Improve exception handling #628

samuel-w · 2020-09-08T01:57:15Z

Still exits on uncaught exception, which is same as before.
Should we translate the log message? Just the "Uncaught exception, file a report on GitHub"
Fixes #621

Example, from #619:

2020-09-07 20:52:17,851 - root - ERROR - Uncaught exception, file a report on GitHub:
Traceback (most recent call last):
  File "/home/user/Projects/vorta/src/vorta/application.py", line 90, in open_main_window_action
    self.main_window = MainWindow(self)
  File "/home/user/Projects/vorta/src/vorta/views/main_window.py", line 94, in __init__
    self.set_status(self.tr('Backup in progress.'))
AttributeError: 'MainWindow' object has no attribute 'set_status'

m3nu · 2020-09-08T02:59:55Z

Works as expected

2020-09-08 10:59:17,093 - root - ERROR - Uncaught exception, file a report on GitHub:
Traceback (most recent call last):
  File "/Users/manu/Workspace/vorta/src/vorta/borg/borg_thread.py", line 162, in run
    self.started_event()
  File "/Users/manu/Workspace/vorta/src/vorta/borg/create.py", line 41, in started_event
    raise Exception('Buha')
Exception: Buha
QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread
zsh: segmentation fault  vorta

m3nu · 2020-09-08T03:00:52Z

How about putting the issue URL to make it even easier?

https://github.com/borgbase/vorta/issues/new

This makes it so that exceptions which don't cause a crash not exit, while exceptions that do cause a crash exit. However, it does print to console twice, which might be weird.

samuel-w · 2020-09-08T03:17:58Z

The issue I'm having is some exceptions don't cause a crash, while others do cause a crash, and I can't differentiate them.

Should we always crash on uncaught exceptions?

…eptions

m3nu · 2020-09-08T03:29:00Z

Which exceptions don't crash? Maybe they are caught somewhere? I wouldn't crash the app more often than we have to.

Another idea would be to open a window showing the exception and offering to direct it to Github? (prefill?)

src/vorta/__main__.py

This reverts commit 916bd49.

samuel-w · 2020-09-08T06:40:44Z

Now it double prints the exception to the terminal. Should be fine for reading bug reports.

samuel-w · 2020-09-08T06:51:52Z

I've decided to always close on uncaught exception because it makes the behaviour more consistent across platforms, such as #613 and #521, where one crashes and the other does not, but both lead to undesired behaviour.

samuel-w · 2020-09-08T06:54:30Z

src/vorta/__main__.py

+    logger.critical("Uncaught exception, file a report at https://github.com/borgbase/vorta/issues/new:",
+                    exc_info=(type, value, tb))
+    sys.__excepthook__(type, value, tb)
+    sys.exit(1)


Remove this line to make uncaught exceptions never crash, may lead to undesired behaviour.

Fine. Lets crash it all the time. At least we get bug reports quickly.

I've made the exception print a notification. Should I still create a crash?

samuel-w · 2020-09-08T07:22:17Z

Sample notification:

m3nu · 2020-09-08T07:26:20Z

How about a real UI window in Qt that has the exception and Github link?

Else we start with just adding it to the logs. Not sure a notification is useful since it can't be clicked and may be disabled.

samuel-w · 2020-09-08T07:46:14Z

This is what is displayed now, exception is generated:

It only happens when the exception is in the foreground, if its in the background it just prints to log.
Text can be copied.

m3nu · 2020-09-08T08:06:51Z

Good solution. Last suggestion: Move the "File a bug report on github.com/..." into the alert, so it can be a real link? Then only the actual exception is logged.

samuel-w · 2020-09-08T08:10:57Z

It gets cutoff though.

m3nu · 2020-09-08T08:18:40Z

It gets cutoff though.

Better add it inside the message, rather than as title. Title could be "Uncaught exception" or something?

samuel-w · 2020-09-08T08:29:13Z

Adding a link destroys formatting.

m3nu · 2020-09-08T08:31:51Z

Because HTML removes line breaks unless there is a <br>.

Still possible to make the alert a more complex UI element with two separate text widgets. Probably even possible programmatically without doing a .ui file.

samuel-w · 2020-09-08T08:48:44Z

There is a way to automate filling out a bug report with https://github.com/borgbase/vorta/issues/new?title=&body=. We could use that, but it seems like too much work compared to copying and pasting.

Edit: Especially for something that most users aren't going to see

This reverts commit 8379d83.

m3nu · 2020-09-08T09:36:47Z

I expected there to be some URL parameters, like for mailto links. I think for now it's already a great improvement if those exceptions end up a) in the logs and b) in a message window when a crash happens.

src/vorta/__main__.py

Because we have a message pop up now there is no need to close, unless Vorta is in the background, where we close to prevent it from freezing up and having to be force closed

codecov-io · 2020-12-13T06:59:36Z

Codecov Report

Merging #628 (0a24aea) into master (19b4946) will decrease coverage by 0.21%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #628      +/-   ##
==========================================
- Coverage   74.48%   74.27%   -0.22%     
==========================================
  Files          53       53              
  Lines        3422     3432      +10     
==========================================
  Hits         2549     2549              
- Misses        873      883      +10

Impacted Files	Coverage Δ
src/vorta/__main__.py	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19b4946...0a24aea. Read the comment docs.

samuel-w added 2 commits September 7, 2020 20:52

Log exceptions

c2c0e01

Merge branch 'master' into LogExeceptions

364e508

samuel-w marked this pull request as draft September 8, 2020 02:56

samuel-w and others added 2 commits September 7, 2020 22:15

Don't exit on exception

f5499c9

This makes it so that exceptions which don't cause a crash not exit, while exceptions that do cause a crash exit. However, it does print to console twice, which might be weird.

Merge branch 'master' into LogExeceptions

4347149

samuel-w added 2 commits September 7, 2020 22:18

Raise exception log level

164d2e0

Merge branch 'LogExeceptions' of github:samuel-w/vorta-1 into LogExec…

001ac77

…eptions

samuel-w added 2 commits September 7, 2020 22:47

Filter out exceptions from stdout

916bd49

Add link to bug reporting page

12042c2

samuel-w commented Sep 8, 2020

View reviewed changes

src/vorta/__main__.py Outdated Show resolved Hide resolved

samuel-w added 3 commits September 8, 2020 01:25

Move hook to main

94b389e

Uncomment original exception hook

1ea58bb

Revert "Filter out exceptions from stdout"

fd9156c

This reverts commit 916bd49.

samuel-w marked this pull request as ready for review September 8, 2020 06:40

samuel-w marked this pull request as draft September 8, 2020 06:43

Always close on uncaught exception

a1db6d5

samuel-w commented Sep 8, 2020

View reviewed changes

samuel-w marked this pull request as ready for review September 8, 2020 06:54

samuel-w added 4 commits September 8, 2020 02:07

Display notification on exception

4d629de

Make exception notification its own level

fccdac1

Improve messages

78a8bc6

Be nice

8812b69

Display message box on exception

e0728c7

samuel-w added 2 commits September 8, 2020 02:48

Cleanup

5aaa175

Change box level

990af34

samuel-w changed the title ~~Log exceptions in log~~ Improve exception handling Sep 8, 2020

Formatting

eb63a4f

Add URL, formatting is broken

8379d83

Revert "Add URL, formatting is broken"

0ff076c

This reverts commit 8379d83.

Remove second exception print

bd5cbcf

m3nu reviewed Sep 9, 2020

View reviewed changes

src/vorta/__main__.py Outdated Show resolved Hide resolved

m3nu and others added 5 commits September 9, 2020 08:48

Merge branch 'master' into LogExeceptions

7c621ef

Don't exit on unhandled exception

6e10bad

Because we have a message pop up now there is no need to close, unless Vorta is in the background, where we close to prevent it from freezing up and having to be force closed

Merge branch 'master' into LogExeceptions

fe0f74c

Merge branch 'master' into LogExeceptions

dde354a

Print messagebox without parent

5f0120b

samuel-w mentioned this pull request Oct 6, 2020

Log borg commands to log files with verbosity #678

Closed

Merge branch 'master' into LogExeceptions

0a24aea

m3nu self-assigned this Dec 15, 2020

Merge branch 'master' into LogExeceptions

64dd71e

m3nu approved these changes Dec 16, 2020

View reviewed changes

m3nu merged commit d6f368a into borgbase:master Dec 16, 2020

samuel-w deleted the LogExeceptions branch December 16, 2020 04:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve exception handling #628

Improve exception handling #628

samuel-w commented Sep 8, 2020 •

edited

Loading

m3nu commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020

samuel-w commented Sep 8, 2020

samuel-w Sep 8, 2020

m3nu Sep 8, 2020

samuel-w Sep 8, 2020

samuel-w commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020 •

edited

Loading

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020 •

edited

Loading

m3nu commented Sep 8, 2020

codecov-io commented Dec 13, 2020 •

edited

Loading

Improve exception handling #628

Improve exception handling #628

Conversation

samuel-w commented Sep 8, 2020 • edited Loading

m3nu commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020

samuel-w commented Sep 8, 2020

samuel-w Sep 8, 2020

Choose a reason for hiding this comment

m3nu Sep 8, 2020

Choose a reason for hiding this comment

samuel-w Sep 8, 2020

Choose a reason for hiding this comment

samuel-w commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020 • edited Loading

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020

m3nu commented Sep 8, 2020

samuel-w commented Sep 8, 2020 • edited Loading

m3nu commented Sep 8, 2020

codecov-io commented Dec 13, 2020 • edited Loading

Codecov Report

samuel-w commented Sep 8, 2020 •

edited

Loading

samuel-w commented Sep 8, 2020 •

edited

Loading

samuel-w commented Sep 8, 2020 •

edited

Loading

codecov-io commented Dec 13, 2020 •

edited

Loading