core: Update LibraryController to catch backend exceptions

mopidy/core/library.py

@@ -5,7 +5,6 @@ import logging
 import operator
 import urlparse
 
-import pykka
 
 from mopidy.utils import deprecation
 
@@ -70,9 +69,16 @@ class LibraryController(object):
         .. versionadded:: 0.18
         """
         if uri is None:
+            directories = set()
             backends = self.backends.with_library_browse.values()
-            unique_dirs = {b.library.root_directory.get() for b in backends}
+            futures = {b: b.library.root_directory for b in backends}
-            return sorted(unique_dirs, key=operator.attrgetter('name'))
+            for backend, future in futures.items():
+                try:
+                    directories.add(future.get())
+                except Exception:
+                    logger.exception('%s backend caused an exception.',
+                                     backend.actor_ref.actor_class.__name__)
+            return sorted(directories, key=operator.attrgetter('name'))
 
         scheme = urlparse.urlparse(uri).scheme
         backend = self.backends.with_library_browse.get(scheme)
@@ -96,11 +102,15 @@ class LibraryController(object):
 
         .. versionadded:: 1.0
         """
-        futures = [b.library.get_distinct(field, query)
-                   for b in self.backends.with_library.values()]
         result = set()
-        for r in pykka.get_all(futures):
+        futures = {b: b.library.get_distinct(field, query)
-            result.update(r)
+                   for b in self.backends.with_library.values()}
+        for backend, future in futures.items():
+            try:
+                result.update(future.get())
+            except Exception:
+                logger.exception('%s backend caused an exception.',
+                                 backend.actor_ref.actor_class.__name__)
         return result
 
     def get_images(self, uris):
@@ -118,15 +128,19 @@ class LibraryController(object):
 
         .. versionadded:: 1.0
         """
-        futures = [
+        futures = {
-            backend.library.get_images(backend_uris)
+            backend: backend.library.get_images(backend_uris)
             for (backend, backend_uris)
-            in self._get_backends_to_uris(uris).items() if backend_uris]
+            in self._get_backends_to_uris(uris).items() if backend_uris}
 
         results = {uri: tuple() for uri in uris}
-        for r in pykka.get_all(futures):
+        for backend, future in futures.items():
-            for uri, images in r.items():
+            try:
-                results[uri] += tuple(images)
+                for uri, images in future.get().items():
+                    results[uri] += tuple(images)
+            except Exception:
+                logger.exception('%s backend caused an exception.',
+                                 backend.actor_ref.actor_class.__name__)
         return results
 
     def find_exact(self, query=None, uris=None, **kwargs):
@@ -171,19 +185,19 @@ class LibraryController(object):
             uris = [uri]
 
         futures = {}
-        result = {}
+        result = {u: [] for u in uris}
-        backends = self._get_backends_to_uris(uris)
 
         # TODO: lookup(uris) to backend APIs
-        for backend, backend_uris in backends.items():
+        for backend, backend_uris in self._get_backends_to_uris(uris).items():
-            for u in backend_uris or []:
+            for u in backend_uris:
-                futures[u] = backend.library.lookup(u)
+                futures[(backend, u)] = backend.library.lookup(u)
 
-        for u in uris:
+        for (backend, u), future in futures.items():
-            if u in futures:
+            try:
-                result[u] = futures[u].get()
+                result[u] = future.get()
-            else:
+            except Exception:
-                result[u] = []
+                logger.exception('%s backend caused an exception.',
+                                 backend.actor_ref.actor_class.__name__)
 
         if uri:
             return result[uri]
@@ -199,11 +213,20 @@ class LibraryController(object):
         if uri is not None:
             backend = self._get_backend(uri)
             if backend:
-                backend.library.refresh(uri).get()
+                try:
+                    backend.library.refresh(uri).get()
+                except Exception:
+                    logger.exception('%s backend caused an exception.',
+                                     backend.actor_ref.actor_class.__name__)
         else:
-            futures = [b.library.refresh(uri)
+            futures = {b: b.library.refresh(uri)
-                       for b in self.backends.with_library.values()]
+                       for b in self.backends.with_library.values()}
-            pykka.get_all(futures)
+            for backend, future in futures.items():
+                try:
+                    future.get()
+                except Exception:
+                    logger.exception('%s backend caused an exception.',
+                                     backend.actor_ref.actor_class.__name__)
 
     def search(self, query=None, uris=None, exact=False, **kwargs):
         """
