From 6da177471b78a210cf2a7768245c370ff034deda Mon Sep 17 00:00:00 2001 From: Timmy Welch Date: Mon, 11 Apr 2022 14:52:41 -0700 Subject: [PATCH 1/2] Fix #242 Fix file encoding inconsistencies, windows defaults to cp1252, which is not Unicode compatible. Add logging for all exceptions in the comicapi package Ensure that all exceptions are logged and shown to the user --- comicapi/comicarchive.py | 32 ++++++++++---------- comicapi/comicbookinfo.py | 2 +- comictaggerlib/comicvinecacher.py | 4 +-- comictaggerlib/imagefetcher.py | 2 +- comictaggerlib/main.py | 50 +++++++++++++++++++++++++++++-- comictaggerlib/settings.py | 4 +-- requirements_dev.txt | 1 + setup.py | 2 +- 8 files changed, 72 insertions(+), 25 deletions(-) diff --git a/comicapi/comicarchive.py b/comicapi/comicarchive.py index 022a9ac..ebd335c 100644 --- a/comicapi/comicarchive.py +++ b/comicapi/comicarchive.py @@ -92,6 +92,7 @@ class SevenZipArchiver: try: self.rebuild_zip_file([archive_file]) except: + logger.exception("Failed to remove %s from 7zip archive", archive_file) return False else: return True @@ -110,6 +111,7 @@ class SevenZipArchiver: zf.writestr(data, archive_file) return True except: + logger.exception("Writing zip file failed") return False def get_filename_list(self): @@ -194,6 +196,7 @@ class ZipArchiver: try: self.rebuild_zip_file([archive_file]) except: + logger.exception("Failed to remove %s from zip archive", archive_file) return False else: return True @@ -306,7 +309,7 @@ class ZipArchiver: else: raise Exception("Failed to write comment to zip file!") except Exception: - logger.exception() + logger.exception("Writing comment to %s failed", filename) return False else: return True @@ -342,7 +345,7 @@ class RarArchiver: self.rar_exe_path = rar_exe_path if RarArchiver.devnull is None: - RarArchiver.devnull = open(os.devnull, "w") + RarArchiver.devnull = open(os.devnull, "bw") # windows only, keeps the cmd.exe from popping up if platform.system() == "Windows": @@ -360,9 +363,8 @@ class RarArchiver: try: # write comment to temp file tmp_fd, tmp_name = tempfile.mkstemp() - f = os.fdopen(tmp_fd, "w+") - f.write(comment) - f.close() + with os.fdopen(tmp_fd, "wb") as f: + f.write(comment.encode("utf-8")) working_dir = os.path.dirname(os.path.abspath(self.path)) @@ -441,7 +443,7 @@ class RarArchiver: # TODO: will this break if 'archive_file' is in a subfolder. i.e. "foo/bar.txt" # will need to create the subfolder above, I guess... - with open(tmp_file, "w") as f: + with open(tmp_file, "wb") as f: f.write(data) # use external program to write file to Rar archive @@ -457,7 +459,9 @@ class RarArchiver: time.sleep(1) os.remove(tmp_file) os.rmdir(tmp_folder) - except: + except Exception as e: + logger.info(str(e)) + logger.exception("Failed write %s to rar archive", archive_file) return False else: return True @@ -479,6 +483,7 @@ class RarArchiver: if platform.system() == "Darwin": time.sleep(1) except: + logger.exception("Failed to remove %s from rar archive", archive_file) return False else: return True @@ -543,7 +548,6 @@ class FolderArchiver: try: with open(fname, "rb") as f: data = f.read() - f.close() except IOError: logger.exception("Failed to read: %s", fname) @@ -553,11 +557,10 @@ class FolderArchiver: fname = os.path.join(self.path, archive_file) try: - with open(fname, "w+") as f: + with open(fname, "wb") as f: f.write(data) - f.close() except: - logger.exception("Failed to read: %s", fname) + logger.exception("Failed to write: %s", fname) return False else: return True @@ -568,7 +571,7 @@ class FolderArchiver: try: os.remove(fname) except: - logger.exception("Failed to read: %s", fname) + logger.exception("Failed to remove: %s", fname) return False else: return True @@ -976,7 +979,7 @@ class ComicArchive: if raw_cix == "": raw_cix = None cix_string = ComicInfoXml().string_from_metadata(metadata, xml=raw_cix) - write_success = self.archiver.write_file(self.ci_xml_filename, cix_string) + write_success = self.archiver.write_file(self.ci_xml_filename, cix_string.encode("utf-8")) if write_success: self.has__cix = True self.cix_md = metadata @@ -1088,8 +1091,7 @@ class ComicArchive: data = self.archiver.read_file(n) except Exception as e: data = "" - err_msg = f"Error reading in Comet XML for validation!: {e}" - logger.warning(err_msg) + logger.warning("Error reading in Comet XML for validation!: %s", e) if CoMet().validate_string(data): # since we found it, save it! self.comet_filename = n diff --git a/comicapi/comicbookinfo.py b/comicapi/comicbookinfo.py index f24f896..2c8a645 100644 --- a/comicapi/comicbookinfo.py +++ b/comicapi/comicbookinfo.py @@ -119,5 +119,5 @@ class ComicBookInfo: cbi_container = self.create_json_dictionary(metadata) - with open(filename, "w") as f: + with open(filename, "w", encoding="utf-8") as f: f.write(json.dumps(cbi_container, indent=4)) diff --git a/comictaggerlib/comicvinecacher.py b/comictaggerlib/comicvinecacher.py index 2106aad..950c3fe 100644 --- a/comictaggerlib/comicvinecacher.py +++ b/comictaggerlib/comicvinecacher.py @@ -59,11 +59,11 @@ class ComicVineCacher: def create_cache_db(self): # create the version file - with open(self.version_file, "w") as f: + with open(self.version_file, "w", encoding="utf-8") as f: f.write(ctversion.version) # this will wipe out any existing version - open(self.db_file, "w").close() + open(self.db_file, "wb").close() con = lite.connect(self.db_file) diff --git a/comictaggerlib/imagefetcher.py b/comictaggerlib/imagefetcher.py index a799ce7..93704d0 100644 --- a/comictaggerlib/imagefetcher.py +++ b/comictaggerlib/imagefetcher.py @@ -120,7 +120,7 @@ class ImageFetcher: def create_image_db(self): # this will wipe out any existing version - open(self.db_file, "w").close() + open(self.db_file, "wb").close() # wipe any existing image cache folder too if os.path.isdir(self.cache_folder): diff --git a/comictaggerlib/main.py b/comictaggerlib/main.py index f9b632a..422ac94 100755 --- a/comictaggerlib/main.py +++ b/comictaggerlib/main.py @@ -37,8 +37,52 @@ logger.setLevel(logging.DEBUG) try: qt_available = True - from PyQt5 import QtGui, QtWidgets + from PyQt5 import QtCore, QtGui, QtWidgets + def show_exception_box(log_msg): + """Checks if a QApplication instance is available and shows a messagebox with the exception message. + If unavailable (non-console application), log an additional notice. + """ + if QtWidgets.QApplication.instance() is not None: + errorbox = QtWidgets.QMessageBox() + errorbox.setText("Oops. An unexpected error occured:\n{0}".format(log_msg)) + errorbox.exec_() + QtWidgets.QApplication.exit(1) + else: + logger.debug("No QApplication instance available.") + + class UncaughtHook(QtCore.QObject): + _exception_caught = QtCore.pyqtSignal(object) + + def __init__(self, *args, **kwargs): + super(UncaughtHook, self).__init__(*args, **kwargs) + + # this registers the exception_hook() function as hook with the Python interpreter + sys.excepthook = self.exception_hook + + # connect signal to execute the message box function always on main thread + self._exception_caught.connect(show_exception_box) + + def exception_hook(self, exc_type, exc_value, exc_traceback): + """Function handling uncaught exceptions. + It is triggered each time an uncaught exception occurs. + """ + if issubclass(exc_type, KeyboardInterrupt): + # ignore keyboard interrupt to support console applications + sys.__excepthook__(exc_type, exc_value, exc_traceback) + else: + exc_info = (exc_type, exc_value, exc_traceback) + log_msg = "\n".join( + ["".join(traceback.format_tb(exc_traceback)), "{0}: {1}".format(exc_type.__name__, exc_value)] + ) + logger.critical( + "Uncaught exception: %s", "{0}: {1}".format(exc_type.__name__, exc_value), exc_info=exc_info + ) + + # trigger message box show + self._exception_caught.emit(log_msg) + + qt_exception_hook = UncaughtHook() from comictaggerlib.taggerwindow import TaggerWindow except ImportError as e: logging.debug(e) @@ -106,7 +150,7 @@ def ctmain(): try: cli.cli_mode(opts, SETTINGS) except: - logger.exception() + logger.exception("CLI mode failed") else: os.environ["QtWidgets.QT_AUTO_SCREEN_SCALE_FACTOR"] = "1" args = [] @@ -149,7 +193,7 @@ def ctmain(): sys.exit(app.exec()) except Exception: - logger.exception() + logger.exception("GUI mode failed") QtWidgets.QMessageBox.critical( QtWidgets.QMainWindow(), "Error", "Unhandled exception in app:\n" + traceback.format_exc() ) diff --git a/comictaggerlib/settings.py b/comictaggerlib/settings.py index fefdd16..0af8402 100644 --- a/comictaggerlib/settings.py +++ b/comictaggerlib/settings.py @@ -239,7 +239,7 @@ class ComicTaggerSettings: yield line line = f.readline() - with open(self.settings_file, "r") as f: + with open(self.settings_file, "r", encoding="utf-8") as f: self.config.read_file(readline_generator(f)) self.rar_exe_path = self.config.get("settings", "rar_exe_path") @@ -451,5 +451,5 @@ class ComicTaggerSettings: self.config.set("autotag", "remove_archive_after_successful_match", self.remove_archive_after_successful_match) self.config.set("autotag", "wait_and_retry_on_rate_limit", self.wait_and_retry_on_rate_limit) - with open(self.settings_file, "w") as configfile: + with open(self.settings_file, "w", encoding="utf-8") as configfile: self.config.write(configfile) diff --git a/requirements_dev.txt b/requirements_dev.txt index b2f34bc..5f025ae 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -4,4 +4,5 @@ setuptools_scm[toml]>=3.4 wheel black>=22 flake8==4.* +flake8-encodings isort>=5.10 diff --git a/setup.py b/setup.py index fcebbaf..a0f302f 100644 --- a/setup.py +++ b/setup.py @@ -24,7 +24,7 @@ def read(fname): str File contents. """ - with open(os.path.join(os.path.dirname(__file__), fname)) as f: + with open(os.path.join(os.path.dirname(__file__), fname), encoding="utf-8") as f: return f.read() From f7f4e41c95b97cbd39a4d2ba975ca69d9f39ecc7 Mon Sep 17 00:00:00 2001 From: Timmy Welch Date: Mon, 11 Apr 2022 17:16:07 -0700 Subject: [PATCH 2/2] Catch exception when displaying raw tags --- comictaggerlib/logwindow.py | 8 ++++++-- comictaggerlib/ui/qtutils.py | 10 +++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/comictaggerlib/logwindow.py b/comictaggerlib/logwindow.py index eb24397..7270f50 100644 --- a/comictaggerlib/logwindow.py +++ b/comictaggerlib/logwindow.py @@ -20,6 +20,7 @@ import logging from PyQt5 import QtCore, QtWidgets, uic from comictaggerlib.settings import ComicTaggerSettings +from comictaggerlib.ui import qtutils logger = logging.getLogger(__name__) @@ -41,6 +42,9 @@ class LogWindow(QtWidgets.QDialog): def set_text(self, text): try: text = text.decode() - except: + self.textEdit.setPlainText(text) + except AttributeError: pass - self.textEdit.setPlainText(text) + except Exception as e: + logger.exception("Displaying raw tags failed") + qtutils.qt_error("Displaying raw tags failed:", e) diff --git a/comictaggerlib/ui/qtutils.py b/comictaggerlib/ui/qtutils.py index e9d82c1..f2fb136 100644 --- a/comictaggerlib/ui/qtutils.py +++ b/comictaggerlib/ui/qtutils.py @@ -2,13 +2,14 @@ import io import logging +import traceback from comictaggerlib.settings import ComicTaggerSettings logger = logging.getLogger(__name__) try: - from PyQt5 import QtGui + from PyQt5 import QtGui, QtWidgets qt_available = True except ImportError: @@ -74,3 +75,10 @@ if qt_available: if not success: img.load(ComicTaggerSettings.get_graphic("nocover.png")) return img + + def qt_error(msg: str, e: Exception = None): + trace = "" + if e: + trace = "\n".join(traceback.format_exception(type(e), e, e.__traceback__)) + + QtWidgets.QMessageBox.critical(QtWidgets.QMainWindow(), "Error", msg + trace)