From a90815f5dfb12de714ffa9fac3d9383ef7f286c2 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 28 Apr 2015 17:57:00 +0200 Subject: [PATCH] Improve errorhandling for enduser --- tests/storage/test_http.py | 3 ++- vdirsyncer/cli/utils.py | 5 +++-- vdirsyncer/exceptions.py | 5 +++++ vdirsyncer/storage/base.py | 2 +- vdirsyncer/storage/dav.py | 6 +++--- vdirsyncer/storage/http.py | 37 +++++++++++++++++--------------- vdirsyncer/storage/memory.py | 3 ++- vdirsyncer/storage/singlefile.py | 6 ++++-- vdirsyncer/sync.py | 4 ++-- 9 files changed, 42 insertions(+), 29 deletions(-) diff --git a/tests/storage/test_http.py b/tests/storage/test_http.py index fc850c2..f2a5c8b 100644 --- a/tests/storage/test_http.py +++ b/tests/storage/test_http.py @@ -6,6 +6,7 @@ from requests import Response from tests import normalize_item +from vdirsyncer.exceptions import UserError from vdirsyncer.storage.http import HttpStorage, prepare_auth @@ -107,7 +108,7 @@ def test_prepare_auth_guess(monkeypatch, auth): monkeypatch.delattr(requests_toolbelt.auth.guess, 'GuessAuth') - with pytest.raises(RuntimeError) as excinfo: + with pytest.raises(UserError) as excinfo: prepare_auth(auth, 'user', 'pwd') assert 'requests_toolbelt is too old' in str(excinfo.value).lower() diff --git a/vdirsyncer/cli/utils.py b/vdirsyncer/cli/utils.py index 15d9ea8..4c92df5 100644 --- a/vdirsyncer/cli/utils.py +++ b/vdirsyncer/cli/utils.py @@ -495,8 +495,9 @@ def handle_storage_init_error(cls, config): .format(cls.storage_name, u', '.join(invalid))) if not problems: - cli_logger.exception('') - problems.append('Internal error, see above.') + if not isinstance(e, exceptions.UserError): + cli_logger.exception('') + problems.append(str(e)) raise CliError(u'Failed to initialize {}'.format(config['instance_name']), problems=problems) diff --git a/vdirsyncer/exceptions.py b/vdirsyncer/exceptions.py index a4d960e..a5e541b 100644 --- a/vdirsyncer/exceptions.py +++ b/vdirsyncer/exceptions.py @@ -17,6 +17,11 @@ class Error(Exception): super(Error, self).__init__(*args) +class UserError(Error, ValueError): + '''Wrapper exception to be used to signify the traceback should not be + shown to the user.''' + + class CollectionNotFound(Error): '''Collection not found''' diff --git a/vdirsyncer/storage/base.py b/vdirsyncer/storage/base.py index 040467d..48222e0 100644 --- a/vdirsyncer/storage/base.py +++ b/vdirsyncer/storage/base.py @@ -77,7 +77,7 @@ class Storage(with_metaclass(StorageMeta)): if read_only is None: read_only = self.read_only if self.read_only and not read_only: - raise ValueError('This storage can only be read-only.') + raise exceptions.UserError('This storage can only be read-only.') self.read_only = bool(read_only) if collection and instance_name: diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index baa0d23..ca77b44 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -591,12 +591,12 @@ class CaldavStorage(DavStorage): item_types=(), **kwargs): super(CaldavStorage, self).__init__(**kwargs) if not isinstance(item_types, (list, tuple)): - raise ValueError('item_types must be a list.') + raise exceptions.UserError('item_types must be a list.') self.item_types = tuple(item_types) if (start_date is None) != (end_date is None): - raise ValueError('If start_date is given, ' - 'end_date has to be given too.') + raise exceptions.UserError('If start_date is given, ' + 'end_date has to be given too.') elif start_date is not None and end_date is not None: namespace = dict(datetime.__dict__) namespace['start_date'] = self.start_date = \ diff --git a/vdirsyncer/storage/http.py b/vdirsyncer/storage/http.py index 7b606a4..b4283fa 100644 --- a/vdirsyncer/storage/http.py +++ b/vdirsyncer/storage/http.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from .base import Item, Storage -from ..exceptions import NotFoundError +from .. import exceptions from ..utils import expand_path from ..utils.compat import iteritems, text_type, urlparse from ..utils.http import request @@ -22,16 +22,19 @@ def prepare_auth(auth, username, password): try: from requests_toolbelt.auth.guess import GuessAuth except ImportError: - raise RuntimeError('Your version of requests_toolbelt is too ' - 'old for `guess` authentication. At least ' - 'version 0.4.0 is required.') + raise exceptions.UserError( + 'Your version of requests_toolbelt is too ' + 'old for `guess` authentication. At least ' + 'version 0.4.0 is required.' + ) else: return GuessAuth(username, password) else: - raise ValueError('Unknown authentication method: {}'.format(auth)) + raise exceptions.UserError('Unknown authentication method: {}' + .format(auth)) elif auth: - raise ValueError('For {} authentication, you need to specify username ' - 'and password.'.format(auth)) + raise exceptions.UserError('You need to specify username and password ' + 'for {} authentication.'.format(auth)) else: return None @@ -40,20 +43,20 @@ def prepare_verify(verify, verify_fingerprint): if isinstance(verify, (text_type, bytes)): verify = expand_path(verify) elif not isinstance(verify, bool): - raise ValueError('Invalid value for verify ({}), ' - 'must be a path to a PEM-file or boolean.' - .format(verify)) + raise exceptions.UserError('Invalid value for verify ({}), ' + 'must be a path to a PEM-file or boolean.' + .format(verify)) if verify_fingerprint is not None: if not isinstance(verify_fingerprint, str): - raise ValueError('Invalid value for verify_fingerprint ({}), ' - 'must be a string or null.' - .format(verify_fingerprint)) + raise exceptions.UserError('Invalid value for verify_fingerprint ' + '({}), must be a string or null.' + .format(verify_fingerprint)) verify = False elif not verify: - raise ValueError('verify = false is forbidden. Consider setting ' - 'verify_fingerprint instead, which also disables ' - 'validation against CAs.') + raise exceptions.UserError('verify = false is forbidden. Consider ' + 'setting verify_fingerprint instead, which ' + 'also disables validation against CAs.') return { 'verify': verify, @@ -161,4 +164,4 @@ class HttpStorage(Storage): try: return self._items[href] except KeyError: - raise NotFoundError(href) + raise exceptions.NotFoundError(href) diff --git a/vdirsyncer/storage/memory.py b/vdirsyncer/storage/memory.py index 90d0767..8a32f3d 100644 --- a/vdirsyncer/storage/memory.py +++ b/vdirsyncer/storage/memory.py @@ -18,7 +18,8 @@ class MemoryStorage(Storage): def __init__(self, fileext='', **kwargs): if kwargs.get('collection') is not None: - raise ValueError('MemoryStorage does not support collections.') + raise exceptions.UserError('MemoryStorage does not support ' + 'collections.') self.items = {} # href => (etag, item) self.fileext = fileext super(MemoryStorage, self).__init__(**kwargs) diff --git a/vdirsyncer/storage/singlefile.py b/vdirsyncer/storage/singlefile.py index 76027aa..45ca28f 100644 --- a/vdirsyncer/storage/singlefile.py +++ b/vdirsyncer/storage/singlefile.py @@ -77,8 +77,10 @@ class SingleFileStorage(Storage): collection = kwargs.get('collection') if collection is not None: - raise ValueError('collection is not a valid argument for {}' - .format(type(self).__name__)) + raise exceptions.UserError( + 'collection is not a valid argument for {}' + .format(type(self).__name__) + ) checkfile(path, create=False) diff --git a/vdirsyncer/sync.py b/vdirsyncer/sync.py index f04c3bd..f57a821 100644 --- a/vdirsyncer/sync.py +++ b/vdirsyncer/sync.py @@ -278,8 +278,8 @@ def _action_conflict_resolve(ident): sync_logger.info('...{} wins.'.format(b.storage)) _action_update(ident, b, a)(a, b, conflict_resolution) else: - raise ValueError('Invalid conflict resolution mode: {}' - .format(conflict_resolution)) + raise exceptions.UserError('Invalid conflict resolution mode: {}' + .format(conflict_resolution)) return inner