preserve tracebacks in verbose mode (#387)

This commit is contained in:
Adrian Sampson 2012-05-18 15:42:20 -07:00
parent 13995201a1
commit 395ba21013
5 changed files with 105 additions and 51 deletions

View file

@ -118,14 +118,14 @@ def _album_for_id(album_id):
try:
return mb.album_for_id(album_id)
except mb.MusicBrainzAPIError as exc:
log.error(exc.log())
exc.log(log)
def _track_for_id(track_id):
"""Get an item for a recording MBID."""
try:
return mb.track_for_id(track_id)
except mb.MusicBrainzAPIError as exc:
log.error(exc.log())
exc.log(log)
def _album_candidates(items, artist, album, va_likely):
"""Search for album matches. ``items`` is a list of Item objects
@ -141,14 +141,14 @@ def _album_candidates(items, artist, album, va_likely):
try:
out.extend(mb.match_album(artist, album, len(items)))
except mb.MusicBrainzAPIError as exc:
log.error(exc.log())
exc.log(log)
# Also add VA matches from MusicBrainz where appropriate.
if va_likely and album:
try:
out.extend(mb.match_album(None, album, len(items)))
except mb.MusicBrainzAPIError as exc:
log.error(exc.log())
exc.log(log)
# Candidates from plugins.
out.extend(plugins.candidates(items))
@ -167,7 +167,7 @@ def _item_candidates(item, artist, title):
try:
out.extend(mb.match_track(artist, title))
except mb.MusicBrainzAPIError as exc:
log.error(exc.log())
exc.log(log)
# Plugin candidates.
out.extend(plugins.item_candidates(item))

View file

@ -16,9 +16,11 @@
"""
import logging
import musicbrainzngs
import traceback
import beets.autotag.hooks
import beets
from beets import util
SEARCH_LIMIT = 5
VARIOUS_ARTISTS_ID = '89ad4ac3-39f7-470e-963a-56509c546377'
@ -26,21 +28,18 @@ VARIOUS_ARTISTS_ID = '89ad4ac3-39f7-470e-963a-56509c546377'
musicbrainzngs.set_useragent('beets', beets.__version__,
'http://beets.radbox.org/')
class MusicBrainzAPIError(Exception):
"""An error while talking to MusicBrainz. Has three fields:
`reason`, the underlying exception; `verb`, the action being taken
(a string); and `query`, the parameter to the action (of any type).
class MusicBrainzAPIError(util.HumanReadableException):
"""An error while talking to MusicBrainz. The `query` field is the
parameter to the action and may have any type.
"""
def __init__(self, reason, verb, query):
self.reason = reason
self.verb = verb
def __init__(self, reason, verb, query, tb=None):
self.query = query
msg = u'"{0}" in {1} with query {2}'.format(reason, verb, repr(query))
super(MusicBrainzAPIError, self).__init__(msg)
super(MusicBrainzAPIError, self).__init__(reason, verb, tb)
def log(self):
"""Produce a human-readable log message."""
return u'MusicBrainz API error: {0}'.format(self)
def get_message(self):
return u'"{0}" in {1} with query {2}'.format(
self._reasonstr(), self.verb, repr(self.query)
)
log = logging.getLogger('beets')
@ -197,7 +196,8 @@ def match_album(artist, album, tracks=None, limit=SEARCH_LIMIT):
try:
res = _mb_release_search(limit=limit, **criteria)
except musicbrainzngs.MusicBrainzError as exc:
raise MusicBrainzAPIError(exc, 'release search', criteria)
raise MusicBrainzAPIError(exc, 'release search', criteria,
traceback.format_exc())
for release in res['release-list']:
# The search result is missing some data (namely, the tracks),
# so we just use the ID and fetch the rest of the information.
@ -220,7 +220,8 @@ def match_track(artist, title, limit=SEARCH_LIMIT):
try:
res = _mb_recording_search(limit=limit, **criteria)
except musicbrainzngs.MusicBrainzError as exc:
raise MusicBrainzAPIError(exc, 'recording search', criteria)
raise MusicBrainzAPIError(exc, 'recording search', criteria,
traceback.format_exc())
for recording in res['recording-list']:
yield track_info(recording)
@ -235,7 +236,8 @@ def album_for_id(albumid):
log.debug('Album ID match failed.')
return None
except musicbrainzngs.MusicBrainzError as exc:
raise MusicBrainzAPIError(exc, 'get release by ID', albumid)
raise MusicBrainzAPIError(exc, 'get release by ID', albumid,
traceback.format_exc())
return album_info(res['release'])
def track_for_id(trackid):
@ -248,5 +250,6 @@ def track_for_id(trackid):
log.debug('Track ID match failed.')
return None
except musicbrainzngs.MusicBrainzError as exc:
raise MusicBrainzAPIError(exc, 'get recording by ID', trackid)
raise MusicBrainzAPIError(exc, 'get recording by ID', trackid,
traceback.format_exc())
return track_info(res['recording'])

View file

@ -749,8 +749,8 @@ def main(args=None, configfh=None):
except UserError as exc:
message = exc.args[0] if exc.args else None
subcommand.parser.error(message)
except util.FilesystemError as exc:
log.error(u'file manipulation error: {0}'.format(exc.args[0]))
except util.HumanReadableException as exc:
exc.log(log)
sys.exit(1)
except IOError as exc:
if exc.errno == errno.EPIPE:

View file

@ -21,41 +21,88 @@ import re
import shutil
import fnmatch
from collections import defaultdict
import traceback
MAX_FILENAME_LENGTH = 200
class FilesystemError(Exception):
"""An error that occurred while performing a filesystem manipulation
via a function in this module. Has three fields: `reason`, the
underlying exception or a string describing the problem; `verb`, the
action being performed; and `paths`, a list of pathnames involved.
class HumanReadableException(Exception):
"""An Exception that can include a human-readable error message to
be logged without a traceback. Can preserve a traceback for
debugging purposes as well.
Has at least two fields: `reason`, the underlying exception or a
string describing the problem; and `verb`, the action being
performed during the error.
If `tb` is provided, it is a string containing a traceback for the
associated exception. (Note that this is not necessary in Python 3.x
and should be removed when we make the transition.)
"""
def __init__(self, reason, verb, paths):
error_kind = 'Error' # Human-readable description of error type.
def __init__(self, reason, verb, tb=None):
self.reason = reason
self.verb = verb
self.paths = paths
self.tb = tb
super(HumanReadableException, self).__init__(self.get_message())
# Use a nicer English phrasing for some specific verbs.
gerund = verb[:-1] if verb.endswith('e') else verb
def _gerund(self):
"""Generate a (likely) gerund form of the English verb.
"""
if ' ' in self.verb:
return self.verb
gerund = self.verb[:-1] if self.verb.endswith('e') else self.verb
gerund += 'ing'
if verb in ('move', 'copy'):
clause = 'while {0} {1} to {2}'.format(gerund, repr(paths[0]),
repr(paths[1]))
elif verb in ('delete',):
clause = 'while {0} {1}'.format(gerund, repr(paths[0]))
return gerund
def _reasonstr(self):
"""Get the reason as a string."""
if isinstance(self.reason, basestring):
return self.reason
elif hasattr(self.reason, 'strerror'): # i.e., EnvironmentError
return self.reason.strerror
else:
return u'"{0}"'.format(self.reason)
def get_message(self):
"""Create the human-readable description of the error, sans
introduction.
"""
raise NotImplementedError
def log(self, logger):
"""Log to the provided `logger` a human-readable message as an
error and a verbose traceback as a debug message.
"""
if self.tb:
logger.debug(self.tb)
logger.error(u'{0}: {1}'.format(self.error_kind, self.args[0]))
class FilesystemError(HumanReadableException):
"""An error that occurred while performing a filesystem manipulation
via a function in this module. The `paths` field is a sequence of
pathnames involved in the operation.
"""
def __init__(self, reason, verb, paths, tb=None):
self.paths = paths
super(FilesystemError, self).__init__(reason, verb, tb)
def get_message(self):
# Use a nicer English phrasing for some specific verbs.
if self.verb in ('move', 'copy'):
clause = 'while {0} {1} to {2}'.format(
self._gerund(), repr(self.paths[0]), repr(self.paths[1])
)
elif self.verb in ('delete',):
clause = 'while {0} {1}'.format(
self._gerund(), repr(self.paths[0])
)
else:
clause = 'during {0} of paths {1}'.format(
self.verb, u', '.join(repr(p) for p in paths)
self.verb, u', '.join(repr(p) for p in self.paths)
)
if isinstance(reason, basestring):
reason_str = reason
elif hasattr(reason, 'strerror'): # i.e., EnvironmentError
reason_str = reason.strerror
else:
reason_str = u'"{0}"'.format(reason)
msg = u'{0} {1}'.format(reason_str, clause)
super(FilesystemError, self).__init__(msg)
return u'{0} {1}'.format(self._reasonstr(), clause)
def normpath(path):
"""Provide the canonical form of the path suitable for storing in
@ -264,7 +311,7 @@ def remove(path, soft=True):
try:
os.remove(path)
except (OSError, IOError) as exc:
raise FilesystemError(exc, 'delete', (path,))
raise FilesystemError(exc, 'delete', (path,), traceback.format_exc())
def copy(path, dest, replace=False, pathmod=os.path):
"""Copy a plain file. Permissions are not copied. If dest already
@ -281,7 +328,8 @@ def copy(path, dest, replace=False, pathmod=os.path):
try:
shutil.copyfile(path, dest)
except (OSError, IOError) as exc:
raise FilesystemError(exc, 'copy', (path, dest))
raise FilesystemError(exc, 'copy', (path, dest),
traceback.format_exc())
def move(path, dest, replace=False, pathmod=os.path):
"""Rename a file. dest may not be a directory. If dest already
@ -293,11 +341,13 @@ def move(path, dest, replace=False, pathmod=os.path):
path = syspath(path)
dest = syspath(dest)
if pathmod.exists(dest):
raise FilesystemError('file exists', 'move', (path, dest))
raise FilesystemError('file exists', 'move', (path, dest),
traceback.format_exc())
try:
shutil.move(path, dest)
except (OSError, IOError) as exc:
raise FilesystemError(exc, 'move', (path, dest))
raise FilesystemError(exc, 'move', (path, dest),
traceback.format_exc())
def unique_path(path):
"""Returns a version of ``path`` that does not exist on the

View file

@ -13,7 +13,8 @@ Changelog
halting the importer.
* Similarly, filesystem manipulation errors now print helpful error messages
instead of a messy traceback. They still interrupt beets, but they should now
be easier for users to understand.
be easier for users to understand. Tracebacks are still available in verbose
mode.
* New plugin event: ``import_task_choice`` is called after an import task has an
action assigned.
* New plugin event: ``library_opened`` is called when beets starts up and