From 2d952570d0085013dd79f577b669f70f6f3d86bc Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 May 2015 00:06:31 +0200 Subject: [PATCH 1/4] main: Catch extension.setup exceptions --- mopidy/__main__.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index ea1cab6b..245a03ce 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -140,7 +140,16 @@ def main(): return 1 for extension in extensions['enabled']: - extension.setup(registry) + try: + extension.setup(registry) + except Exception: + # TODO: would be nice a transactional registry. But sadly this + # is a bit tricky since our current API is giving out a mutable + # list. We might however be able to replace this with a + # collections.Sequence to provide a RO view. + logger.exception('Extension %s failed during setup, this might' + ' have left the registry in a bad state.', + extension.ext_name) # Anything that wants to exit after this point must use # mopidy.internal.process.exit_process as actors can have been started. From cb4b23f416fdf4187982132a09747707e19140a2 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 May 2015 00:35:00 +0200 Subject: [PATCH 2/4] startup: Handle potential mixer startup issues. --- mopidy/commands.py | 26 +++++++++++++++++++------- mopidy/mixer.py | 4 ++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/mopidy/commands.py b/mopidy/commands.py index 24acfb7d..137e4a3e 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -10,6 +10,8 @@ import glib import gobject +import pykka + from mopidy import config as config_lib, exceptions from mopidy.audio import Audio from mopidy.core import Core @@ -230,6 +232,7 @@ class Command(object): raise NotImplementedError +# TODO: move out of this utility class class RootCommand(Command): def __init__(self): @@ -276,6 +279,8 @@ class RootCommand(Command): mixer = None if mixer_class is not None: mixer = self.start_mixer(config, mixer_class) + if mixer: + self.configure_mixer(config, mixer) audio = self.start_audio(config, mixer) backends = self.start_backends(config, backend_classes, audio) core = self.start_core(config, mixer, backends, audio) @@ -322,16 +327,23 @@ class RootCommand(Command): return selected_mixers[0] def start_mixer(self, config, mixer_class): + logger.info('Starting Mopidy mixer: %s', mixer_class.__name__) + mixer = None + try: - logger.info('Starting Mopidy mixer: %s', mixer_class.__name__) mixer = mixer_class.start(config=config).proxy() - self.configure_mixer(config, mixer) - return mixer + mixer.ping().get() except exceptions.MixerError as exc: - logger.error( - 'Mixer (%s) initialization error: %s', - mixer_class.__name__, exc.message) - raise + logger.error('Mixer (%s) initialization error: %s', + mixer_class.__name__, exc.message) + except pykka.ActorDeadError as exc: + mixer = None + logger.error('Mixer actor died: %s', exc) + except Exception: + logger.exception('Mixer (%s) initialization exception:', + mixer_class.__name__) + + return mixer def configure_mixer(self, config, mixer): volume = config['audio']['mixer_volume'] diff --git a/mopidy/mixer.py b/mopidy/mixer.py index b25688fb..eb43d810 100644 --- a/mopidy/mixer.py +++ b/mopidy/mixer.py @@ -110,6 +110,10 @@ class Mixer(object): logger.debug('Mixer event: mute_changed(mute=%s)', mute) MixerListener.send('mute_changed', mute=mute) + def ping(self): + """Called to check if the actor is still alive.""" + return True + class MixerListener(listener.Listener): From 399124bf464d11357ac5fa4c3413ee3b832e27cf Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 May 2015 21:43:50 +0200 Subject: [PATCH 3/4] startup: Handle frontend and backend failures --- mopidy/backend.py | 4 +++ mopidy/commands.py | 64 ++++++++++++++++++++++++++-------------------- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/mopidy/backend.py b/mopidy/backend.py index 72293f94..0cea0f85 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -58,6 +58,10 @@ class Backend(object): def has_playlists(self): return self.playlists is not None + def ping(self): + """Called to check if the actor is still alive.""" + return True + class LibraryProvider(object): diff --git a/mopidy/commands.py b/mopidy/commands.py index 137e4a3e..4890c722 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, print_function, unicode_literals import argparse import collections +import contextlib import logging import os import sys @@ -232,6 +233,23 @@ class Command(object): raise NotImplementedError +@contextlib.contextmanager +def _actor_error_handling(name): + try: + yield + except exceptions.BackendError as exc: + logger.error( + 'Backend (%s) initialization error: %s', name, exc.message) + except exceptions.FrontendError as exc: + logger.error( + 'Frontend (%s) initialization error: %s', name, exc.message) + except exceptions.MixerError as exc: + logger.error( + 'Mixer (%s) initialization error: %s', name, exc.message) + except Exception: + logger.exception('Got un-handled exception from %s', name) + + # TODO: move out of this utility class class RootCommand(Command): @@ -328,22 +346,14 @@ class RootCommand(Command): def start_mixer(self, config, mixer_class): logger.info('Starting Mopidy mixer: %s', mixer_class.__name__) - mixer = None - - try: + with _actor_error_handling(mixer_class.__name__): mixer = mixer_class.start(config=config).proxy() - mixer.ping().get() - except exceptions.MixerError as exc: - logger.error('Mixer (%s) initialization error: %s', - mixer_class.__name__, exc.message) - except pykka.ActorDeadError as exc: - mixer = None - logger.error('Mixer actor died: %s', exc) - except Exception: - logger.exception('Mixer (%s) initialization exception:', - mixer_class.__name__) - - return mixer + try: + mixer.ping().get() + return mixer + except pykka.ActorDeadError as exc: + logger.error('Actor died: %s', exc) + return None def configure_mixer(self, config, mixer): volume = config['audio']['mixer_volume'] @@ -364,16 +374,19 @@ class RootCommand(Command): backends = [] for backend_class in backend_classes: - try: + with _actor_error_handling(backend_class.__name__): with timer.time_logger(backend_class.__name__): backend = backend_class.start( config=config, audio=audio).proxy() - backends.append(backend) - except exceptions.BackendError as exc: - logger.error( - 'Backend (%s) initialization error: %s', - backend_class.__name__, exc.message) - raise + backends.append(backend) + + # Block until all on_starts have finished, letting them run in parallel + for backend in backends[:]: + try: + backend.ping().get() + except pykka.ActorDeadError as exc: + backends.remove(backend) + logger.error('Actor died: %s', exc) return backends @@ -388,14 +401,9 @@ class RootCommand(Command): ', '.join(f.__name__ for f in frontend_classes) or 'none') for frontend_class in frontend_classes: - try: + with _actor_error_handling(frontend_class.__name__): with timer.time_logger(frontend_class.__name__): frontend_class.start(config=config, core=core) - except exceptions.FrontendError as exc: - logger.error( - 'Frontend (%s) initialization error: %s', - frontend_class.__name__, exc.message) - raise def stop_frontends(self, frontend_classes): logger.info('Stopping Mopidy frontends') From 8c7b3c69fb30294bb45f69694e374bb648262d57 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 12 May 2015 22:39:30 +0200 Subject: [PATCH 4/4] core: Assume backend.has_* calls could fail --- mopidy/core/actor.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index b6318492..f20b0ba2 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import collections import itertools +import logging import pykka @@ -18,6 +19,9 @@ from mopidy.internal import versioning from mopidy.internal.deprecation import deprecated_property +logger = logging.getLogger(__name__) + + class Core( pykka.ThreadingActor, audio.AudioListener, backend.BackendListener, mixer.MixerListener): @@ -145,10 +149,15 @@ class Backends(list): return b.actor_ref.actor_class.__name__ for b in backends: - has_library = b.has_library().get() - has_library_browse = b.has_library_browse().get() - has_playback = b.has_playback().get() - has_playlists = b.has_playlists().get() + try: + has_library = b.has_library().get() + has_library_browse = b.has_library_browse().get() + has_playback = b.has_playback().get() + has_playlists = b.has_playlists().get() + except Exception: + self.remove(b) + logger.exception('Fetching backend info for %s failed', + b.actor_ref.actor_class.__name__) for scheme in b.uri_schemes.get(): assert scheme not in backends_by_scheme, (