From 92061099feeeee644ae4536f1ca512051a69f72a Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 11 Mar 2014 16:49:04 +0100
Subject: [PATCH 1/8] Add BeforeWriteError for plugins

The idea is that plugins may want to prevent beets from writing a file (for
example if an integrity check failed).
---
 beets/importer.py    | 11 +++++------
 beets/plugins.py     |  9 +++++++++
 docs/dev/plugins.rst |  4 +++-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/beets/importer.py b/beets/importer.py
index 00510e47d..6bce22fea 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -895,13 +895,12 @@ 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 (mediafile.UnreadableFileError,
+                        util.FilesystemError,
+                        plugins.BeforeWriteError) as exc:
+                    log.error(u'could not write {0}: {1}'.format(
+                        util.displayable_path(item.path), exc
                     ))
-                except util.FilesystemError as exc:
-                    exc.log(log)
 
         # Save new paths.
         with session.lib.transaction():
diff --git a/beets/plugins.py b/beets/plugins.py
index 813084ca9..da4588a5c 100755
--- a/beets/plugins.py
+++ b/beets/plugins.py
@@ -30,6 +30,15 @@ LASTFM_KEY = '2dc3914abf35f0d9c92d97d8f8e42b43'
 log = logging.getLogger('beets')
 
 
+class BeforeWriteError(Exception):
+    """May be raised by plugins in a ``write`` event handler to abort
+    prevent writing the item's file.
+
+    Beets will catch this exception during a call to ``item.write()``
+    and display it to the user as an error.
+    """
+
+
 # Managing the plugins themselves.
 
 class BeetsPlugin(object):
diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst
index f21f41a38..418414806 100644
--- a/docs/dev/plugins.rst
+++ b/docs/dev/plugins.rst
@@ -141,7 +141,9 @@ 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 ``plugins.BeforeWriteError`` to prevent beets from
+  writing the file and display an error to the user.
 
 * *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).

From 13cbcad5819ed7e456f6568850b304f2168e3162 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 11 Mar 2014 16:54:25 +0100
Subject: [PATCH 2/8] Unify exception handling of `item.write()` in UI
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Don’t abort the program when there is something wrong with the filesystem.
---
 beets/importer.py    |  1 +
 beets/ui/commands.py | 15 +++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/beets/importer.py b/beets/importer.py
index 6bce22fea..8e8d5587c 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -893,6 +893,7 @@ def manipulate_files(session):
                     item.move(True)
 
             if config['import']['write'] and task.should_write_tags():
