mirror of
https://github.com/beetbox/beets.git
synced 2026-01-30 03:54:21 +01:00
mediafile: Cleanup mutagen error handling
Instead of the individial mutagen format exceptions use the mutagen.MutagenError exception introduced in 1.25. Since 1.33 mutagen will only raise MutagenError for load/save/delete and no longer raise IOError. Translate both errors to UnreadableFileError to support older and newer mutagen versions. Unify error handling in __init__(), save() and delete(). Since it's no longer possible to get an IOError from MediaFile, adjust all callers and tests accordingly. This was tested with mutagen 1.27 and current mutagen master.
This commit is contained in:
parent
45404bce85
commit
629241efd3
5 changed files with 65 additions and 47 deletions
|
|
@ -26,7 +26,7 @@ import six
|
|||
from unidecode import unidecode
|
||||
|
||||
from beets import logging
|
||||
from beets.mediafile import MediaFile, MutagenError, UnreadableFileError
|
||||
from beets.mediafile import MediaFile, UnreadableFileError
|
||||
from beets import plugins
|
||||
from beets import util
|
||||
from beets.util import bytestring_path, syspath, normpath, samefile
|
||||
|
|
@ -568,7 +568,7 @@ class Item(LibModel):
|
|||
read_path = normpath(read_path)
|
||||
try:
|
||||
mediafile = MediaFile(syspath(read_path))
|
||||
except (OSError, IOError, UnreadableFileError) as exc:
|
||||
except UnreadableFileError as exc:
|
||||
raise ReadError(read_path, exc)
|
||||
|
||||
for key in self._media_fields:
|
||||
|
|
@ -615,14 +615,14 @@ class Item(LibModel):
|
|||
try:
|
||||
mediafile = MediaFile(syspath(path),
|
||||
id3v23=beets.config['id3v23'].get(bool))
|
||||
except (OSError, IOError, UnreadableFileError) as exc:
|
||||
except UnreadableFileError as exc:
|
||||
raise ReadError(self.path, exc)
|
||||
|
||||
# Write the tags to the file.
|
||||
mediafile.update(item_tags)
|
||||
try:
|
||||
mediafile.save()
|
||||
except (OSError, IOError, MutagenError) as exc:
|
||||
except UnreadableFileError as exc:
|
||||
raise WriteError(self.path, exc)
|
||||
|
||||
# The file has a new mtime.
|
||||
|
|
|
|||
|
|
@ -1346,32 +1346,12 @@ class MediaFile(object):
|
|||
path = syspath(path)
|
||||
self.path = path
|
||||
|
||||
unreadable_exc = (
|
||||
mutagen.mp3.error,
|
||||
mutagen.id3.error,
|
||||
mutagen.flac.error,
|
||||
mutagen.monkeysaudio.MonkeysAudioHeaderError,
|
||||
mutagen.mp4.error,
|
||||
mutagen.oggopus.error,
|
||||
mutagen.oggvorbis.error,
|
||||
mutagen.ogg.error,
|
||||
mutagen.asf.error,
|
||||
mutagen.apev2.error,
|
||||
mutagen.aiff.error,
|
||||
)
|
||||
try:
|
||||
self.mgfile = mutagen.File(path)
|
||||
except unreadable_exc as exc:
|
||||
log.debug(u'header parsing failed: {0}', six.text_type(exc))
|
||||
except (mutagen.MutagenError, IOError) as exc:
|
||||
# Mutagen <1.33 could raise IOError
|
||||
log.debug(u'parsing failed: {0}', six.text_type(exc))
|
||||
raise UnreadableFileError(path)
|
||||
except IOError as exc:
|
||||
if type(exc) == IOError:
|
||||
# This is a base IOError, not a subclass from Mutagen or
|
||||
# anywhere else.
|
||||
raise
|
||||
else:
|
||||
log.debug(u'{}', traceback.format_exc())
|
||||
raise MutagenError(path, exc)
|
||||
except Exception as exc:
|
||||
# Isolate bugs in Mutagen.
|
||||
log.debug(u'{}', traceback.format_exc())
|
||||
|
|
@ -1418,7 +1398,8 @@ class MediaFile(object):
|
|||
self.id3v23 = id3v23 and self.type == 'mp3'
|
||||
|
||||
def save(self):
|
||||
"""Write the object's tags back to the file.
|
||||
"""Write the object's tags back to the file. May
|
||||
throw `UnreadableFileError`.
|
||||
"""
|
||||
# Possibly save the tags to ID3v2.3.
|
||||
kwargs = {}
|
||||
|
|
@ -1430,28 +1411,41 @@ class MediaFile(object):
|
|||
id3.update_to_v23()
|
||||
kwargs['v2_version'] = 3
|
||||
|
||||
# Isolate bugs in Mutagen.
|
||||
try:
|
||||
self.mgfile.save(**kwargs)
|
||||
except (IOError, OSError):
|
||||
# Propagate these through: they don't represent Mutagen bugs.
|
||||
raise
|
||||
except (mutagen.MutagenError, IOError) as exc:
|
||||
# Mutagen <1.33 could raise IOError
|
||||
log.debug(u'saving failed: {0}', six.text_type(exc))
|
||||
raise UnreadableFileError(self.path)
|
||||
except Exception as exc:
|
||||
# Isolate bugs in Mutagen.
|
||||
log.debug(u'{}', traceback.format_exc())
|
||||
log.error(u'uncaught Mutagen exception in save: {0}', exc)
|
||||
raise MutagenError(self.path, exc)
|
||||
|
||||
def delete(self):
|
||||
"""Remove the current metadata tag from the file.
|
||||
"""Remove the current metadata tag from the file. May
|
||||
throw `UnreadableFileError`.
|
||||
"""
|
||||
|
||||
try:
|
||||
self.mgfile.delete()
|
||||
except NotImplementedError:
|
||||
# FIXME: This is fixed in mutagen >=1.31
|
||||
# For Mutagen types that don't support deletion (notably,
|
||||
# ASF), just delete each tag individually.
|
||||
for tag in self.mgfile.keys():
|
||||
del self.mgfile[tag]
|
||||
try:
|
||||
self.mgfile.delete()
|
||||
except NotImplementedError:
|
||||
# FIXME: This is fixed in mutagen >=1.31
|
||||
# For Mutagen types that don't support deletion (notably,
|
||||
# ASF), just delete each tag individually.
|
||||
for tag in self.mgfile.keys():
|
||||
del self.mgfile[tag]
|
||||
except (mutagen.MutagenError, IOError) as exc:
|
||||
# Mutagen <1.33 could raise IOError
|
||||
log.debug(u'deleting failed: {0}', six.text_type(exc))
|
||||
raise UnreadableFileError(self.path)
|
||||
except Exception as exc:
|
||||
# Isolate bugs in Mutagen.
|
||||
log.debug(u'{}', traceback.format_exc())
|
||||
log.error(u'uncaught Mutagen exception in save: {0}', exc)
|
||||
raise MutagenError(self.path, exc)
|
||||
|
||||
# Convenient access to the set of available fields.
|
||||
|
||||
|
|
|
|||
|
|
@ -119,7 +119,7 @@ class ScrubPlugin(BeetsPlugin):
|
|||
try:
|
||||
mf = mediafile.MediaFile(util.syspath(item.path),
|
||||
config['id3v23'].get(bool))
|
||||
except IOError as exc:
|
||||
except mediafile.UnreadableFileError as exc:
|
||||
self._log.error(u'could not open file to scrub: {0}',
|
||||
exc)
|
||||
art = mf.art
|
||||
|
|
@ -133,10 +133,13 @@ class ScrubPlugin(BeetsPlugin):
|
|||
item.try_write()
|
||||
if art:
|
||||
self._log.debug(u'restoring art')
|
||||
mf = mediafile.MediaFile(util.syspath(item.path),
|
||||
config['id3v23'].get(bool))
|
||||
mf.art = art
|
||||
mf.save()
|
||||
try:
|
||||
mf = mediafile.MediaFile(util.syspath(item.path),
|
||||
config['id3v23'].get(bool))
|
||||
mf.art = art
|
||||
mf.save()
|
||||
except mediafile.UnreadableFileError as exc:
|
||||
self._log.error(u'could not write tags: {0}', exc)
|
||||
|
||||
def import_task_files(self, session, task):
|
||||
"""Automatically scrub imported files."""
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ from test import _common
|
|||
from test._common import unittest
|
||||
from beets.mediafile import MediaFile, MediaField, Image, \
|
||||
MP3DescStorageStyle, StorageStyle, MP4StorageStyle, \
|
||||
ASFStorageStyle, ImageType, CoverArtField
|
||||
ASFStorageStyle, ImageType, CoverArtField, UnreadableFileError
|
||||
from beets.library import Item
|
||||
from beets.plugins import BeetsPlugin
|
||||
from beets.util import bytestring_path
|
||||
|
|
@ -455,6 +455,27 @@ class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin,
|
|||
if os.path.isdir(self.temp_dir):
|
||||
shutil.rmtree(self.temp_dir)
|
||||
|
||||
def test_read_nonexisting(self):
|
||||
mediafile = self._mediafile_fixture('full')
|
||||
os.remove(mediafile.path)
|
||||
self.assertRaises(UnreadableFileError, MediaFile, mediafile.path)
|
||||
|
||||
def test_save_nonexisting(self):
|
||||
mediafile = self._mediafile_fixture('full')
|
||||
os.remove(mediafile.path)
|
||||
try:
|
||||
mediafile.save()
|
||||
except UnreadableFileError:
|
||||
pass
|
||||
|
||||
def test_delete_nonexisting(self):
|
||||
mediafile = self._mediafile_fixture('full')
|
||||
os.remove(mediafile.path)
|
||||
try:
|
||||
mediafile.delete()
|
||||
except UnreadableFileError:
|
||||
pass
|
||||
|
||||
def test_read_audio_properties(self):
|
||||
mediafile = self._mediafile_fixture('full')
|
||||
for key, value in self.audio_properties.items():
|
||||
|
|
|
|||
|
|
@ -193,7 +193,7 @@ class SafetyTest(unittest.TestCase, TestHelper):
|
|||
fn = os.path.join(_common.RSRC, b'brokenlink')
|
||||
os.symlink('does_not_exist', fn)
|
||||
try:
|
||||
self.assertRaises(IOError,
|
||||
self.assertRaises(beets.mediafile.UnreadableFileError,
|
||||
beets.mediafile.MediaFile, fn)
|
||||
finally:
|
||||
os.unlink(fn)
|
||||
|
|
|
|||
Loading…
Reference in a new issue