Initial attempt at error handling for config.py

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...")

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()
 

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))

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
 

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_()

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)