+                # FIXME duplicates code from `ui.commands.write_items`.
                 try:
                     item.write()
                 except (mediafile.UnreadableFileError,
diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index de6349532..bebdcaa48 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -1135,7 +1135,16 @@ def modify_items(lib, mods, query, write, move, album, confirm):
         else:
             changed_items = changed
         for item in changed_items:
-            item.write()
+            # FIXME duplicates code from `ui.commands.write_items`.
+            try:
+                item.write()
+            except (mediafile.UnreadableFileError,
+                    util.FilesystemError,
+                    plugins.BeforeWriteError) as exc:
+                log.error(u'could not write {0}: {1}'.format(
+                    util.displayable_path(item.path), exc
+                ))
+                continue
 
 modify_cmd = ui.Subcommand('modify',
     help='change metadata fields', aliases=('mod',))
@@ -1234,7 +1243,9 @@ def write_items(lib, query, pretend):
         if changed and not pretend:
             try:
                 item.write()
-            except Exception as exc:
+            except (mediafile.UnreadableFileError,
+                    util.FilesystemError,
+                    plugins.BeforeWriteError) as exc:
                 log.error(u'could not write {0}: {1}'.format(
                     util.displayable_path(item.path), exc
                 ))

From ddf5233daad6bf07b2a792d470f356e1166de163 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 11 Mar 2014 17:12:21 +0100
Subject: [PATCH 3/8] Add _item_write method and remove duplicate code

---
 beets/importer.py    | 11 ++---------
 beets/ui/commands.py | 34 +++++++++++++++-------------------
 2 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/beets/importer.py b/beets/importer.py
index 8e8d5587c..eca578f6f 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -29,6 +29,7 @@ from beets import dbcore
 from beets import plugins
 from beets import util
 from beets import config
+from beets import ui
 from beets.util import pipeline
 from beets.util import syspath, normpath, displayable_path
 from beets.util.enumeration import enum
@@ -893,15 +894,7 @@ def manipulate_files(session):
                     item.move(True)
 
             if config['import']['write'] and task.should_write_tags():
-                # FIXME duplicates code from `ui.commands.write_items`.
-                try:
-                    item.write()
-                except (mediafile.UnreadableFileError,
-                        util.FilesystemError,
-                        plugins.BeforeWriteError) as exc:
-                    log.error(u'could not write {0}: {1}'.format(
-                        util.displayable_path(item.path), exc
-                    ))
+                ui.commands._item_write(item)
 
         # Save new paths.
         with session.lib.transaction():
diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index bebdcaa48..dbc6fda3e 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -76,6 +76,19 @@ def _do_query(lib, query, album, also_items=True):
 
     return items, albums
 
+def _item_write(item):
+    """Wrapper for ``item.write()`` that handles exceptions and logs
+    errors during user interaction.
+    """
+    try:
+        item.write()
+    except (mediafile.UnreadableFileError,
+            util.FilesystemError,
+            plugins.BeforeWriteError) as exc:
+        log.error(u'could not write {0}: {1}'.format(
+            util.displayable_path(item.path), exc
+        ))
+
 
 
 # fields: Shows a list of available fields for queries and format strings.
@@ -1135,16 +1148,7 @@ def modify_items(lib, mods, query, write, move, album, confirm):
         else:
             changed_items = changed
         for item in changed_items:
-            # FIXME duplicates code from `ui.commands.write_items`.
-            try:
-                item.write()
-            except (mediafile.UnreadableFileError,
-                    util.FilesystemError,
-                    plugins.BeforeWriteError) as exc:
-                log.error(u'could not write {0}: {1}'.format(
-                    util.displayable_path(item.path), exc
-                ))
-                continue
+            _item_write(item)
 
 modify_cmd = ui.Subcommand('modify',
     help='change metadata fields', aliases=('mod',))
@@ -1241,15 +1245,7 @@ def write_items(lib, query, pretend):
         changed = ui.show_model_changes(item, clean_item,
                                         library.ITEM_KEYS_WRITABLE, always=True)
         if changed and not pretend:
-            try:
-                item.write()
-            except (mediafile.UnreadableFileError,
-                    util.FilesystemError,
-                    plugins.BeforeWriteError) as exc:
-                log.error(u'could not write {0}: {1}'.format(
-                    util.displayable_path(item.path), exc
-                ))
-                continue
+            _item_write(item)
 
 write_cmd = ui.Subcommand('write', help='write tag information to files')
 write_cmd.parser.add_option('-p', '--pretend', action='store_true',

From a744e8ea597e64a74fb00f61403ddbab5da50c2a Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 11 Mar 2014 19:42:06 +0100
Subject: [PATCH 4/8] Fix missing import

---
 beets/ui/commands.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index dbc6fda3e..75ad37b49 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -25,7 +25,7 @@ import codecs
 import platform
 
 import beets
-from beets import ui
+from beets import ui, mediafile
 from beets.ui import print_, input_, decargs
 from beets import autotag
 from beets.autotag import recommendation

From a399f294e8eb2f4dc2197604af6d688ed2ab1d36 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Thu, 13 Mar 2014 13:46:07 +0100
Subject: [PATCH 5/8] Handle exceptions in item.write and use plugin abort

---
 beets/importer.py    |  2 +-
 beets/library.py     | 26 ++++++++++++++++++++------
 beets/plugins.py     | 16 ++--------------
 beets/ui/commands.py | 18 ++----------------
 docs/dev/plugins.rst |  4 ++--
 5 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/beets/importer.py b/beets/importer.py
index eca578f6f..93d2bbed5 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -894,7 +894,7 @@ def manipulate_files(session):
                     item.move(True)
 
             if config['import']['write'] and task.should_write_tags():
-                ui.commands._item_write(item)
+                item.write()
 
         # Save new paths.
         with session.lib.transaction():
diff --git a/beets/library.py b/beets/library.py
index 49ba4a9ca..100c0201b 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -371,15 +371,26 @@ class Item(LibModel):
         self.path = read_path
 
     def write(self):
-        """Writes the item's metadata to the associated file.
+        """Try to write the item's metadata to the associated file.
+
+        Returns ``True`` if the write was successful. The method catches
+        file system read and write exceptions and logs an error message.
+        If any of 'write' event handlers returns a truthy value the
+        write will not be performed and an error message is logged.
         """
-        plugins.send('write', item=self)
+        if any(plugins.send('write', item=self)):
+            log.error(u'plugin aborted writing {0}'.format(
+                           util.displayable_path(item.path)))
+            return
+
 
         try:
             f = MediaFile(syspath(self.path))
         except (OSError, IOError) as exc:
-            raise util.FilesystemError(exc, 'read', (self.path,),
-                                       traceback.format_exc())
+            log.error(u'could not read {0}: {1}'.format(
+                util.displayable_path(item.path), exc
+            ))
+            return
 
         for key in ITEM_KEYS_WRITABLE:
             setattr(f, key, self[key])
@@ -387,12 +398,15 @@ class Item(LibModel):
         try:
             f.save(id3v23=beets.config['id3v23'].get(bool))
         except (OSError, IOError) as exc:
-            raise util.FilesystemError(exc, 'write', (self.path,),
-                                       traceback.format_exc())
+            log.error(u'could not write {0}: {1}'.format(
+                util.displayable_path(item.path), exc
+            ))
+            return
 
         # The file has a new mtime.
         self.mtime = self.current_mtime()
         plugins.send('after_write', item=self)
+        return True
 
 
     # Files themselves.
diff --git a/beets/plugins.py b/beets/plugins.py
index da4588a5c..6a58777cd 100755
--- a/beets/plugins.py
+++ b/beets/plugins.py
@@ -30,15 +30,6 @@ LASTFM_KEY = '2dc3914abf35f0d9c92d97d8f8e42b43'
 log = logging.getLogger('beets')
 
 
-class BeforeWriteError(Exception):
-    """May be raised by plugins in a ``write`` event handler to abort
-    prevent writing the item's file.
-
-    Beets will catch this exception during a call to ``item.write()``
-    and display it to the user as an error.
-    """
-
-
 # Managing the plugins themselves.
 
 class BeetsPlugin(object):
@@ -362,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]]
diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index 75ad37b49..1fd18f7e0 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -76,20 +76,6 @@ def _do_query(lib, query, album, also_items=True):
 
     return items, albums
 
-def _item_write(item):
-    """Wrapper for ``item.write()`` that handles exceptions and logs
-    errors during user interaction.
-    """
-    try:
-        item.write()
-    except (mediafile.UnreadableFileError,
-            util.FilesystemError,
-            plugins.BeforeWriteError) as exc:
-        log.error(u'could not write {0}: {1}'.format(
-            util.displayable_path(item.path), exc
-        ))
-
-
 
 # fields: Shows a list of available fields for queries and format strings.
 
@@ -1148,7 +1134,7 @@ def modify_items(lib, mods, query, write, move, album, confirm):
         else:
             changed_items = changed
         for item in changed_items:
-            _item_write(item)
+            item.write()
 
 modify_cmd = ui.Subcommand('modify',
     help='change metadata fields', aliases=('mod',))
@@ -1245,7 +1231,7 @@ def write_items(lib, query, pretend):
         changed = ui.show_model_changes(item, clean_item,
                                         library.ITEM_KEYS_WRITABLE, always=True)
         if changed and not pretend:
-            _item_write(item)
+            item.write()
 
 write_cmd = ui.Subcommand('write', help='write tag information to files')
 write_cmd.parser.add_option('-p', '--pretend', action='store_true',
diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst
index 418414806..408c002c3 100644
--- a/docs/dev/plugins.rst
+++ b/docs/dev/plugins.rst
@@ -142,8 +142,8 @@ currently available are:
 
 * *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). Event
-  handlers may raise ``plugins.BeforeWriteError`` to prevent beets from
-  writing the file and display an error to the user.
+  handlers may return a truthy value to prevent beets from writing the
+  file. In that case make sure that you log an appropriate message.
 
 * *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).

From 85b16e4e3d9bb8e3b4203115ff9696f0fbd7a986 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Sat, 22 Mar 2014 13:25:02 +0100
Subject: [PATCH 6/8] Remove unused code

---
 beets/ui/commands.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index 1fd18f7e0..d75e43e77 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -25,7 +25,7 @@ import codecs
 import platform
 
 import beets
-from beets import ui, mediafile
+from beets import ui
 from beets.ui import print_, input_, decargs
 from beets import autotag
 from beets.autotag import recommendation

From 574903e9861d1330c85c6052b7bf1652502daff7 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 24 Mar 2014 14:22:27 +0100
Subject: [PATCH 7/8] Add FileOperationError and handling in item.write()

---
 beets/importer.py    |  6 +++++-
 beets/library.py     | 46 ++++++++++++++++++++------------------------
 beets/ui/commands.py | 12 ++++++++++--
 docs/dev/plugins.rst |  5 +++--
 test/test_library.py | 16 +++++++++++++++
 5 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/beets/importer.py b/beets/importer.py
index 93d2bbed5..c002779df 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -894,7 +894,11 @@ def manipulate_files(session):
                     item.move(True)
 
             if config['import']['write'] and task.should_write_tags():
-                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))
 
         # Save new paths.
         with session.lib.transaction():
diff --git a/beets/library.py b/beets/library.py
index 100c0201b..48806405e 100644
--- a/beets/library.py
+++ b/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,42 +382,27 @@ class Item(LibModel):
         self.path = read_path
 
     def write(self):
-        """Try to write the item's metadata to the associated file.
+        """Write the item's metadata to the associated file.
 
-        Returns ``True`` if the write was successful. The method catches
-        file system read and write exceptions and logs an error message.
-        If any of 'write' event handlers returns a truthy value the
-        write will not be performed and an error message is logged.
+        Raises ``ReadError`` or ``WriteError``.
         """
-        if any(plugins.send('write', item=self)):
-            log.error(u'plugin aborted writing {0}'.format(
-                           util.displayable_path(item.path)))
-            return
-
-
         try:
             f = MediaFile(syspath(self.path))
         except (OSError, IOError) as exc:
-            log.error(u'could not read {0}: {1}'.format(
-                util.displayable_path(item.path), exc
-            ))
-            return
+            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:
-            log.error(u'could not write {0}: {1}'.format(
-                util.displayable_path(item.path), exc
-            ))
-            return
+        except (OSError, IOError, MutagenError) as exc:
+            raise WriteError(str(exc))
 
         # The file has a new mtime.
         self.mtime = self.current_mtime()
         plugins.send('after_write', item=self)
-        return True
 
 
     # Files themselves.
diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index d75e43e77..40c13a998 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -1134,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',))
@@ -1231,7 +1235,11 @@ def write_items(lib, query, pretend):
         changed = ui.show_model_changes(item, clean_item,
                                         library.ITEM_KEYS_WRITABLE, always=True)
         if changed and not pretend:
-            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))
 
 write_cmd = ui.Subcommand('write', help='write tag information to files')
 write_cmd.parser.add_option('-p', '--pretend', action='store_true',
diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst
index 408c002c3..cdea8938a 100644
--- a/docs/dev/plugins.rst
+++ b/docs/dev/plugins.rst
@@ -142,8 +142,9 @@ currently available are:
 
 * *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). Event
-  handlers may return a truthy value to prevent beets from writing the
-  file. In that case make sure that you log an appropriate message.
+  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).
diff --git a/test/test_library.py b/test/test_library.py
index 8eabb4122..3fa3d1173 100644
--- a/test/test_library.py
+++ b/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__)

From 86b2c215f20b2915261c70f6b223fa5bcdc831c7 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 24 Mar 2014 17:55:36 +0100
Subject: [PATCH 8/8] Remove unused import

---
 beets/importer.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/beets/importer.py b/beets/importer.py
index c002779df..a01c21d1e 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -29,7 +29,6 @@ from beets import dbcore
 from beets import plugins
 from beets import util
 from beets import config
-from beets import ui
 from beets.util import pipeline
 from beets.util import syspath, normpath, displayable_path
 from beets.util.enumeration import enum