Merge pull request #603 from geigerzaehler/write-exception-handling

Exception handling for item writes

beets/importer.py

@@ -895,13 +895,9 @@ def manipulate_files(session):
             if config['import']['write'] and task.should_write_tags():
                 try:
                     item.write()
-                except mediafile.UnreadableFileError as exc:
-                    log.error(u'error while writing ({0}): {0}'.format(
-                        exc,
-                        util.displayable_path(item.path)
-                    ))
-                except util.FilesystemError as exc:
-                    exc.log(log)
+                except library.FileOperationError as exc:
+                    log.error(u'could not write {0}: {1}'.format(
+                                  util.displayable_path(item.path), exc))
 
         # Save new paths.
         with session.lib.transaction():

beets/library.py

@@ -23,7 +23,7 @@ import unicodedata
 import traceback
 import time
 from unidecode import unidecode
-from beets.mediafile import MediaFile
+from beets.mediafile import MediaFile, MutagenError
 from beets import plugins
 from beets import util
 from beets.util import bytestring_path, syspath, normpath, samefile
@@ -283,6 +283,16 @@ class LibModel(dbcore.Model):
         super(LibModel, self).add(lib)
         plugins.send('database_change', lib=self._db)
 
+class FileOperationError(Exception):
+    """Raised by ``item.write()`` to indicate an error when interacting
+    with the file.
+    """
+
+class ReadError(FileOperationError):
+    pass
+
+class WriteError(FileOperationError):
+    pass
 
 class Item(LibModel):
     _fields = dict((name, typ) for (name, typ, _, _) in ITEM_FIELDS)
@@ -342,6 +352,8 @@ class Item(LibModel):
     def read(self, read_path=None):
         """Read the metadata from the associated file. If read_path is
         specified, read metadata from that file instead.
+
+        Raises ``ReadError`` if the file could not be read.
         """
         if read_path is None:
             read_path = self.path
@@ -350,8 +362,7 @@ class Item(LibModel):
         try:
             f = MediaFile(syspath(read_path))
         except (OSError, IOError) as exc:
-            raise util.FilesystemError(exc, 'read', (read_path,),
-                                       traceback.format_exc())
+            raise ReadError(exc.message)
 
         for key in ITEM_KEYS_META:
             value = getattr(f, key)
@@ -371,24 +382,23 @@ class Item(LibModel):
         self.path = read_path
 
     def write(self):
-        """Writes the item's metadata to the associated file.
-        """
-        plugins.send('write', item=self)
+        """Write the item's metadata to the associated file.
 
+        Raises ``ReadError`` or ``WriteError``.
+        """
         try:
             f = MediaFile(syspath(self.path))
         except (OSError, IOError) as exc:
-            raise util.FilesystemError(exc, 'read', (self.path,),
-                                       traceback.format_exc())
+            raise ReadError(str(exc))
+
+        plugins.send('write', item=self)
 
         for key in ITEM_KEYS_WRITABLE:
             setattr(f, key, self[key])
-
         try:
             f.save(id3v23=beets.config['id3v23'].get(bool))
-        except (OSError, IOError) as exc:
-            raise util.FilesystemError(exc, 'write', (self.path,),
-                                       traceback.format_exc())
+        except (OSError, IOError, MutagenError) as exc:
+            raise WriteError(str(exc))
 
         # The file has a new mtime.
         self.mtime = self.current_mtime()

beets/plugins.py

@@ -353,10 +353,7 @@ def send(event, **arguments):
     name of  the event to send, all other named arguments go to the
     event handler(s).
 
-    Returns the number of handlers called.
+    Returns a list of return values from the handlers.
     """
     log.debug('Sending event: %s' % event)
-    handlers = event_handlers()[event]
-    for handler in handlers:
-        handler(**arguments)
-    return len(handlers)
+    return [handler(**arguments) for handler in event_handlers()[event]]

beets/ui/commands.py

@@ -77,7 +77,6 @@ def _do_query(lib, query, album, also_items=True):
     return items, albums
 
 
-
 # fields: Shows a list of available fields for queries and format strings.
 
 fields_cmd = ui.Subcommand('fields',
@@ -1135,7 +1134,11 @@ def modify_items(lib, mods, query, write, move, album, confirm):
         else:
             changed_items = changed
         for item in changed_items:
-            item.write()
+            try:
+                item.write()
+            except library.FileOperationError as exc:
+                log.error(u'could not write {0}: {1}'.format(
+                              util.displayable_path(item.path), exc))
 
 modify_cmd = ui.Subcommand('modify',
     help='change metadata fields', aliases=('mod',))
@@ -1234,11 +1237,9 @@ def write_items(lib, query, pretend):
         if changed and not pretend:
             try:
                 item.write()
-            except Exception as exc:
+            except library.FileOperationError as exc:
                 log.error(u'could not write {0}: {1}'.format(
-                    util.displayable_path(item.path), exc
-                ))
-                continue
+                              util.displayable_path(item.path), exc))
 
 write_cmd = ui.Subcommand('write', help='write tag information to files')
 write_cmd.parser.add_option('-p', '--pretend', action='store_true',

docs/dev/plugins.rst

@@ -141,7 +141,10 @@ currently available are:
   deleted from disk).
 
 * *write*: called with an ``Item`` object just before a file's metadata is
-  written to disk (i.e., just before the file on disk is opened).
+  written to disk (i.e., just before the file on disk is opened). Event
+  handlers may raise a ``library.FileOperationError`` exception to abort
+  the write operation. Beets will catch that exception, print an error
+  message and continue.
 
 * *after_write*: called with an ``Item`` object after a file's metadata is
   written to disk (i.e., just after the file on disk is closed).

test/test_library.py

@@ -15,6 +15,8 @@
 """Tests for non-query database functions of Item.
 """
 import os
+import os.path
+import stat
 import shutil
 import re
 import unicodedata
@@ -942,6 +944,20 @@ class TemplateTest(_common.LibTestCase):
         self.album.store()
         self.assertEqual(self.i.evaluate_template('$foo'), 'baz')
 
+class WriteTest(_common.LibTestCase):
+
+    def test_write_nonexistant(self):
+        self.i.path = '/path/does/not/exist'
+        self.assertRaises(beets.library.ReadError, self.i.write)
+
+    def test_no_write_permission(self):
+        path = os.path.join(self.temp_dir, 'file.mp3')
+        shutil.copy(os.path.join(_common.RSRC, 'empty.mp3'), path)
+        os.chmod(path, stat.S_IRUSR)
+
+        self.i.path = path
+        self.assertRaises(beets.library.WriteError, self.i.write)
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)