From 9b18e994191d175e47fd2ae5a046a60f2f9abf33 Mon Sep 17 00:00:00 2001 From: Siddharth Pradhan Date: Mon, 29 Jun 2026 12:08:03 -0400 Subject: [PATCH 1/3] Fix Docker boot hang when Spotify is the configured primary source. Avoid blocking Spotify auth probes during gunicorn worker import and add a request timeout to the auth probe client so unreachable API calls cannot stall startup indefinitely. Co-authored-by: Cursor --- Dockerfile | 1 + core/metadata/registry.py | 13 ++++++++- core/metadata_service.py | 2 ++ core/spotify_client.py | 3 ++- .../test_configured_primary_source.py | 27 +++++++++++++++++++ web_server.py | 6 +++-- 6 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 tests/metadata/test_configured_primary_source.py diff --git a/Dockerfile b/Dockerfile index 566de2c5..5ade62ce 100644 --- a/Dockerfile +++ b/Dockerfile @@ -139,6 +139,7 @@ HEALTHCHECK --interval=30s --timeout=10s --start-period=60s --retries=3 \ ENV PYTHONPATH=/app ENV PYTHONUNBUFFERED=1 ENV DATABASE_PATH=/app/data/music_library.db +ENV SOULSYNC_CONFIG_PATH=/app/config/config.json ENV PUID=1000 ENV PGID=1000 ENV UMASK=022 diff --git a/core/metadata/registry.py b/core/metadata/registry.py index a6c6ea39..8bb8c1aa 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -352,10 +352,21 @@ def get_hydrabase_client(allow_fallback: bool = True, require_enabled: bool = Tr return None +def get_configured_primary_source() -> str: + """Return metadata.fallback_source as stored in config, without runtime downgrade. + + Unlike get_primary_source(), this never probes Spotify auth. Use at boot and + anywhere blocking network I/O must be avoided (gunicorn worker import, Docker + cold start when Spotify is unreachable). + """ + _default = METADATA_SOURCE_PRIORITY[0] + return _get_config_value("metadata.fallback_source", _default) or _default + + def get_primary_source(spotify_client_factory: Optional[MetadataClientFactory] = None) -> str: """Return configured primary metadata source.""" _default = METADATA_SOURCE_PRIORITY[0] - source = _get_config_value("metadata.fallback_source", _default) or _default + source = get_configured_primary_source() if source == "spotify": try: diff --git a/core/metadata_service.py b/core/metadata_service.py index 7c390746..fd412a99 100644 --- a/core/metadata_service.py +++ b/core/metadata_service.py @@ -49,6 +49,7 @@ from core.metadata.registry import ( get_discogs_client, get_hydrabase_client, get_itunes_client, + get_configured_primary_source, get_primary_client, get_primary_source, get_primary_source_label, @@ -117,6 +118,7 @@ __all__ = [ "get_metadata_service", "get_musicmap_similar_artists", "get_primary_client", + "get_configured_primary_source", "get_primary_source", "get_primary_source_label", "get_spotify_client_for_profile", diff --git a/core/spotify_client.py b/core/spotify_client.py index b5318037..d0d74c55 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -857,7 +857,8 @@ class SpotifyClient: # Use a dedicated probe client (retries=0) so a 429 here propagates # immediately and we can detect long Retry-After bans. try: - probe = spotipy.Spotify(auth_manager=self.sp.auth_manager, retries=0) + probe = spotipy.Spotify( + auth_manager=self.sp.auth_manager, retries=0, requests_timeout=15) probe.current_user() result = True except Exception as e: diff --git a/tests/metadata/test_configured_primary_source.py b/tests/metadata/test_configured_primary_source.py new file mode 100644 index 00000000..68ae014b --- /dev/null +++ b/tests/metadata/test_configured_primary_source.py @@ -0,0 +1,27 @@ +"""Tests for boot-safe configured primary source lookup.""" + +from unittest.mock import MagicMock, patch + +from core.metadata import registry + + +def test_get_configured_primary_source_reads_config_without_auth_probe(monkeypatch): + monkeypatch.setattr( + registry, + "_get_config_value", + lambda key, default=None: "spotify" if key == "metadata.fallback_source" else default, + ) + + with patch.object(registry, "get_spotify_client") as get_client: + assert registry.get_configured_primary_source() == "spotify" + get_client.assert_not_called() + + +def test_get_primary_source_still_downgrades_unauthenticated_spotify(monkeypatch): + monkeypatch.setattr(registry, "get_configured_primary_source", lambda: "spotify") + + spotify = MagicMock() + spotify.is_spotify_authenticated.return_value = False + monkeypatch.setattr(registry, "get_spotify_client", lambda **_: spotify) + + assert registry.get_primary_source() == registry.METADATA_SOURCE_PRIORITY[0] diff --git a/web_server.py b/web_server.py index 15b5de83..d06ebb4f 100644 --- a/web_server.py +++ b/web_server.py @@ -36242,11 +36242,13 @@ except Exception as e: # they explicitly want background Spotify enrichment. spotify_enrichment_worker = None try: - from core.metadata_service import get_primary_source as _get_primary_source + from core.metadata_service import get_configured_primary_source from database.music_database import MusicDatabase spotify_enrichment_db = MusicDatabase() spotify_enrichment_worker = SpotifyWorker(database=spotify_enrichment_db) - _primary = _get_primary_source() + # Use configured source only — get_primary_source() probes Spotify auth and can + # block gunicorn worker boot indefinitely when the API is unreachable. + _primary = get_configured_primary_source() _user_paused = config_manager.get('spotify_enrichment_paused', False) if _user_paused or _primary != 'spotify': spotify_enrichment_worker.paused = True # Set BEFORE start() to prevent race condition From 9fc3628062d5e8928f0abf1cbd6eb6efa1a489c7 Mon Sep 17 00:00:00 2001 From: Siddharth Pradhan Date: Mon, 29 Jun 2026 12:17:44 -0400 Subject: [PATCH 2/3] Defer all provider API probes during boot to prevent startup hangs. Introduce a boot-phase guard so gunicorn worker import never blocks on Spotify, Qobuz, Deezer, or Tidal network validation. Network auth checks run only after module initialization completes. --- core/boot_phase.py | 27 +++++++ core/deezer_download_client.py | 13 +++- core/metadata/registry.py | 21 +++++ core/qobuz_client.py | 6 ++ core/spotify_client.py | 30 ++++++++ core/tidal_client.py | 10 ++- core/tidal_download_client.py | 50 ++++++++++++ tests/metadata/test_boot_phase_providers.py | 77 +++++++++++++++++++ .../test_configured_primary_source.py | 5 ++ web_server.py | 5 ++ 10 files changed, 240 insertions(+), 4 deletions(-) create mode 100644 core/boot_phase.py create mode 100644 tests/metadata/test_boot_phase_providers.py diff --git a/core/boot_phase.py b/core/boot_phase.py new file mode 100644 index 00000000..9d6853df --- /dev/null +++ b/core/boot_phase.py @@ -0,0 +1,27 @@ +"""Boot-phase guard for non-blocking container startup. + +While the gunicorn worker is importing ``web_server`` (module-level client and +worker initialization), external provider API probes must not block startup. +Network validation is deferred until ``mark_boot_complete()`` runs at the end +of that import pass. +""" + +from __future__ import annotations + +import threading + +_boot_lock = threading.Lock() +_boot_active = True + + +def is_boot_phase() -> bool: + """Return True while module import must avoid blocking provider API calls.""" + with _boot_lock: + return _boot_active + + +def mark_boot_complete() -> None: + """End the boot phase — provider clients may perform network probes again.""" + global _boot_active + with _boot_lock: + _boot_active = False diff --git a/core/deezer_download_client.py b/core/deezer_download_client.py index e1830f64..cdc80499 100644 --- a/core/deezer_download_client.py +++ b/core/deezer_download_client.py @@ -121,6 +121,7 @@ class DeezerDownloadClient(DownloadSourcePlugin): self._license_token = None self._user_data = None self._authenticated = False + self._pending_arl: Optional[str] = None # Quality preference self._quality = quality_tier_for_source('deezer', default='flac') @@ -128,7 +129,12 @@ class DeezerDownloadClient(DownloadSourcePlugin): # Try to authenticate on init if ARL is configured arl = config_manager.get('deezer_download.arl', '') if arl: - self._authenticate(arl) + from core.boot_phase import is_boot_phase + if is_boot_phase(): + self._pending_arl = arl + logger.debug("Deezer ARL present — authentication deferred until after boot") + else: + self._authenticate(arl) logger.info(f"Deezer download client initialized (download path: {self.download_path})") @@ -227,6 +233,11 @@ class DeezerDownloadClient(DownloadSourcePlugin): return self._authenticated def is_authenticated(self) -> bool: + if self._pending_arl and not self._authenticated: + from core.boot_phase import is_boot_phase + if not is_boot_phase(): + self._authenticate(self._pending_arl) + self._pending_arl = None return self._authenticated async def check_connection(self) -> bool: diff --git a/core/metadata/registry.py b/core/metadata/registry.py index 8bb8c1aa..7002afa2 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -365,6 +365,11 @@ def get_configured_primary_source() -> str: def get_primary_source(spotify_client_factory: Optional[MetadataClientFactory] = None) -> str: """Return configured primary metadata source.""" + from core.boot_phase import is_boot_phase + + if is_boot_phase(): + return get_configured_primary_source() + _default = METADATA_SOURCE_PRIORITY[0] source = get_configured_primary_source() @@ -453,7 +458,19 @@ def get_primary_source_status( musicbrainz_client_factory: Optional[MetadataClientFactory] = None, ) -> Dict[str, Any]: """Return a generic status snapshot for the active primary metadata source.""" + from core.boot_phase import is_boot_phase + source = _get_config_value("metadata.fallback_source", "deezer") or "deezer" + if is_boot_phase(): + display_source = source + if source == "spotify" and _get_config_value("metadata.spotify_free", False): + display_source = "spotify_free" + return { + "source": display_source, + "connected": False, + "response_time": 0, + } + started = time.time() connected = False @@ -521,9 +538,13 @@ def get_client_for_source( musicbrainz_client_factory: Optional[MetadataClientFactory] = None, ): """Return exact client for a source, or None if unavailable.""" + from core.boot_phase import is_boot_phase + if source == "spotify": try: client = get_spotify_client(client_factory=spotify_client_factory) + if is_boot_phase(): + return client if client and getattr(client, "sp", None) else None if client and client.is_spotify_authenticated(): return client except Exception as e: diff --git a/core/qobuz_client.py b/core/qobuz_client.py index e90e934a..1e6e7099 100644 --- a/core/qobuz_client.py +++ b/core/qobuz_client.py @@ -164,6 +164,8 @@ class QobuzClient(DownloadSourcePlugin): def _restore_session(self): """Try to restore saved session from config.""" + from core.boot_phase import is_boot_phase + saved = config_manager.get('qobuz.session', {}) app_id = saved.get('app_id', '') app_secret = saved.get('app_secret', '') @@ -178,6 +180,10 @@ class QobuzClient(DownloadSourcePlugin): 'X-User-Auth-Token': self.user_auth_token, }) + if is_boot_phase(): + logger.info("Loaded Qobuz session from config (verification deferred until after boot)") + return + # Verify the token is still valid try: resp = self.session.get( diff --git a/core/spotify_client.py b/core/spotify_client.py index d0d74c55..6c9868d0 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -751,6 +751,16 @@ class SpotifyClient: self._auth_cached_result = None self._auth_cache_time = 0 + def _has_cached_oauth_token(self) -> bool: + """Return True when a persisted OAuth token exists (no network I/O).""" + if self.sp is None: + return False + try: + cache_handler = getattr(self.sp.auth_manager, 'cache_handler', None) + return bool(cache_handler and cache_handler.get_cached_token() is not None) + except Exception: + return False + def is_spotify_authenticated(self) -> bool: """Check if Spotify client is specifically authenticated (not just iTunes fallback). Results are cached for 60 seconds to avoid excessive API calls. @@ -826,6 +836,26 @@ class SpotifyClient: logger.debug("publish_spotify_status cache hit: %s", e) return self._auth_cached_result + from core.boot_phase import is_boot_phase + if is_boot_phase(): + result = self._has_cached_oauth_token() + with self._auth_cache_lock: + self._auth_cached_result = result + self._auth_cache_time = time.time() + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=result, + authenticated=result, + rate_limited=False, + rate_limit=None, + post_ban_cooldown=None, + ) + except Exception as e: + logger.debug("publish_spotify_status boot-phase: %s", e) + return result + # Cache miss — make API call outside the lock. # Safety: if there's no cached token, return False immediately. # Without this guard, spotipy's auth_manager will try to start an interactive diff --git a/core/tidal_client.py b/core/tidal_client.py index 4b542d3d..49270403 100644 --- a/core/tidal_client.py +++ b/core/tidal_client.py @@ -520,10 +520,14 @@ class TidalClient: return True - def is_authenticated(self): + def is_authenticated(self) -> bool: """Check if client is authenticated, refreshing expired tokens if possible""" - if self.access_token and time.time() < self.token_expires_at: - return True + from core.boot_phase import is_boot_phase + + if is_boot_phase(): + if self.access_token and time.time() < self.token_expires_at: + return True + return bool(self.access_token and self.refresh_token) # Backoff: if refresh recently failed, don't retry for 5 minutes if hasattr(self, '_refresh_failed_at') and self._refresh_failed_at: diff --git a/core/tidal_download_client.py b/core/tidal_download_client.py index 89757d42..5d577419 100644 --- a/core/tidal_download_client.py +++ b/core/tidal_download_client.py @@ -134,6 +134,7 @@ class TidalDownloadClient(DownloadSourcePlugin): self._device_auth_future = None self._device_auth_link = None + self._boot_session_tokens: Optional[dict] = None # Engine reference is populated by set_engine() at registration # time. Until then dispatch returns None — orchestrator wires @@ -163,6 +164,14 @@ class TidalDownloadClient(DownloadSourcePlugin): expiry_time = saved.get('expiry_time', 0) if token_type and access_token: + from core.boot_phase import is_boot_phase + if is_boot_phase(): + self._boot_session_tokens = saved + logger.info( + "Loaded Tidal download session from config (verification deferred until after boot)" + ) + return + try: expiry_dt = datetime.fromtimestamp(expiry_time, tz=timezone.utc) if expiry_time else None @@ -181,6 +190,39 @@ class TidalDownloadClient(DownloadSourcePlugin): except Exception as e: logger.warning(f"Could not restore Tidal session: {e}") + def _complete_deferred_session(self) -> bool: + """Finish restoring a session that was deferred during boot.""" + pending = getattr(self, '_boot_session_tokens', None) + if not pending or tidalapi is None: + self._boot_session_tokens = None + return False + + if not self.session: + self.session = tidalapi.Session() + + token_type = pending.get('token_type', '') + access_token = pending.get('access_token', '') + refresh_token = pending.get('refresh_token', '') + expiry_time = pending.get('expiry_time', 0) + self._boot_session_tokens = None + + try: + expiry_dt = datetime.fromtimestamp(expiry_time, tz=timezone.utc) if expiry_time else None + restored = self.session.load_oauth_session( + token_type=token_type, + access_token=access_token, + refresh_token=refresh_token, + expiry_time=expiry_dt, + ) + if restored and self.session.check_login(): + logger.info("Restored Tidal download session from saved tokens") + self._save_session() + return True + logger.warning("Saved Tidal session tokens are invalid/expired") + except Exception as e: + logger.warning(f"Could not restore Tidal session: {e}") + return False + def _save_session(self): if not self.session: return @@ -192,6 +234,14 @@ class TidalDownloadClient(DownloadSourcePlugin): }) def is_authenticated(self) -> bool: + from core.boot_phase import is_boot_phase + if is_boot_phase(): + pending = getattr(self, '_boot_session_tokens', None) + return bool(pending and pending.get('access_token')) + + if getattr(self, '_boot_session_tokens', None): + return self._complete_deferred_session() + if not self.session: return False try: diff --git a/tests/metadata/test_boot_phase_providers.py b/tests/metadata/test_boot_phase_providers.py new file mode 100644 index 00000000..f67b468d --- /dev/null +++ b/tests/metadata/test_boot_phase_providers.py @@ -0,0 +1,77 @@ +"""Boot-phase guards must defer blocking provider network probes.""" + +from unittest.mock import MagicMock, patch + +from core.boot_phase import is_boot_phase, mark_boot_complete +from core.metadata import registry + + +def setup_function(): + mark_boot_complete() + + +def teardown_function(): + mark_boot_complete() + + +def test_get_primary_source_skips_spotify_probe_during_boot(monkeypatch): + import core.boot_phase as boot_phase + + boot_phase._boot_active = True + monkeypatch.setattr(registry, "get_configured_primary_source", lambda: "spotify") + + with patch.object(registry, "get_spotify_client") as get_client: + assert registry.get_primary_source() == "spotify" + get_client.assert_not_called() + + +def test_get_primary_source_status_skips_client_probe_during_boot(monkeypatch): + import core.boot_phase as boot_phase + + boot_phase._boot_active = True + monkeypatch.setattr( + registry, "_get_config_value", + lambda key, default=None: "spotify" if key == "metadata.fallback_source" else default, + ) + + with patch.object(registry, "get_client_for_source") as get_client: + status = registry.get_primary_source_status() + get_client.assert_not_called() + assert status["source"] == "spotify" + assert status["connected"] is False + + +def test_spotify_auth_uses_token_presence_only_during_boot(monkeypatch): + import core.boot_phase as boot_phase + from core.spotify_client import SpotifyClient + + boot_phase._boot_active = True + client = SpotifyClient.__new__(SpotifyClient) + client.sp = MagicMock() + client._auth_cache_lock = __import__('threading').Lock() + client._auth_cached_result = None + client._auth_cache_time = 0 + client._AUTH_CACHE_TTL = 900 + + monkeypatch.setattr(client, "_has_cached_oauth_token", lambda: True) + + with patch("spotipy.Spotify") as spotify_cls: + assert client.is_spotify_authenticated() is True + spotify_cls.assert_not_called() + + +def test_deezer_download_defers_arl_auth_during_boot(monkeypatch): + import core.boot_phase as boot_phase + from core.deezer_download_client import DeezerDownloadClient + + boot_phase._boot_active = True + monkeypatch.setattr( + "config.settings.config_manager.get", + lambda key, default=None: "fake-arl" if key == "deezer_download.arl" else default, + ) + + with patch.object(DeezerDownloadClient, "_authenticate") as authenticate: + client = DeezerDownloadClient(download_path="/tmp/deezer-test") + authenticate.assert_not_called() + assert client._pending_arl == "fake-arl" + assert client.is_authenticated() is False diff --git a/tests/metadata/test_configured_primary_source.py b/tests/metadata/test_configured_primary_source.py index 68ae014b..2154673c 100644 --- a/tests/metadata/test_configured_primary_source.py +++ b/tests/metadata/test_configured_primary_source.py @@ -2,9 +2,14 @@ from unittest.mock import MagicMock, patch +from core.boot_phase import mark_boot_complete from core.metadata import registry +def setup_function(): + mark_boot_complete() + + def test_get_configured_primary_source_reads_config_without_auth_probe(monkeypatch): monkeypatch.setattr( registry, diff --git a/web_server.py b/web_server.py index d06ebb4f..ff53faa4 100644 --- a/web_server.py +++ b/web_server.py @@ -38728,6 +38728,11 @@ def start_runtime_services(): _runtime_started = True +# Module import is complete — provider clients may now perform network probes. +from core.boot_phase import mark_boot_complete +mark_boot_complete() + + # Direct execution: python web_server.py (dev/Windows fallback) # Production should use: gunicorn -c gunicorn.conf.py wsgi:application if _DIRECT_RUN: From a8520bc9b24b26c486c733935d80e0574288d1a2 Mon Sep 17 00:00:00 2001 From: Siddharth Pradhan Date: Mon, 29 Jun 2026 12:25:50 -0400 Subject: [PATCH 3/3] revert dockerfile changes --- Dockerfile | 1 - 1 file changed, 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 5ade62ce..566de2c5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -139,7 +139,6 @@ HEALTHCHECK --interval=30s --timeout=10s --start-period=60s --retries=3 \ ENV PYTHONPATH=/app ENV PYTHONUNBUFFERED=1 ENV DATABASE_PATH=/app/data/music_library.db -ENV SOULSYNC_CONFIG_PATH=/app/config/config.json ENV PUID=1000 ENV PGID=1000 ENV UMASK=022