Improve config error handling

- Errors are now combined if possible
- Rich text output in message boxes
- ConfigContainer errors are collected properly
This commit is contained in:
Florian Bruhin 2017-09-14 22:47:06 +02:00
parent 490de32b49
commit b8fb88f4c2
7 changed files with 92 additions and 78 deletions

View File

@ -408,7 +408,7 @@ def _init_modules(args, crash_handler):
objreg.register('readline-bridge', readline_bridge)
log.init.debug("Initializing config...")
config.init(args, qApp)
config.init(qApp)
save_manager.init_autosave()
log.init.debug("Initializing sql...")

View File

@ -24,10 +24,11 @@ import contextlib
import functools
from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QUrl
from PyQt5.QtWidgets import QApplication, QMessageBox
from qutebrowser.config import configdata, configexc, configtypes, configfiles
from qutebrowser.utils import utils, objreg, message, log, usertypes, error
from qutebrowser.misc import objects
from qutebrowser.utils import utils, objreg, message, log, usertypes
from qutebrowser.misc import objects, msgbox
from qutebrowser.commands import cmdexc, cmdutils
from qutebrowser.completion.models import configmodel
@ -35,6 +36,7 @@ from qutebrowser.completion.models import configmodel
val = None
instance = None
key_instance = None
_errbox = None
# Keeping track of all change filters to validate them later.
_change_filters = []
@ -500,27 +502,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 and
do not raise exceptions.
_errors: If confpy=True is given, a list of configexc.ConfigErrorDesc.
_configapi: If given, get values suitable for config.py and
add errors to the given ConfigAPI object.
"""
def __init__(self, config, confpy=False, prefix=''):
def __init__(self, config, configapi=None, prefix=''):
self._config = config
self._prefix = prefix
self._confpy = confpy
self._errors = []
self._configapi = configapi
def __repr__(self):
return utils.get_repr(self, constructor=True, config=self._config,
confpy=self._confpy, prefix=self._prefix)
configapi=self._configapi, 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))
if self._configapi is None:
raise
text = "While {} '{}'".format(action, name)
self._configapi.errors.append(configexc.ConfigErrorDesc(text, e))
def __getattr__(self, attr):
"""Get an option or a new ConfigContainer with the added prefix.
@ -536,14 +539,17 @@ class ConfigContainer:
name = self._join(attr)
if configdata.is_valid_prefix(name):
return ConfigContainer(config=self._config, confpy=self._confpy,
return ConfigContainer(config=self._config,
configapi=self._configapi,
prefix=name)
with self._handle_error('getting', name):
if self._confpy:
return self._config.get_obj(name)
else:
if self._configapi is None:
# access from Python code
return self._config.get(name)
else:
# access from config.py
return self._config.get_obj(name)
def __setattr__(self, attr, value):
"""Set the given option in the config."""
@ -630,7 +636,7 @@ class StyleSheetObserver(QObject):
instance.changed.connect(self._update_stylesheet)
def init(args, parent=None):
def init(parent=None):
"""Initialize the config.
Args:
@ -655,9 +661,13 @@ def init(args, parent=None):
try:
config_api = configfiles.read_config_py()
except configexc.ConfigFileError as e:
error.handle_fatal_exc(e, args=args,
title="Error while loading config")
except configexc.ConfigFileErrors as e:
global _errbox
_errbox = msgbox.msgbox(parent=None,
title="Error while reading config",
text=e.to_html(),
icon=QMessageBox.Warning,
plain_text=False)
config_api = None
if getattr(config_api, 'load_autoconfig', True):

View File

@ -20,7 +20,7 @@
"""Exceptions related to config parsing."""
import traceback
from qutebrowser.utils import jinja
class Error(Exception):
@ -75,49 +75,46 @@ class NoOptionError(Error):
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.
text: The text to show.
exception: The exception which happened.
traceback: The formatted traceback of the exception.
"""
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)
def __init__(self, text, exception, traceback=None):
self.text = text
self.exception = exception
self.traceback = traceback
class ConfigFileErrors(ConfigFileError):
class ConfigFileErrors(Error):
"""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))
super().__init__("Errors occurred while reading {}".format(basename))
self.basename = basename
self.errors = errors
def to_html(self):
template = jinja.environment.from_string("""
Errors occurred while reading {{ basename }}:
<ul>
{% for error in errors %}
<li>
<b>{{ error.text }}</b>: {{ error.exception }}
{% if error.traceback != none %}
<pre>
""".rstrip() + "\n{{ error.traceback }}" + """
</pre>
{% endif %}
</li>
{% endfor %}
</ul>
""")
return template.render(basename=self.basename, errors=self.errors)

View File

@ -110,24 +110,24 @@ class ConfigAPI:
errors: Errors which occurred while setting options.
"""
def __init__(self, config, keyconfig, container):
def __init__(self, config, keyconfig):
self._config = config
self._keyconfig = keyconfig
self.val = container
self.load_autoconfig = True
self.errors = []
self.val = None # Set when initialized
@contextlib.contextmanager
def _handle_error(self, action, name):
try:
yield
except configexc.Error as e:
self.errors.append(configexc.ConfigErrorDesc(action, name, e))
text = "While {} '{}'".format(action, name)
self.errors.append(configexc.ConfigErrorDesc(text, 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):
with self._handle_error('getting', name):
@ -155,32 +155,42 @@ def read_config_py(filename=None):
if not os.path.exists(filename):
return None
container = config.ConfigContainer(config.instance, confpy=True)
api = ConfigAPI(config.instance, config.key_instance, container)
api = ConfigAPI(config.instance, config.key_instance)
container = config.ConfigContainer(config.instance, configapi=api)
api.val = container
module = types.ModuleType('config')
module.config = api
module.c = api.val
module.c = container
module.__file__ = filename
basename = os.path.basename(filename)
try:
with open(filename, mode='rb') as f:
source = f.read()
except OSError as e:
raise configexc.ConfigFileError(basename, e.strerror)
text = "Error while reading {}".format(basename)
desc = configexc.ConfigErrorDesc(text, e)
raise configexc.ConfigFileErrors(basename, [desc])
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)
desc = configexc.ConfigErrorDesc("Error while compiling", e)
raise configexc.ConfigFileErrors(basename, [desc])
except SyntaxError as e:
desc = configexc.ConfigErrorDesc("Syntax Error", e,
traceback=traceback.format_exc())
raise configexc.ConfigFileErrors(basename, [desc])
try:
exec(code, module.__dict__)
except Exception:
raise configexc.ConfigFileUnhandledException(basename)
except Exception as e:
api.errors.append(configexc.ConfigErrorDesc(
"Unhandled exception",
exception=e, traceback=traceback.format_exc()))
api.finalize()
if api.errors:

