From 29fdacc8ca5195485c286be3a1a680ab01780d55 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Fri, 31 Jan 2020 09:00:04 +0100 Subject: [PATCH 1/7] PICARD-1718: Try to log and show exception details on crash If Picard crashes try to log the traceback to a file and show a dialog to the user before exiting. --- scripts/picard.in | 31 +++++++++++++++++++++++++++++-- tagger.py.in | 28 ++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/scripts/picard.in b/scripts/picard.in index 7b67f8ecf..70b7dd4df 100644 --- a/scripts/picard.in +++ b/scripts/picard.in @@ -1,3 +1,30 @@ #!/usr/bin/env python3 -from picard.tagger import main -main('%(localedir)s', %(autoupdate)s) + +try: + from picard.tagger import main + main('%(localedir)s', %(autoupdate)s) +except SystemExit: + raise # Just continue with a normal application exit +except: # noqa: F722 + # First try to get traceback information and write it to a log file + # with minimum chance to fail. + import os + import sys + import tempfile + import traceback + trace = traceback.format_exc() + logfile = None + try: + (fd, logfile) = tempfile.mkstemp(".log", "picard-crash-") + os.write(fd, trace.encode(errors="replace")) + os.close(fd) + except: # noqa: F722 + print("Failed writing log file {0}".format(logfile), file=sys.stderr) + + # Display the crash information to the user as a dialog. This requires + # importing Qt5 and has some potential to fail if things are broken. + from PyQt5 import QtWidgets + app = QtWidgets.QApplication(sys.argv) + msg = "A logfile has been written to {0}\n\nError details:\n{1}".format(logfile, trace) + QtWidgets.QMessageBox.critical(None, "Picard terminated unexpectedly", msg) + raise diff --git a/tagger.py.in b/tagger.py.in index 35cb4f204..b54de29b2 100644 --- a/tagger.py.in +++ b/tagger.py.in @@ -15,5 +15,29 @@ else: if sys.platform == 'win32': os.environ['PATH'] = basedir + ';' + os.environ['PATH'] -from picard.tagger import main -main(os.path.join(basedir, 'locale'), %(autoupdate)s) +try: + from picard.tagger import main + main(os.path.join(basedir, 'locale'), %(autoupdate)s) +except SystemExit: + raise # Just continue with a normal application exit +except: # noqa: F722 + # First try to get traceback information and write it to a log file + # with minimum chance to fail. + import tempfile + import traceback + trace = traceback.format_exc() + logfile = None + try: + (fd, logfile) = tempfile.mkstemp(".log", "picard-crash-") + os.write(fd, trace.encode(errors="replace")) + os.close(fd) + except: # noqa: F722 + print("Failed writing log file {0}".format(logfile), file=sys.stderr) + + # Display the crash information to the user as a dialog. This requires# + # importing Qt5 and has some potential to fail if things are broken. + from PyQt5 import QtWidgets + app = QtWidgets.QApplication(sys.argv) + msg = "A logfile has been written to {0}\n\nError details:\n{1}".format(logfile, trace) + QtWidgets.QMessageBox.critical(None, "Picard terminated unexpectedly", msg) + raise From b9a7219ecd26645c0b6f5d1a79e0654df28f3155 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 10 Feb 2020 19:08:27 +0100 Subject: [PATCH 2/7] PICARD-1718: Do not print logfile location if writing log failed --- scripts/picard.in | 9 ++++++--- tagger.py.in | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/scripts/picard.in b/scripts/picard.in index 70b7dd4df..741bb1a4f 100644 --- a/scripts/picard.in +++ b/scripts/picard.in @@ -13,18 +13,21 @@ except: # noqa: F722 import tempfile import traceback trace = traceback.format_exc() - logfile = None try: (fd, logfile) = tempfile.mkstemp(".log", "picard-crash-") os.write(fd, trace.encode(errors="replace")) os.close(fd) except: # noqa: F722 print("Failed writing log file {0}".format(logfile), file=sys.stderr) + logfile = None # Display the crash information to the user as a dialog. This requires # importing Qt5 and has some potential to fail if things are broken. from PyQt5 import QtWidgets app = QtWidgets.QApplication(sys.argv) - msg = "A logfile has been written to {0}\n\nError details:\n{1}".format(logfile, trace) - QtWidgets.QMessageBox.critical(None, "Picard terminated unexpectedly", msg) + msg = [] + if logfile: + msg.append("A logfile has been written to {0}".format(logfile)) + msg.append("Error details: \n{0}".format(trace)) + QtWidgets.QMessageBox.critical(None, "Picard terminated unexpectedly", "\n\n".join(msg)) raise diff --git a/tagger.py.in b/tagger.py.in index b54de29b2..34270d288 100644 --- a/tagger.py.in +++ b/tagger.py.in @@ -26,18 +26,21 @@ except: # noqa: F722 import tempfile import traceback trace = traceback.format_exc() - logfile = None try: (fd, logfile) = tempfile.mkstemp(".log", "picard-crash-") os.write(fd, trace.encode(errors="replace")) os.close(fd) except: # noqa: F722 print("Failed writing log file {0}".format(logfile), file=sys.stderr) + logfile = None # Display the crash information to the user as a dialog. This requires# # importing Qt5 and has some potential to fail if things are broken. from PyQt5 import QtWidgets app = QtWidgets.QApplication(sys.argv) - msg = "A logfile has been written to {0}\n\nError details:\n{1}".format(logfile, trace) - QtWidgets.QMessageBox.critical(None, "Picard terminated unexpectedly", msg) + msg = [] + if logfile: + msg.append("A logfile has been written to {0}".format(logfile)) + msg.append("Error details: \n{0}".format(trace)) + QtWidgets.QMessageBox.critical(None, "Picard terminated unexpectedly", "\n\n".join(msg)) raise From 70e6dfd2333295e48dbe24807cf6f0b6ac3643b2 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 10 Feb 2020 19:15:03 +0100 Subject: [PATCH 3/7] PICARD-1718: Use NamedTemporaryFile for creating log file --- scripts/picard.in | 8 ++++---- tagger.py.in | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/picard.in b/scripts/picard.in index 741bb1a4f..d6d7f8897 100644 --- a/scripts/picard.in +++ b/scripts/picard.in @@ -10,13 +10,13 @@ except: # noqa: F722 # with minimum chance to fail. import os import sys - import tempfile + from tempfile import NamedTemporaryFile import traceback trace = traceback.format_exc() try: - (fd, logfile) = tempfile.mkstemp(".log", "picard-crash-") - os.write(fd, trace.encode(errors="replace")) - os.close(fd) + with NamedTemporaryFile(suffix='.log', prefix='picard-crash-', delete=False) as f: + f.write(trace.encode(errors="replace")) + logfile = f.name except: # noqa: F722 print("Failed writing log file {0}".format(logfile), file=sys.stderr) logfile = None diff --git a/tagger.py.in b/tagger.py.in index 34270d288..39cbb2548 100644 --- a/tagger.py.in +++ b/tagger.py.in @@ -23,13 +23,13 @@ except SystemExit: except: # noqa: F722 # First try to get traceback information and write it to a log file # with minimum chance to fail. - import tempfile + from tempfile import NamedTemporaryFile import traceback trace = traceback.format_exc() try: - (fd, logfile) = tempfile.mkstemp(".log", "picard-crash-") - os.write(fd, trace.encode(errors="replace")) - os.close(fd) + with NamedTemporaryFile(suffix='.log', prefix='picard-crash-', delete=False) as f: + f.write(trace.encode(errors="replace")) + logfile = f.name except: # noqa: F722 print("Failed writing log file {0}".format(logfile), file=sys.stderr) logfile = None From 0c5d908a4c0c1ec5cf75d94376c3d335fd2f6e17 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Tue, 11 Feb 2020 08:14:44 +0100 Subject: [PATCH 4/7] PICARD-1718: Show more details in crash dialog Also set stack trace into the detailedText. Nicer display and ensures eventual HTML inside this string does not get rendered. --- scripts/picard.in | 24 ++++++++++++++++++------ tagger.py.in | 24 ++++++++++++++++++------ 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/scripts/picard.in b/scripts/picard.in index d6d7f8897..a4535c8e8 100644 --- a/scripts/picard.in +++ b/scripts/picard.in @@ -23,11 +23,23 @@ except: # noqa: F722 # Display the crash information to the user as a dialog. This requires # importing Qt5 and has some potential to fail if things are broken. - from PyQt5 import QtWidgets - app = QtWidgets.QApplication(sys.argv) - msg = [] + from PyQt5.QtCore import Qt, QUrl + from PyQt5.QtWidgets import QApplication, QMessageBox + _unused = QApplication(sys.argv) + msgbox = QMessageBox() + msgbox.setIcon(QMessageBox.Critical) + msgbox.setWindowTitle("Picard terminated unexpectedly") + msgbox.setTextFormat(Qt.RichText) + msgbox.setText( + 'An unexpected error has caused Picard to crash. ' + 'Please report this issue on the MusicBrainz bug tracker.') if logfile: - msg.append("A logfile has been written to {0}".format(logfile)) - msg.append("Error details: \n{0}".format(trace)) - QtWidgets.QMessageBox.critical(None, "Picard terminated unexpectedly", "\n\n".join(msg)) + logfile_url = QUrl.fromLocalFile(logfile) + msgbox.setInformativeText( + 'A logfile has been written to {1}.' + .format(logfile_url.url(), logfile)) + msgbox.setDetailedText(trace) + msgbox.setStandardButtons(QMessageBox.Close) + msgbox.setDefaultButton(QMessageBox.Close) + msgbox.exec_() raise diff --git a/tagger.py.in b/tagger.py.in index 39cbb2548..fc0c08e74 100644 --- a/tagger.py.in +++ b/tagger.py.in @@ -36,11 +36,23 @@ except: # noqa: F722 # Display the crash information to the user as a dialog. This requires# # importing Qt5 and has some potential to fail if things are broken. - from PyQt5 import QtWidgets - app = QtWidgets.QApplication(sys.argv) - msg = [] + from PyQt5.QtCore import Qt, QUrl + from PyQt5.QtWidgets import QApplication, QMessageBox + _unused = QApplication(sys.argv) + msgbox = QMessageBox() + msgbox.setIcon(QMessageBox.Critical) + msgbox.setWindowTitle("Picard terminated unexpectedly") + msgbox.setTextFormat(Qt.RichText) + msgbox.setText( + 'An unexpected error has caused Picard to crash. ' + 'Please report this issue on the MusicBrainz bug tracker.') if logfile: - msg.append("A logfile has been written to {0}".format(logfile)) - msg.append("Error details: \n{0}".format(trace)) - QtWidgets.QMessageBox.critical(None, "Picard terminated unexpectedly", "\n\n".join(msg)) + logfile_url = QUrl.fromLocalFile(logfile) + msgbox.setInformativeText( + 'A logfile has been written to {1}.' + .format(logfile_url.url(), logfile)) + msgbox.setDetailedText(trace) + msgbox.setStandardButtons(QMessageBox.Close) + msgbox.setDefaultButton(QMessageBox.Close) + msgbox.exec_() raise From c960089b80ebb4f8d10611b41c452c6879d40c91 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Fri, 30 Apr 2021 14:39:57 +0200 Subject: [PATCH 5/7] PICARD-1718: Moved crash handler code into central function Avoid code duplication, by still having a rather minimal dependency of internal code. --- picard/__init__.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ scripts/picard.in | 40 +++------------------------------------- tagger.py.in | 38 +++----------------------------------- 3 files changed, 51 insertions(+), 72 deletions(-) diff --git a/picard/__init__.py b/picard/__init__.py index 5458ca47e..f936c52f3 100644 --- a/picard/__init__.py +++ b/picard/__init__.py @@ -87,3 +87,48 @@ api_versions = [ ] api_versions_tuple = [Version.from_string(v) for v in api_versions] + + +def crash_handler(): + """Implements minimal handling of an exception crashing the application. + This function tries to log the exception to a log file and display + a minimal crash dialog to the user. + This function is supposed to be called from inside an except blog. + """ + # First try to get traceback information and write it to a log file + # with minimum chance to fail. + import sys + from tempfile import NamedTemporaryFile + import traceback + trace = traceback.format_exc() + logfile = None + try: + with NamedTemporaryFile(suffix='.log', prefix='picard-crash-', delete=False) as f: + f.write(trace.encode(errors="replace")) + logfile = f.name + except: # noqa: E722,F722 # pylint: disable=bare-except + print("Failed writing log file {0}".format(logfile), file=sys.stderr) + logfile = None + + # Display the crash information to the user as a dialog. This requires + # importing Qt5 and has some potential to fail if things are broken. + from PyQt5.QtCore import Qt, QUrl + from PyQt5.QtWidgets import QApplication, QMessageBox + # assigning QApplication to a variable is required to keep the object alive + _unused = QApplication(sys.argv) # noqa: F841 + msgbox = QMessageBox() + msgbox.setIcon(QMessageBox.Critical) + msgbox.setWindowTitle("Picard terminated unexpectedly") + msgbox.setTextFormat(Qt.RichText) + msgbox.setText( + 'An unexpected error has caused Picard to crash. ' + 'Please report this issue on the MusicBrainz bug tracker.') + if logfile: + logfile_url = QUrl.fromLocalFile(logfile) + msgbox.setInformativeText( + 'A logfile has been written to {1}.' + .format(logfile_url.url(), logfile)) + msgbox.setDetailedText(trace) + msgbox.setStandardButtons(QMessageBox.Close) + msgbox.setDefaultButton(QMessageBox.Close) + msgbox.exec_() diff --git a/scripts/picard.in b/scripts/picard.in index a4535c8e8..ab04f5546 100644 --- a/scripts/picard.in +++ b/scripts/picard.in @@ -5,41 +5,7 @@ try: main('%(localedir)s', %(autoupdate)s) except SystemExit: raise # Just continue with a normal application exit -except: # noqa: F722 - # First try to get traceback information and write it to a log file - # with minimum chance to fail. - import os - import sys - from tempfile import NamedTemporaryFile - import traceback - trace = traceback.format_exc() - try: - with NamedTemporaryFile(suffix='.log', prefix='picard-crash-', delete=False) as f: - f.write(trace.encode(errors="replace")) - logfile = f.name - except: # noqa: F722 - print("Failed writing log file {0}".format(logfile), file=sys.stderr) - logfile = None - - # Display the crash information to the user as a dialog. This requires - # importing Qt5 and has some potential to fail if things are broken. - from PyQt5.QtCore import Qt, QUrl - from PyQt5.QtWidgets import QApplication, QMessageBox - _unused = QApplication(sys.argv) - msgbox = QMessageBox() - msgbox.setIcon(QMessageBox.Critical) - msgbox.setWindowTitle("Picard terminated unexpectedly") - msgbox.setTextFormat(Qt.RichText) - msgbox.setText( - 'An unexpected error has caused Picard to crash. ' - 'Please report this issue on the MusicBrainz bug tracker.') - if logfile: - logfile_url = QUrl.fromLocalFile(logfile) - msgbox.setInformativeText( - 'A logfile has been written to {1}.' - .format(logfile_url.url(), logfile)) - msgbox.setDetailedText(trace) - msgbox.setStandardButtons(QMessageBox.Close) - msgbox.setDefaultButton(QMessageBox.Close) - msgbox.exec_() +except: # noqa: E722,F722 # pylint: disable=bare-except + from picard import crash_handler + crash_handler() raise diff --git a/tagger.py.in b/tagger.py.in index fc0c08e74..ceaf22262 100644 --- a/tagger.py.in +++ b/tagger.py.in @@ -20,39 +20,7 @@ try: main(os.path.join(basedir, 'locale'), %(autoupdate)s) except SystemExit: raise # Just continue with a normal application exit -except: # noqa: F722 - # First try to get traceback information and write it to a log file - # with minimum chance to fail. - from tempfile import NamedTemporaryFile - import traceback - trace = traceback.format_exc() - try: - with NamedTemporaryFile(suffix='.log', prefix='picard-crash-', delete=False) as f: - f.write(trace.encode(errors="replace")) - logfile = f.name - except: # noqa: F722 - print("Failed writing log file {0}".format(logfile), file=sys.stderr) - logfile = None - - # Display the crash information to the user as a dialog. This requires# - # importing Qt5 and has some potential to fail if things are broken. - from PyQt5.QtCore import Qt, QUrl - from PyQt5.QtWidgets import QApplication, QMessageBox - _unused = QApplication(sys.argv) - msgbox = QMessageBox() - msgbox.setIcon(QMessageBox.Critical) - msgbox.setWindowTitle("Picard terminated unexpectedly") - msgbox.setTextFormat(Qt.RichText) - msgbox.setText( - 'An unexpected error has caused Picard to crash. ' - 'Please report this issue on the MusicBrainz bug tracker.') - if logfile: - logfile_url = QUrl.fromLocalFile(logfile) - msgbox.setInformativeText( - 'A logfile has been written to {1}.' - .format(logfile_url.url(), logfile)) - msgbox.setDetailedText(trace) - msgbox.setStandardButtons(QMessageBox.Close) - msgbox.setDefaultButton(QMessageBox.Close) - msgbox.exec_() +except: # noqa: E722,F722 # pylint: disable=bare-except + from picard import crash_handler + crash_handler() raise From 6be39d1be4afaf25dddba67c027f95f082023e9d Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Wed, 5 May 2021 14:53:08 +0200 Subject: [PATCH 6/7] PICARD-1718: Cleanly shutdown crash handler app --- picard/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/picard/__init__.py b/picard/__init__.py index f936c52f3..2c8a7c36e 100644 --- a/picard/__init__.py +++ b/picard/__init__.py @@ -115,7 +115,7 @@ def crash_handler(): from PyQt5.QtCore import Qt, QUrl from PyQt5.QtWidgets import QApplication, QMessageBox # assigning QApplication to a variable is required to keep the object alive - _unused = QApplication(sys.argv) # noqa: F841 + app = QApplication(sys.argv) msgbox = QMessageBox() msgbox.setIcon(QMessageBox.Critical) msgbox.setWindowTitle("Picard terminated unexpectedly") @@ -132,3 +132,4 @@ def crash_handler(): msgbox.setStandardButtons(QMessageBox.Close) msgbox.setDefaultButton(QMessageBox.Close) msgbox.exec_() + app.quit() From 7330425de1cb8e0793e7b59f2fddc0b93c94e307 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Fri, 7 May 2021 13:22:52 +0200 Subject: [PATCH 7/7] PICARD-1718: Crash handle must not create additional QApplication instance --- picard/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/picard/__init__.py b/picard/__init__.py index 2c8a7c36e..5b726a05d 100644 --- a/picard/__init__.py +++ b/picard/__init__.py @@ -112,10 +112,11 @@ def crash_handler(): # Display the crash information to the user as a dialog. This requires # importing Qt5 and has some potential to fail if things are broken. - from PyQt5.QtCore import Qt, QUrl + from PyQt5.QtCore import QCoreApplication, Qt, QUrl from PyQt5.QtWidgets import QApplication, QMessageBox - # assigning QApplication to a variable is required to keep the object alive - app = QApplication(sys.argv) + app = QCoreApplication.instance() + if not app: + app = QApplication(sys.argv) msgbox = QMessageBox() msgbox.setIcon(QMessageBox.Critical) msgbox.setWindowTitle("Picard terminated unexpectedly")