From ec07e4f8beaf4df6eb6ea3e744f24844d4c782ad Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 10 Dec 2014 18:00:49 +0100 Subject: [PATCH] Handle OSError exceptions where appropriate. Fixes #25. --- qutebrowser/app.py | 62 +++++++++++++++++++---------- qutebrowser/browser/adblock.py | 21 +++++++--- qutebrowser/browser/commands.py | 4 +- qutebrowser/browser/downloads.py | 7 +++- qutebrowser/commands/userscripts.py | 61 ++++++++++++++++++---------- qutebrowser/config/config.py | 9 ++++- qutebrowser/config/keyconfparser.py | 40 ++++++++++--------- qutebrowser/utils/editor.py | 30 +++++++++----- qutebrowser/utils/jinja.py | 2 +- qutebrowser/utils/qtutils.py | 4 +- qutebrowser/utils/standarddir.py | 17 +++++--- qutebrowser/utils/utils.py | 25 +++++++----- qutebrowser/utils/version.py | 2 +- 13 files changed, 184 insertions(+), 100 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index a568d2b6c..dcb4c2fa7 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -31,7 +31,7 @@ import functools import traceback import faulthandler -from PyQt5.QtWidgets import QApplication, QDialog +from PyQt5.QtWidgets import QApplication, QDialog, QMessageBox from PyQt5.QtGui import QDesktopServices from PyQt5.QtCore import (pyqtSlot, qInstallMessageHandler, QTimer, QUrl, QStandardPaths, QObject, Qt) @@ -70,6 +70,7 @@ class Application(QApplication): Args: Argument namespace from argparse. """ + # pylint: disable=too-many-statements self._quit_status = { 'crash': True, 'tabs': False, @@ -114,7 +115,14 @@ class Application(QApplication): self.setApplicationName("qutebrowser") self.setApplicationVersion(qutebrowser.__version__) utils.actute_warning() - self._init_modules() + try: + self._init_modules() + except OSError as e: + msgbox = QMessageBox( + QMessageBox.Critical, "Error while initializing!", + "Error while initializing: {}".format(e)) + msgbox.exec_() + sys.exit(1) QTimer.singleShot(0, self._open_pages) log.init.debug("Initializing eventfilter...") @@ -184,32 +192,37 @@ class Application(QApplication): """Handle a segfault from a previous run.""" path = standarddir.get(QStandardPaths.DataLocation) logname = os.path.join(path, 'crash.log') - # First check if an old logfile exists. - if os.path.exists(logname): - with open(logname, 'r', encoding='ascii') as f: - data = f.read() - try: + try: + # First check if an old logfile exists. + if os.path.exists(logname): + with open(logname, 'r', encoding='ascii') as f: + data = f.read() os.remove(logname) - except PermissionError: - log.init.warning("Could not remove crash log!") - else: self._init_crashlogfile() - if data: - # Crashlog exists and has data in it, so something crashed - # previously. - self._crashdlg = crash.FatalCrashDialog(self._args.debug, data) - self._crashdlg.show() - else: - # There's no log file, so we can use this to display crashes to the - # user on the next start. + if data: + # Crashlog exists and has data in it, so something crashed + # previously. + self._crashdlg = crash.FatalCrashDialog(self._args.debug, + data) + self._crashdlg.show() + else: + # There's no log file, so we can use this to display crashes to + # the user on the next start. + self._init_crashlogfile() + except OSError: + log.init.exception("Error while handling crash log file!") self._init_crashlogfile() def _init_crashlogfile(self): """Start a new logfile and redirect faulthandler to it.""" path = standarddir.get(QStandardPaths.DataLocation) logname = os.path.join(path, 'crash.log') - self._crashlogfile = open(logname, 'w', encoding='ascii') - earlyinit.init_faulthandler(self._crashlogfile) + try: + self._crashlogfile = open(logname, 'w', encoding='ascii') + except OSError: + log.init.exception("Error while opening crash log file!") + else: + earlyinit.init_faulthandler(self._crashlogfile) def _open_pages(self): """Open startpage etc. and process commandline args.""" @@ -446,10 +459,10 @@ class Application(QApplication): faulthandler.enable(sys.__stderr__) else: faulthandler.disable() - self._crashlogfile.close() try: + self._crashlogfile.close() os.remove(self._crashlogfile.name) - except (PermissionError, FileNotFoundError): + except OSError: log.destroy.exception("Could not remove crash log!") def _exception_hook(self, exctype, excvalue, tb): @@ -756,6 +769,11 @@ class Application(QApplication): what, utils.qualname(handler))) try: handler() + except OSError as e: + msgbox = QMessageBox( + QMessageBox.Critical, "Error while saving!", + "Error while saving {}: {}".format(what, e)) + msgbox.exec_() except AttributeError as e: log.destroy.warning("Could not save {}.".format(what)) log.destroy.debug(e) diff --git a/qutebrowser/browser/adblock.py b/qutebrowser/browser/adblock.py index 6aa7ca548..99009ca13 100644 --- a/qutebrowser/browser/adblock.py +++ b/qutebrowser/browser/adblock.py @@ -100,9 +100,12 @@ class HostBlocker: """Read hosts from the existing blocked-hosts file.""" self.blocked_hosts = set() if os.path.exists(self._hosts_file): - with open(self._hosts_file, 'r', encoding='utf-8') as f: - for line in f: - self.blocked_hosts.add(line.strip()) + try: + with open(self._hosts_file, 'r', encoding='utf-8') as f: + for line in f: + self.blocked_hosts.add(line.strip()) + except OSError: + log.misc.exception("Failed to read host blocklist!") else: if config.get('content', 'host-block-lists') is not None: message.info('last-focused', @@ -120,7 +123,10 @@ class HostBlocker: return for url in urls: if url.scheme() == 'file': - fileobj = open(url.path(), 'rb') + try: + fileobj = open(url.path(), 'rb') + except OSError: + log.misc.exception("Failed to open block list!") download = FakeDownload(fileobj) self._in_progress.append(download) self.on_download_finished(download) @@ -145,7 +151,7 @@ class HostBlocker: line_count = 0 try: f = get_fileobj(byte_io) - except (FileNotFoundError, UnicodeDecodeError, zipfile.BadZipFile, + except (OSError, UnicodeDecodeError, zipfile.BadZipFile, zipfile.LargeZipFile) as e: message.error('last-focused', "adblock: Error while reading {}: " "{} - {}".format( @@ -213,4 +219,7 @@ class HostBlocker: finally: download.fileobj.close() if not self._in_progress: - self.on_lists_downloaded() + try: + self.on_lists_downloaded() + except OSError: + log.misc.exception("Failed to write host block list!") diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index deac9b26c..fd15e4881 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -206,10 +206,10 @@ class CommandDispatcher: def _editor_cleanup(self, oshandle, filename): """Clean up temporary file when the editor was closed.""" - os.close(oshandle) try: + os.close(oshandle) os.remove(filename) - except PermissionError: + except OSError: raise cmdexc.CommandError("Failed to delete tempfile...") def _get_selection_override(self, left, right, opposite): diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 9a3a78ab4..41d47ea65 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -307,8 +307,11 @@ class DownloadItem(QObject): self.reply = None if self.fileobj is not None: self.fileobj.close() - if self._filename is not None and os.path.exists(self._filename): - os.remove(self._filename) + try: + if self._filename is not None and os.path.exists(self._filename): + os.remove(self._filename) + except OSError: + log.downloads.exception("Failed to remove partial file") self.finished.emit() def set_filename(self, filename): diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index e8d150893..4f85f6db5 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -61,13 +61,18 @@ class _BlockingFIFOReader(QObject): def read(self): """Blocking read loop which emits got_line when a new line arrived.""" - # We open as R/W so we never get EOF and have to reopen the pipe. - # See http://www.outflux.net/blog/archives/2008/03/09/using-select-on-a-fifo/ - # We also use os.open and os.fdopen rather than built-in open so we can - # add O_NONBLOCK. - fd = os.open(self._filepath, os.O_RDWR | - os.O_NONBLOCK) # pylint: disable=no-member - self.fifo = os.fdopen(fd, 'r') + try: + # We open as R/W so we never get EOF and have to reopen the pipe. + # See http://www.outflux.net/blog/archives/2008/03/09/using-select-on-a-fifo/ + # We also use os.open and os.fdopen rather than built-in open so we + # can add O_NONBLOCK. + fd = os.open(self._filepath, os.O_RDWR | + os.O_NONBLOCK) # pylint: disable=no-member + self.fifo = os.fdopen(fd, 'r') + except OSError: + log.procs.exception("Failed to read FIFO") + self.finished.emit() + return while True: log.procs.debug("thread loop") ready_r, _ready_w, _ready_e = select.select([self.fifo], [], [], 1) @@ -141,7 +146,7 @@ class _BaseUserscriptRunner(QObject): """Clean up the temporary file.""" try: os.remove(self._filepath) - except PermissionError as e: + except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's # executed async. message.error(self._win_id, @@ -196,13 +201,18 @@ class _POSIXUserscriptRunner(_BaseUserscriptRunner): def run(self, cmd, *args, env=None): rundir = standarddir.get(QStandardPaths.RuntimeLocation) - # tempfile.mktemp is deprecated and discouraged, but we use it here to - # create a FIFO since the only other alternative would be to create a - # directory and place the FIFO there, which sucks. Since os.kfifo will - # raise an exception anyways when the path doesn't exist, it shouldn't - # be a big issue. - self._filepath = tempfile.mktemp(prefix='userscript-', dir=rundir) - os.mkfifo(self._filepath) # pylint: disable=no-member + try: + # tempfile.mktemp is deprecated and discouraged, but we use it here + # to create a FIFO since the only other alternative would be to + # create a directory and place the FIFO there, which sucks. Since + # os.kfifo will raise an exception anyways when the path doesn't + # exist, it shouldn't be a big issue. + self._filepath = tempfile.mktemp(prefix='userscript-', dir=rundir) + os.mkfifo(self._filepath) # pylint: disable=no-member + except OSError as e: + message.error(self._win_id, "Error while creating FIFO: {}".format( + e)) + return self._reader = _BlockingFIFOReader(self._filepath) self._thread = QThread(self) @@ -262,16 +272,22 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): def _cleanup(self): """Clean up temporary files after the userscript finished.""" - os.close(self._oshandle) + try: + os.close(self._oshandle) + except OSError: + log.procs.exception("Failed to close file handle!") super()._cleanup() self._oshandle = None def on_proc_finished(self): """Read back the commands when the process finished.""" log.procs.debug("proc finished") - with open(self._filepath, 'r', encoding='utf-8') as f: - for line in f: - self.got_cmd.emit(line.rstrip()) + try: + with open(self._filepath, 'r', encoding='utf-8') as f: + for line in f: + self.got_cmd.emit(line.rstrip()) + except OSError: + log.procs.exception("Failed to read command file!") self._cleanup() self.finished.emit() @@ -282,7 +298,12 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner): self.finished.emit() def run(self, cmd, *args, env=None): - self._oshandle, self._filepath = tempfile.mkstemp(text=True) + try: + self._oshandle, self._filepath = tempfile.mkstemp(text=True) + except OSError as e: + message.error(self._win_id, "Error while creating tempfile: " + "{}".format(e)) + return self._run_process(cmd, *args, env=env) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 149d19b43..66adc271f 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -572,7 +572,14 @@ class ConfigManager(QObject): if self._initialized: self._after_set(sectname, optname) - @cmdutils.register(instance='config') + @cmdutils.register(instance='config', name='save') + def save_command(self): + """Save the config file.""" + try: + self.save() + except OSError as e: + raise cmdexc.CommandError("Could not save config: {}".format(e)) + def save(self): """Save the config file.""" if self._configdir is None: diff --git a/qutebrowser/config/keyconfparser.py b/qutebrowser/config/keyconfparser.py index a1412709a..c5b0e1808 100644 --- a/qutebrowser/config/keyconfparser.py +++ b/qutebrowser/config/keyconfparser.py @@ -210,24 +210,28 @@ class KeyConfigParser(QObject): def _read(self): """Read the config file from disk and parse it.""" - with open(self._configfile, 'r', encoding='utf-8') as f: - for i, line in enumerate(f): - line = line.rstrip() - try: - if not line.strip() or line.startswith('#'): - continue - elif line.startswith('[') and line.endswith(']'): - sectname = line[1:-1] - self._cur_section = self._normalize_sectname(sectname) - elif line.startswith((' ', '\t')): - line = line.strip() - self._read_keybinding(line) - else: - line = line.strip() - self._read_command(line) - except KeyConfigError as e: - e.lineno = i - raise + try: + with open(self._configfile, 'r', encoding='utf-8') as f: + for i, line in enumerate(f): + line = line.rstrip() + try: + if not line.strip() or line.startswith('#'): + continue + elif line.startswith('[') and line.endswith(']'): + sectname = line[1:-1] + self._cur_section = self._normalize_sectname( + sectname) + elif line.startswith((' ', '\t')): + line = line.strip() + self._read_keybinding(line) + else: + line = line.strip() + self._read_command(line) + except KeyConfigError as e: + e.lineno = i + raise + except OSError: + log.keyboard.exception("Failed to read keybindings!") for sectname in self.keybindings: self.changed.emit(sectname) diff --git a/qutebrowser/utils/editor.py b/qutebrowser/utils/editor.py index ddb3992e7..328273639 100644 --- a/qutebrowser/utils/editor.py +++ b/qutebrowser/utils/editor.py @@ -52,10 +52,10 @@ class ExternalEditor(QObject): def _cleanup(self): """Clean up temporary files after the editor closed.""" - os.close(self._oshandle) try: + os.close(self._oshandle) os.remove(self._filename) - except PermissionError as e: + except OSError as e: # NOTE: Do not replace this with "raise CommandError" as it's # executed async. message.error(self._win_id, @@ -80,8 +80,15 @@ class ExternalEditor(QObject): "{})!".format(exitcode)) return encoding = config.get('general', 'editor-encoding') - with open(self._filename, 'r', encoding=encoding) as f: - text = ''.join(f.readlines()) + try: + with open(self._filename, 'r', encoding=encoding) as f: + text = ''.join(f.readlines()) + except OSError as e: + # NOTE: Do not replace this with "raise CommandError" as it's + # executed async. + message.error(self._win_id, "Failed to read back edited file: " + "{}".format(e)) + return log.procs.debug("Read back: {}".format(text)) self.editing_finished.emit(text) finally: @@ -114,11 +121,16 @@ class ExternalEditor(QObject): if self._text is not None: raise ValueError("Already editing a file!") self._text = text - self._oshandle, self._filename = tempfile.mkstemp(text=True) - if text: - encoding = config.get('general', 'editor-encoding') - with open(self._filename, 'w', encoding=encoding) as f: - f.write(text) + try: + self._oshandle, self._filename = tempfile.mkstemp(text=True) + if text: + encoding = config.get('general', 'editor-encoding') + with open(self._filename, 'w', encoding=encoding) as f: + f.write(text) + except OSError as e: + message.error(self._win_id, "Failed to create initial file: " + "{}".format(e)) + return self._proc = QProcess(self) self._proc.finished.connect(self.on_proc_closed) self._proc.error.connect(self.on_proc_error) diff --git a/qutebrowser/utils/jinja.py b/qutebrowser/utils/jinja.py index 62148aa80..d18ac4f41 100644 --- a/qutebrowser/utils/jinja.py +++ b/qutebrowser/utils/jinja.py @@ -41,7 +41,7 @@ class Loader(jinja2.BaseLoader): path = os.path.join(self._subdir, template) try: source = utils.read_file(path) - except FileNotFoundError: + except OSError: raise jinja2.TemplateNotFound(template) # Currently we don't implement auto-reloading, so we always return True # for up-to-date. diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py index e56e95fb0..ffe3cdfb6 100644 --- a/qutebrowser/utils/qtutils.py +++ b/qutebrowser/utils/qtutils.py @@ -161,6 +161,7 @@ def deserialize(data, obj): def savefile_open(filename, binary=False, encoding='utf-8'): """Context manager to easily use a QSaveFile.""" f = QSaveFile(filename) + new_f = None try: ok = f.open(QIODevice.WriteOnly) if not ok: # pylint: disable=used-before-assignment @@ -174,7 +175,8 @@ def savefile_open(filename, binary=False, encoding='utf-8'): f.cancelWriting() raise finally: - new_f.flush() + if new_f is not None: + new_f.flush() ok = f.commit() if not ok: raise OSError(f.errorString()) diff --git a/qutebrowser/utils/standarddir.py b/qutebrowser/utils/standarddir.py index f82c73704..f7b9e42ea 100644 --- a/qutebrowser/utils/standarddir.py +++ b/qutebrowser/utils/standarddir.py @@ -24,6 +24,8 @@ import os.path from PyQt5.QtCore import QCoreApplication, QStandardPaths +from qutebrowser.utils import log + def _writable_location(typ): """Wrapper around QStandardPaths.writableLocation.""" @@ -124,9 +126,12 @@ def init(): # http://www.brynosaurus.com/cachedir/spec.html cachedir_tag = os.path.join(cache_dir, 'CACHEDIR.TAG') if not os.path.exists(cachedir_tag): - with open(cachedir_tag, 'w', encoding='utf-8') as f: - f.write("Signature: 8a477f597d28d172789f06886806bc55\n") - f.write("# This file is a cache directory tag created by " - "qutebrowser.\n") - f.write("# For information about cache directory tags, see:\n") - f.write("# http://www.brynosaurus.com/cachedir/\n") + try: + with open(cachedir_tag, 'w', encoding='utf-8') as f: + f.write("Signature: 8a477f597d28d172789f06886806bc55\n") + f.write("# This file is a cache directory tag created by " + "qutebrowser.\n") + f.write("# For information about cache directory tags, see:\n") + f.write("# http://www.brynosaurus.com/cachedir/\n") + except OSError: + log.misc.exception("Failed to create CACHEDIR.TAG") diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index da9ccd40b..c10c36cf1 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -130,17 +130,20 @@ def actute_warning(): return except ValueError: pass - with open('/usr/share/X11/locale/en_US.UTF-8/Compose', 'r', - encoding='utf-8') as f: - for line in f: - if '' in line: - if sys.stdout is not None: - sys.stdout.flush() - print("Note: If you got a 'dead_actute' warning above, that " - "is not a bug in qutebrowser! See " - "https://bugs.freedesktop.org/show_bug.cgi?id=69476 for " - "details.") - break + try: + with open('/usr/share/X11/locale/en_US.UTF-8/Compose', 'r', + encoding='utf-8') as f: + for line in f: + if '' in line: + if sys.stdout is not None: + sys.stdout.flush() + print("Note: If you got a 'dead_actute' warning above, " + "that is not a bug in qutebrowser! See " + "https://bugs.freedesktop.org/show_bug.cgi?id=69476 " + "for details.") + break + except OSError: + log.misc.exception("Failed to read Compose file") def _get_color_percentage(a_c1, a_c2, a_c3, b_c1, b_c2, b_c3, percent): diff --git a/qutebrowser/utils/version.py b/qutebrowser/utils/version.py index b401c542a..e1e73f20b 100644 --- a/qutebrowser/utils/version.py +++ b/qutebrowser/utils/version.py @@ -73,7 +73,7 @@ def _git_str(): # If that fails, check the git-commit-id file. try: return utils.read_file('git-commit-id') - except (FileNotFoundError, ImportError): + except (OSError, ImportError): return None