View File

@ -41,6 +41,7 @@ def msgbox(parent, title, text, *, icon, buttons=QMessageBox.Ok,
A new QMessageBox.
"""
box = QMessageBox(parent)
box.setAttribute(Qt.WA_DeleteOnClose)
box.setIcon(icon)
box.setStandardButtons(buttons)
if on_finished is not None:

View File

@ -36,8 +36,7 @@ def _get_name(exc):
return name
def handle_fatal_exc(exc, args, title, *, pre_text='', post_text='',
richtext=False):
def handle_fatal_exc(exc, args, title, *, pre_text='', post_text=''):
"""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
@ -49,7 +48,6 @@ 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 = [
@ -69,6 +67,4 @@ 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_()

View File

@ -777,12 +777,12 @@ class TestContainer:
new_container = new_container.favicons
assert new_container._prefix == 'tabs.favicons'
@pytest.mark.parametrize('confpy, expected', [
(True, 'rgb'),
(False, QColor.Rgb),
@pytest.mark.parametrize('configapi, expected', [
(object(), 'rgb'),
(None, QColor.Rgb),
])
def test_getattr_option(self, container, confpy, expected):
container._confpy = confpy
def test_getattr_option(self, container, configapi, expected):
container._configapi = configapi
# Use an option with a to_py() so we can check the conversion.
assert container.colors.downloads.system.fg == expected
@ -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(args=None)
config.init()
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(args=None)
config.init()