@@ -273,6 +296,12 @@ class LibraryController(object):
                 logger.warning(
                     '%s does not implement library.search() with "exact" '
                     'support. Please upgrade it.', backend_name)
+            except LookupError:
+                raise
+            except Exception:
+                logger.exception('%s backend caused an exception.',
+                                 backend.actor_ref.actor_class.__name__)
+
         return [r for r in results if r]
 
 

tests/core/test_library.py

@@ -419,3 +419,71 @@ class LegacyFindExactToSearchLibraryTest(unittest.TestCase):
         self.backend.library.search.return_value.get.side_effect = TypeError
         self.core.library.search(query={'any': ['a']}, exact=True)
         # We are just testing that this doesn't fail.
+
+
+@mock.patch('mopidy.core.library.logger')
+class BackendFailuresCoreLibraryTest(unittest.TestCase):
+
+    def setUp(self):  # noqa: N802
+        dummy_root = Ref.directory(uri='dummy:directory', name='dummy')
+
+        self.library = mock.Mock(spec=backend.LibraryProvider)
+        self.library.root_directory.get.return_value = dummy_root
+
+        self.backend = mock.Mock()
+        self.backend.actor_ref.actor_class.__name__ = 'DummyBackend'
+        self.backend.uri_schemes.get.return_value = ['dummy']
+        self.backend.library = self.library
+
+        self.core = core.Core(mixer=None, backends=[self.backend])
+
+    def test_browse_backend_get_root_exception_gets_ignored(self, logger):
+        # Might happen if root_directory is a property for some weird reason.
+        self.library.root_directory.get.side_effect = Exception
+        self.assertEqual([], self.core.library.browse(None))
+        logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
+
+    def test_browse_backend_browse_uri_exception_gets_through(self, logger):
+        # TODO: is this behavior desired?
+        self.library.browse.return_value.get.side_effect = Exception
+        with self.assertRaises(Exception):
+            self.core.library.browse('dummy:directory')
+
+    def test_get_distinct_backend_exception_gets_ignored(self, logger):
+        self.library.get_distinct.return_value.get.side_effect = Exception
+        self.assertEqual(set(), self.core.library.get_distinct('artist'))
+        logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
+
+    def test_get_images_backend_exception_get_ignored(self, logger):
+        self.library.get_images.return_value.get.side_effect = Exception
+        self.assertEqual(
+            {'dummy:/1': tuple()}, self.core.library.get_images(['dummy:/1']))
+        logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
+
+    def test_lookup_backend_exceptiosn_gets_ignores(self, logger):
+        self.library.lookup.return_value.get.side_effect = Exception
+        self.assertEqual(
+            {'dummy:/1': []}, self.core.library.lookup(uris=['dummy:/1']))
+        logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
+
+    def test_refresh_backend_exception_gets_ignored(self, logger):
+        self.library.refresh.return_value.get.side_effect = Exception
+        self.core.library.refresh()
+        logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
+
+    def test_refresh_uri_backend_exception_gets_ignored(self, logger):
+        self.library.refresh.return_value.get.side_effect = Exception
+        self.core.library.refresh('dummy:/1')
+        logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
+
+    def test_search_backend_exception_gets_ignored(self, logger):
+        self.library.search.return_value.get.side_effect = Exception
+        self.assertEqual([], self.core.library.search(query={'any': ['foo']}))
+        logger.exception.assert_called_with(mock.ANY, 'DummyBackend')
+
+    def test_search_backend_lookup_error_gets_through(self, logger):
+        # TODO: is this behavior desired? Do we need to continue handling
+        # LookupError case specially.
+        self.library.search.return_value.get.side_effect = LookupError
+        with self.assertRaises(LookupError):
+            self.core.library.search(query={'any': ['foo']})