diff --git a/qutebrowser/app.py b/qutebrowser/app.py index adf54c1b5..8cee8d45c 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -408,7 +408,7 @@ def _init_modules(args, crash_handler): objreg.register('readline-bridge', readline_bridge) log.init.debug("Initializing config...") - config.init(qApp) + config.init(args, qApp) save_manager.init_autosave() log.init.debug("Initializing sql...") diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 846c75642..f8587e112 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -26,7 +26,7 @@ import functools from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QUrl from qutebrowser.config import configdata, configexc, configtypes, configfiles -from qutebrowser.utils import utils, objreg, message, log, usertypes +from qutebrowser.utils import utils, objreg, message, log, usertypes, error from qutebrowser.misc import objects from qutebrowser.commands import cmdexc, cmdutils from qutebrowser.completion.models import configmodel @@ -500,18 +500,28 @@ class ConfigContainer: Attributes: _config: The Config object. _prefix: The __getattr__ chain leading up to this object. - _confpy: If True, get values suitable for config.py. + _confpy: If True, get values suitable for config.py and + do not raise exceptions. + _errors: If confpy=True is given, a list of configexc.ConfigErrorDesc. """ def __init__(self, config, confpy=False, prefix=''): self._config = config self._prefix = prefix self._confpy = confpy + self._errors = [] def __repr__(self): return utils.get_repr(self, constructor=True, config=self._config, confpy=self._confpy, prefix=self._prefix) + @contextlib.contextmanager + def _handle_error(self, action, name): + try: + yield + except configexc.Error as e: + self._errors.append(configexc.ConfigErrorDesc(action, name, e)) + def __getattr__(self, attr): """Get an option or a new ConfigContainer with the added prefix. @@ -529,16 +539,20 @@ class ConfigContainer: return ConfigContainer(config=self._config, confpy=self._confpy, prefix=name) - if self._confpy: - return self._config.get_obj(name) - else: - return self._config.get(name) + with self._handle_error('getting', name): + if self._confpy: + return self._config.get_obj(name) + else: + return self._config.get(name) def __setattr__(self, attr, value): """Set the given option in the config.""" if attr.startswith('_'): return super().__setattr__(attr, value) - self._config.set_obj(self._join(attr), value) + + name = self._join(attr) + with self._handle_error('setting', name): + self._config.set_obj(name, value) def _join(self, attr): """Get the prefix joined with the given attribute.""" @@ -616,7 +630,7 @@ class StyleSheetObserver(QObject): instance.changed.connect(self._update_stylesheet) -def init(parent=None): +def init(args, parent=None): """Initialize the config. Args: @@ -639,7 +653,13 @@ def init(parent=None): config_commands = ConfigCommands(instance, key_instance) objreg.register('config-commands', config_commands) - config_api = configfiles.read_config_py() + try: + config_api = configfiles.read_config_py() + except configexc.ConfigFileError as e: + error.handle_fatal_exc(e, args=args, + title="Error while loading config") + config_api = None + if getattr(config_api, 'load_autoconfig', True): instance.read_yaml() diff --git a/qutebrowser/config/configexc.py b/qutebrowser/config/configexc.py index 6bebc55e7..f83b4e371 100644 --- a/qutebrowser/config/configexc.py +++ b/qutebrowser/config/configexc.py @@ -20,6 +20,9 @@ """Exceptions related to config parsing.""" +import traceback + + class Error(Exception): """Base exception for config-related errors.""" @@ -70,3 +73,51 @@ class NoOptionError(Error): def __init__(self, option): super().__init__("No option {!r}".format(option)) self.option = option + + +class ConfigFileError(Error): + + """Raised when there was an error while loading a config file.""" + + def __init__(self, basename, msg): + super().__init__("Failed to load {}: {}".format(basename, msg)) + + +class ConfigFileUnhandledException(ConfigFileError): + + """Raised when there was an unhandled exception while loading config.py. + + Needs to be raised from an exception handler. + """ + + def __init__(self, basename): + super().__init__(basename, "Unhandled exception\n\n{}".format( + traceback.format_exc())) + + +class ConfigErrorDesc: + + """A description of an error happening while reading the config. + + Attributes: + _action: What action has been taken, e.g 'set' + _name: The option which was set, or the key which was bound. + _exception: The exception which happened. + """ + + def __init__(self, action, name, exception): + self._action = action + self._exception = exception + self._name = name + + def __str__(self): + return "While {} {}: {}".format( + self._action, self._name, self._exception) + + +class ConfigFileErrors(ConfigFileError): + + """Raised when multiple errors occurred inside the config.""" + + def __init__(self, basename, errors): + super().__init__(basename, "\n\n".join(str(err) for err in errors)) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index f4ff912ae..4442a5bd8 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -22,10 +22,13 @@ import types import os.path import textwrap +import traceback import configparser +import contextlib from PyQt5.QtCore import QSettings +from qutebrowser.config import configexc from qutebrowser.utils import objreg, standarddir, utils, qtutils @@ -104,6 +107,7 @@ class ConfigAPI: _keyconfig: The KeyConfig object. val: A matching ConfigContainer object. load_autoconfig: Whether autoconfig.yml should be loaded. + errors: Errors which occurred while setting options. """ def __init__(self, config, keyconfig, container): @@ -111,24 +115,41 @@ class ConfigAPI: self._keyconfig = keyconfig self.val = container self.load_autoconfig = True + self.errors = [] + + @contextlib.contextmanager + def _handle_error(self, action, name): + try: + yield + except configexc.Error as e: + self.errors.append(configexc.ConfigErrorDesc(action, name, e)) + + def finalize(self): + """Needs to get called after reading config.py is done.""" + self._config.update_mutables() + self.errors += self.val._errors # pylint: disable=protected-access def get(self, name): - return self._config.get_obj(name) + with self._handle_error('getting', name): + return self._config.get_obj(name) def set(self, name, value): - self._config.set_obj(name, value) + with self._handle_error('setting', name): + self._config.set_obj(name, value) def bind(self, key, command, *, mode, force=False): - self._keyconfig.bind(key, command, mode=mode, force=force) + with self._handle_error('binding', key): + self._keyconfig.bind(key, command, mode=mode, force=force) def unbind(self, key, *, mode): - self._keyconfig.unbind(key, mode=mode) + with self._handle_error('unbinding', key): + self._keyconfig.unbind(key, mode=mode) def read_config_py(filename=None): """Read a config.py file.""" from qutebrowser.config import config - # FIXME:conf error handling + if filename is None: filename = os.path.join(standarddir.config(), 'config.py') if not os.path.exists(filename): @@ -140,13 +161,30 @@ def read_config_py(filename=None): module.config = api module.c = api.val module.__file__ = filename + basename = os.path.basename(filename) - with open(filename, mode='rb') as f: - source = f.read() - code = compile(source, filename, 'exec') - exec(code, module.__dict__) + try: + with open(filename, mode='rb') as f: + source = f.read() + except OSError as e: + raise configexc.ConfigFileError(basename, e.strerror) - config.instance.update_mutables() + try: + code = compile(source, filename, 'exec') + except ValueError as e: + # source contains NUL bytes + raise configexc.ConfigFileError(basename, str(e)) + except SyntaxError: + raise configexc.ConfigFileUnhandledException(basename) + + try: + exec(code, module.__dict__) + except Exception: + raise configexc.ConfigFileUnhandledException(basename) + + api.finalize() + if api.errors: + raise configexc.ConfigFileErrors(basename, api.errors) return api diff --git a/qutebrowser/utils/error.py b/qutebrowser/utils/error.py index 0d045bf19..410e795bc 100644 --- a/qutebrowser/utils/error.py +++ b/qutebrowser/utils/error.py @@ -19,6 +19,7 @@ """Tools related to error printing/displaying.""" +from PyQt5.QtCore import Qt from PyQt5.QtWidgets import QMessageBox from qutebrowser.utils import log, utils @@ -35,7 +36,8 @@ def _get_name(exc): return name -def handle_fatal_exc(exc, args, title, *, pre_text='', post_text=''): +def handle_fatal_exc(exc, args, title, *, pre_text='', post_text='', + richtext=False): """Handle a fatal "expected" exception by displaying an error box. If --no-err-windows is given as argument, the text is logged to the error @@ -47,6 +49,7 @@ def handle_fatal_exc(exc, args, title, *, pre_text='', post_text=''): title: The title to be used for the error message. pre_text: The text to be displayed before the exception text. post_text: The text to be displayed after the exception text. + richtext: If given, interpret the given text as rich text. """ if args.no_err_windows: lines = [ @@ -66,4 +69,6 @@ def handle_fatal_exc(exc, args, title, *, pre_text='', post_text=''): if post_text: msg_text += '\n\n{}'.format(post_text) msgbox = QMessageBox(QMessageBox.Critical, title, msg_text) + if richtext: + msgbox.setTextFormat(Qt.RichText) msgbox.exec_() diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 99a57d479..529693d59 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -877,7 +877,7 @@ def test_init(init_patch, fake_save_manager, config_tmpdir, load_autoconfig): config_py_lines.append('config.load_autoconfig = False') config_py_file.write_text('\n'.join(config_py_lines), 'utf-8', ensure=True) - config.init() + config.init(args=None) objreg.get('config-commands') assert isinstance(config.instance, config.Config) @@ -899,4 +899,4 @@ def test_init(init_patch, fake_save_manager, config_tmpdir, load_autoconfig): def test_init_invalid_change_filter(init_patch): config.change_filter('foobar') with pytest.raises(configexc.NoOptionError): - config.init() + config.init(args=None)