core: Make sure we handle bad mixer data and exceptions.

mopidy/core/mixer.py

@@ -1,10 +1,24 @@
 from __future__ import absolute_import, unicode_literals
 
+import contextlib
 import logging
 
+from mopidy import exceptions
 from mopidy.utils import validation
 
 
+@contextlib.contextmanager
+def _mixer_error_handling(mixer):
+    try:
+        yield
+    except exceptions.ValidationError as e:
+        logger.error('%s mixer returned bad data: %s',
+                     mixer.actor_ref.actor_class.__name__, e)
+    except Exception:
+        logger.exception('%s mixer caused an exception.',
+                         mixer.actor_ref.actor_class.__name__)
+
+
 logger = logging.getLogger(__name__)
 
 
@@ -21,8 +35,15 @@ class MixerController(object):
 
         The volume scale is linear.
         """
-        if self._mixer is not None:
-            return self._mixer.get_volume().get()
+        if self._mixer is None:
+            return None
+
+        with _mixer_error_handling(self._mixer):
+            volume = self._mixer.get_volume().get()
+            volume is None or validation.check_integer(volume, min=0, max=100)
+            return volume
+
+        return None
 
     def set_volume(self, volume):
         """Set the volume.
@@ -37,8 +58,12 @@ class MixerController(object):
 
         if self._mixer is None:
             return False
-        else:
-            return self._mixer.set_volume(volume).get()
+
+        with _mixer_error_handling(self._mixer):
+            # TODO: log non-bool return values?
+            return bool(self._mixer.set_volume(volume).get())
+
+        return False
 
     def get_mute(self):
         """Get mute state.
@@ -46,8 +71,15 @@ class MixerController(object):
         :class:`True` if muted, :class:`False` unmuted, :class:`None` if
         unknown.
         """
-        if self._mixer is not None:
-            return self._mixer.get_mute().get()
+        if self._mixer is None:
+            return None
+
+        with _mixer_error_handling(self._mixer):
+            mute = self._mixer.get_mute().get()
+            mute is None or validation.check_instance(mute, bool)
+            return mute
+
+        return None
 
     def set_mute(self, mute):
         """Set mute state.
@@ -59,5 +91,9 @@ class MixerController(object):
         validation.check_boolean(mute)
         if self._mixer is None:
             return False
-        else:
-            return self._mixer.set_mute(bool(mute)).get()
+
+        with _mixer_error_handling(self._mixer):
+            # TODO: log non-bool return values?
+            return bool(self._mixer.set_mute(bool(mute)).get())
+
+        return None

tests/core/test_mixer.py

@@ -92,3 +92,51 @@ class CoreNoneMixerListenerTest(unittest.TestCase):
     def test_forwards_mixer_mute_changed_event_to_frontends(self, send):
         self.core.mixer.set_mute(mute=True)
         self.assertEqual(send.call_count, 0)
+
+
+class CoreBadMixerTest(unittest.TestCase):
+
+    def setUp(self):  # noqa: N802
+        self.mixer = mock.Mock()
+        self.mixer.actor_ref.actor_class.__name__ = 'DummyMixer'
+        self.core = core.Core(mixer=self.mixer, backends=[])
+
+    def test_get_volume_raises_exception(self):
+        self.mixer.get_volume.return_value.get.side_effect = Exception
+        self.assertEqual(self.core.mixer.get_volume(), None)
+
+    def test_get_volume_returns_negative(self):
+        self.mixer.get_volume.return_value.get.return_value = -1
+        self.assertEqual(self.core.mixer.get_volume(), None)
+
+    def test_get_volume_returns_out_of_bound(self):
+        self.mixer.get_volume.return_value.get.return_value = 1000
+        self.assertEqual(self.core.mixer.get_volume(), None)
+
+    def test_get_volume_returns_wrong_type(self):
+        self.mixer.get_volume.return_value.get.return_value = '12'
+        self.assertEqual(self.core.mixer.get_volume(), None)
+
+    def test_set_volume_exception(self):
+        self.mixer.set_volume.return_value.get.side_effect = Exception
+        self.assertFalse(self.core.mixer.set_volume(30))
+
+    def test_set_volume_non_bool_return_value(self):
+        self.mixer.set_volume.return_value.get.return_value = 'done'
+        self.assertIs(self.core.mixer.set_volume(30), True)
+
+    def test_get_mute_raises_exception(self):
+        self.mixer.get_mute.return_value.get.side_effect = Exception
+        self.assertEqual(self.core.mixer.get_mute(), None)
+
+    def test_get_mute_returns_wrong_type(self):
+        self.mixer.get_mute.return_value.get.return_value = '12'
+        self.assertEqual(self.core.mixer.get_mute(), None)
+
+    def test_set_mute_exception(self):
+        self.mixer.set_mute.return_value.get.side_effect = Exception
+        self.assertFalse(self.core.mixer.set_mute(True))
+
+    def test_set_mute_non_bool_return_value(self):
+        self.mixer.set_mute.return_value.get.return_value = 'done'
+        self.assertIs(self.core.mixer.set_mute(True), True)