mirror of
https://github.com/HabiRabbu/Musicseerr
synced 2026-04-21 13:37:27 +00:00
fix: resolve Lidarr library scan failures after settings change
This commit is contained in:
parent
491947ecab
commit
eed1ce700d
15 changed files with 467 additions and 68 deletions
9
Makefile
9
Makefile
|
|
@ -91,6 +91,15 @@ backend-test-search-top-result: $(BACKEND_VENV_STAMP) ## Run search top result d
|
|||
backend-test-cache-cleanup: $(BACKEND_VENV_STAMP) ## Run cache cleanup tests
|
||||
cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/test_cache_cleanup.py -v
|
||||
|
||||
backend-test-lidarr-url: $(BACKEND_VENV_STAMP) ## Run dynamic Lidarr URL resolution tests
|
||||
cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/test_lidarr_url_dynamic.py -v
|
||||
|
||||
backend-test-sync-coordinator: $(BACKEND_VENV_STAMP) ## Run sync coordinator tests (cooldown, dedup)
|
||||
cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/test_sync_coordinator.py -v
|
||||
|
||||
backend-test-local-files-fallback: $(BACKEND_VENV_STAMP) ## Run local files stale-while-error fallback tests
|
||||
cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/test_local_files_fallback.py -v
|
||||
|
||||
test-audiodb-all: backend-test-audiodb backend-test-audiodb-prewarm backend-test-audiodb-settings backend-test-coverart-audiodb backend-test-audiodb-phase8 backend-test-audiodb-phase9 frontend-test-audiodb-images ## Run every AudioDB test target
|
||||
|
||||
frontend-install: ## Install frontend npm dependencies
|
||||
|
|
|
|||
|
|
@ -95,10 +95,9 @@ async def sync_library(
|
|||
try:
|
||||
return await library_service.sync_library(is_manual=True)
|
||||
except ExternalServiceError as e:
|
||||
logger.error(f"Couldn't sync the library: {e}")
|
||||
if "cooldown" in str(e).lower():
|
||||
raise HTTPException(status_code=429, detail="Sync is on cooldown, please wait")
|
||||
raise HTTPException(status_code=503, detail="External service unavailable")
|
||||
raise
|
||||
|
||||
|
||||
@router.get("/stats", response_model=LibraryStatsResponse)
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ from models.error import (
|
|||
CONFIGURATION_ERROR,
|
||||
SOURCE_RESOLUTION_ERROR,
|
||||
INTERNAL_ERROR,
|
||||
CIRCUIT_BREAKER_OPEN,
|
||||
STATUS_TO_CODE,
|
||||
)
|
||||
|
||||
|
|
@ -41,7 +42,11 @@ async def external_service_error_handler(request: Request, exc: ExternalServiceE
|
|||
|
||||
async def circuit_open_error_handler(request: Request, exc: CircuitOpenError) -> MsgSpecJSONResponse:
|
||||
logger.error("Circuit breaker open: %s - %s %s", exc, request.method, request.url.path)
|
||||
return error_response(status.HTTP_503_SERVICE_UNAVAILABLE, SERVICE_UNAVAILABLE, "Service temporarily unavailable")
|
||||
return error_response(
|
||||
status.HTTP_503_SERVICE_UNAVAILABLE,
|
||||
CIRCUIT_BREAKER_OPEN,
|
||||
"Service temporarily unavailable due to repeated connection failures. Check your settings or wait for the service to recover.",
|
||||
)
|
||||
|
||||
|
||||
async def validation_error_handler(request: Request, exc: ValidationError) -> MsgSpecJSONResponse:
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ CONFIGURATION_ERROR = "CONFIGURATION_ERROR"
|
|||
SOURCE_RESOLUTION_ERROR = "SOURCE_RESOLUTION_ERROR"
|
||||
INTERNAL_ERROR = "INTERNAL_ERROR"
|
||||
RATE_LIMITED = "RATE_LIMITED"
|
||||
CIRCUIT_BREAKER_OPEN = "CIRCUIT_BREAKER_OPEN"
|
||||
CLIENT_DISCONNECTED = "CLIENT_DISCONNECTED"
|
||||
|
||||
FORBIDDEN = "FORBIDDEN"
|
||||
|
|
|
|||
|
|
@ -46,7 +46,10 @@ class LidarrBase:
|
|||
self._settings = settings
|
||||
self._client = http_client
|
||||
self._cache = cache
|
||||
self._base_url = settings.lidarr_url
|
||||
|
||||
@property
|
||||
def _base_url(self) -> str:
|
||||
return self._settings.lidarr_url
|
||||
|
||||
def is_configured(self) -> bool:
|
||||
return bool(self._settings.lidarr_api_key)
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ from infrastructure.cache.disk_cache import DiskMetadataCache
|
|||
from infrastructure.cover_urls import prefer_release_group_cover_url
|
||||
from infrastructure.serialization import clone_with_updates
|
||||
from core.exceptions import ExternalServiceError
|
||||
from infrastructure.resilience.retry import CircuitOpenError
|
||||
from services.cache_status_service import CacheStatusService
|
||||
from services.library_precache_service import LibraryPrecacheService
|
||||
|
||||
|
|
@ -84,6 +85,7 @@ class LibraryService:
|
|||
self._manual_sync_cooldown: float = 60.0
|
||||
self._global_sync_cooldown: float = 30.0
|
||||
self._sync_lock = asyncio.Lock()
|
||||
self._sync_future: asyncio.Future | None = None
|
||||
|
||||
def _update_last_sync_timestamp(self) -> None:
|
||||
try:
|
||||
|
|
@ -208,6 +210,8 @@ class LibraryService:
|
|||
for album in albums_data
|
||||
]
|
||||
return albums, total
|
||||
except (ExternalServiceError, CircuitOpenError):
|
||||
raise
|
||||
except Exception as e: # noqa: BLE001
|
||||
logger.error(f"Failed to fetch paginated albums: {e}")
|
||||
raise ExternalServiceError(f"Failed to fetch paginated albums: {e}")
|
||||
|
|
@ -242,6 +246,8 @@ class LibraryService:
|
|||
for artist in artists_data
|
||||
]
|
||||
return artists, total
|
||||
except (ExternalServiceError, CircuitOpenError):
|
||||
raise
|
||||
except Exception as e: # noqa: BLE001
|
||||
logger.error(f"Failed to fetch paginated artists: {e}")
|
||||
raise ExternalServiceError(f"Failed to fetch paginated artists: {e}")
|
||||
|
|
@ -317,64 +323,100 @@ class LibraryService:
|
|||
else:
|
||||
logger.info("Library sync already in progress - skipping auto-sync")
|
||||
return SyncLibraryResponse(status="skipped", artists=0, albums=0)
|
||||
|
||||
if self._sync_future is not None and not self._sync_future.done():
|
||||
existing_future = self._sync_future
|
||||
else:
|
||||
existing_future = None
|
||||
loop = asyncio.get_running_loop()
|
||||
self._sync_future = loop.create_future()
|
||||
|
||||
# Shield so waiter cancellation doesn't poison the shared future
|
||||
if existing_future is not None:
|
||||
return await asyncio.shield(existing_future)
|
||||
|
||||
sync_succeeded = False
|
||||
try:
|
||||
logger.info("Starting library sync from Lidarr")
|
||||
|
||||
albums = await self._lidarr_repo.get_library()
|
||||
artists = await self._lidarr_repo.get_artists_from_library()
|
||||
|
||||
self._last_sync_time = current_time
|
||||
albums_data = [
|
||||
{
|
||||
'mbid': album.musicbrainz_id or f"unknown_{album.album}",
|
||||
'artist_mbid': album.artist_mbid,
|
||||
'artist_name': album.artist,
|
||||
'title': album.album,
|
||||
'year': album.year,
|
||||
'cover_url': self._normalized_album_cover_url(
|
||||
album.musicbrainz_id,
|
||||
album.cover_url,
|
||||
),
|
||||
'monitored': album.monitored,
|
||||
'date_added': album.date_added
|
||||
}
|
||||
for album in albums
|
||||
]
|
||||
|
||||
await self._library_db.save_library(artists, albums_data)
|
||||
logger.info("Library cache updated - unmonitored items removed")
|
||||
|
||||
now = time.time()
|
||||
self._last_sync_time = now
|
||||
if is_manual:
|
||||
self._last_manual_sync = current_time
|
||||
|
||||
logger.info("Starting library sync from Lidarr")
|
||||
self._last_manual_sync = now
|
||||
|
||||
albums = await self._lidarr_repo.get_library()
|
||||
artists = await self._lidarr_repo.get_artists_from_library()
|
||||
|
||||
albums_data = [
|
||||
{
|
||||
'mbid': album.musicbrainz_id or f"unknown_{album.album}",
|
||||
'artist_mbid': album.artist_mbid,
|
||||
'artist_name': album.artist,
|
||||
'title': album.album,
|
||||
'year': album.year,
|
||||
'cover_url': self._normalized_album_cover_url(
|
||||
album.musicbrainz_id,
|
||||
album.cover_url,
|
||||
),
|
||||
'monitored': album.monitored,
|
||||
'date_added': album.date_added
|
||||
}
|
||||
for album in albums
|
||||
]
|
||||
|
||||
await self._library_db.save_library(artists, albums_data)
|
||||
logger.info("Library cache updated - unmonitored items removed")
|
||||
if self._precache_service is None:
|
||||
logger.warning("Precache skipped — sync_state_store/genre_index not provided")
|
||||
self._update_last_sync_timestamp()
|
||||
result = SyncLibraryResponse(status='success', artists=len(artists), albums=len(albums))
|
||||
self._sync_future.set_result(result)
|
||||
return result
|
||||
|
||||
if self._precache_service is None:
|
||||
logger.warning("Precache skipped — sync_state_store/genre_index not provided")
|
||||
return
|
||||
task = asyncio.create_task(self._precache_service.precache_library_resources(artists, albums))
|
||||
|
||||
task = asyncio.create_task(self._precache_service.precache_library_resources(artists, albums))
|
||||
def on_task_done(t: asyncio.Task):
|
||||
try:
|
||||
exc = t.exception()
|
||||
if exc:
|
||||
logger.error(f"Precache task failed: {exc}")
|
||||
except asyncio.CancelledError:
|
||||
logger.info("Precache task was cancelled")
|
||||
finally:
|
||||
status_service.set_current_task(None)
|
||||
|
||||
def on_task_done(t: asyncio.Task):
|
||||
try:
|
||||
exc = t.exception()
|
||||
if exc:
|
||||
logger.error(f"Precache task failed: {exc}")
|
||||
except asyncio.CancelledError:
|
||||
logger.info("Precache task was cancelled")
|
||||
finally:
|
||||
status_service.set_current_task(None)
|
||||
task.add_done_callback(on_task_done)
|
||||
status_service.set_current_task(task)
|
||||
|
||||
task.add_done_callback(on_task_done)
|
||||
status_service.set_current_task(task)
|
||||
logger.info(f"Library sync complete: {len(artists)} artists, {len(albums)} albums")
|
||||
|
||||
logger.info(f"Library sync complete: {len(artists)} artists, {len(albums)} albums")
|
||||
self._update_last_sync_timestamp()
|
||||
|
||||
self._update_last_sync_timestamp()
|
||||
|
||||
return SyncLibraryResponse(
|
||||
status='success',
|
||||
artists=len(artists),
|
||||
albums=len(albums),
|
||||
)
|
||||
result = SyncLibraryResponse(
|
||||
status='success',
|
||||
artists=len(artists),
|
||||
albums=len(albums),
|
||||
)
|
||||
sync_succeeded = True
|
||||
self._sync_future.set_result(result)
|
||||
return result
|
||||
except BaseException as exc:
|
||||
if self._sync_future is not None and not self._sync_future.done():
|
||||
self._sync_future.set_exception(exc)
|
||||
raise
|
||||
finally:
|
||||
if not sync_succeeded:
|
||||
future = self._sync_future
|
||||
self._sync_future = None
|
||||
# Suppress "Future exception was never retrieved" if no waiter
|
||||
if future is not None and future.done() and not future.cancelled():
|
||||
try:
|
||||
future.exception()
|
||||
except BaseException:
|
||||
pass
|
||||
except (ExternalServiceError, CircuitOpenError):
|
||||
raise
|
||||
except Exception as e: # noqa: BLE001
|
||||
logger.error(f"Couldn't sync the library: {e}")
|
||||
raise ExternalServiceError(f"Couldn't sync the library: {e}")
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ from infrastructure.cache.cache_keys import LOCAL_FILES_PREFIX
|
|||
from infrastructure.cache.memory_cache import CacheInterface
|
||||
from infrastructure.cover_urls import prefer_release_group_cover_url
|
||||
from infrastructure.constants import STREAM_CHUNK_SIZE
|
||||
from infrastructure.resilience.retry import CircuitOpenError
|
||||
from infrastructure.serialization import to_jsonable
|
||||
from repositories.protocols import LidarrRepositoryProtocol
|
||||
from services.preferences_service import PreferencesService
|
||||
|
|
@ -96,12 +97,24 @@ class LocalFilesService:
|
|||
cached = await self._cache.get(cache_key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
data = await self._lidarr.get_all_albums()
|
||||
if data:
|
||||
await self._cache.set(
|
||||
cache_key, data, ttl_seconds=self._ALBUM_LIST_TTL
|
||||
)
|
||||
return data or []
|
||||
try:
|
||||
data = await self._lidarr.get_all_albums()
|
||||
except (ExternalServiceError, CircuitOpenError, ConnectionError, OSError):
|
||||
# Stale-while-error: serve last-known data if Lidarr is down
|
||||
try:
|
||||
stale = await self._cache.get(f"{cache_key}:stale")
|
||||
except Exception: # noqa: BLE001
|
||||
stale = None
|
||||
if stale is not None:
|
||||
logger.warning("Lidarr unavailable — serving stale local album data")
|
||||
return stale
|
||||
raise
|
||||
result = data or []
|
||||
if result:
|
||||
await self._cache.set(cache_key, result, ttl_seconds=self._ALBUM_LIST_TTL)
|
||||
# Keep a long-lived stale copy for fallback (24h)
|
||||
await self._cache.set(f"{cache_key}:stale", result, ttl_seconds=86400)
|
||||
return result
|
||||
|
||||
def _resolve_and_validate_path(self, lidarr_path: str) -> Path:
|
||||
music_path, _ = self._get_config()
|
||||
|
|
|
|||
|
|
@ -166,8 +166,8 @@ async def test_get_albums_caches_lidarr_response(service):
|
|||
assert result.total == 1
|
||||
|
||||
assert cache.set.called
|
||||
call_args = cache.set.call_args
|
||||
assert call_args[0][0] == "local_files_all_albums"
|
||||
cache_keys = [call.args[0] for call in cache.set.call_args_list]
|
||||
assert "local_files_all_albums" in cache_keys
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -70,5 +70,5 @@ async def test_circuit_open_error_hides_details():
|
|||
|
||||
body = resp.json()
|
||||
assert resp.status_code == 503
|
||||
assert body["error"]["message"] == "Service temporarily unavailable"
|
||||
assert body["error"]["message"] == "Service temporarily unavailable due to repeated connection failures. Check your settings or wait for the service to recover."
|
||||
assert "JellyfinRepository" not in resp.text
|
||||
|
|
|
|||
58
backend/tests/test_lidarr_url_dynamic.py
Normal file
58
backend/tests/test_lidarr_url_dynamic.py
Normal file
|
|
@ -0,0 +1,58 @@
|
|||
"""Tests that LidarrBase reads URL and API key dynamically from Settings."""
|
||||
import pytest
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mutable_settings():
|
||||
settings = MagicMock()
|
||||
settings.lidarr_url = "http://old-host:8686"
|
||||
settings.lidarr_api_key = "old-key"
|
||||
return settings
|
||||
|
||||
|
||||
class TestLidarrDynamicUrl:
|
||||
def test_base_url_reads_from_settings_dynamically(self, mutable_settings):
|
||||
from repositories.lidarr.base import LidarrBase
|
||||
|
||||
base = LidarrBase(mutable_settings, MagicMock(), MagicMock())
|
||||
assert base._base_url == "http://old-host:8686"
|
||||
|
||||
mutable_settings.lidarr_url = "http://192.168.50.99:8686"
|
||||
assert base._base_url == "http://192.168.50.99:8686"
|
||||
|
||||
def test_api_key_reads_from_settings_dynamically(self, mutable_settings):
|
||||
from repositories.lidarr.base import LidarrBase
|
||||
|
||||
base = LidarrBase(mutable_settings, MagicMock(), MagicMock())
|
||||
headers = base._get_headers()
|
||||
assert headers["X-Api-Key"] == "old-key"
|
||||
|
||||
mutable_settings.lidarr_api_key = "new-key"
|
||||
headers = base._get_headers()
|
||||
assert headers["X-Api-Key"] == "new-key"
|
||||
|
||||
def test_media_cover_url_uses_dynamic_base_url(self, mutable_settings):
|
||||
from repositories.lidarr.base import LidarrBase
|
||||
|
||||
base = LidarrBase(mutable_settings, MagicMock(), MagicMock())
|
||||
|
||||
url1 = base._build_api_media_cover_url(1, "poster.jpg", 500)
|
||||
assert "http://old-host:8686" in url1
|
||||
|
||||
mutable_settings.lidarr_url = "http://new-host:8686"
|
||||
url2 = base._build_api_media_cover_url(1, "poster.jpg", 500)
|
||||
assert "http://new-host:8686" in url2
|
||||
assert "http://old-host:8686" not in url2
|
||||
|
||||
def test_album_cover_url_uses_dynamic_base_url(self, mutable_settings):
|
||||
from repositories.lidarr.base import LidarrBase
|
||||
|
||||
base = LidarrBase(mutable_settings, MagicMock(), MagicMock())
|
||||
|
||||
url1 = base._build_api_media_cover_url_album(1, "cover.jpg", 500)
|
||||
assert "http://old-host:8686" in url1
|
||||
|
||||
mutable_settings.lidarr_url = "http://new-host:8686"
|
||||
url2 = base._build_api_media_cover_url_album(1, "cover.jpg", 500)
|
||||
assert "http://new-host:8686" in url2
|
||||
94
backend/tests/test_local_files_fallback.py
Normal file
94
backend/tests/test_local_files_fallback.py
Normal file
|
|
@ -0,0 +1,94 @@
|
|||
"""Tests for LocalFilesService stale-while-error fallback."""
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
from core.exceptions import ExternalServiceError
|
||||
|
||||
|
||||
def _make_local_files_service(lidarr=None, cache=None):
|
||||
from services.local_files_service import LocalFilesService
|
||||
|
||||
lidarr = lidarr or AsyncMock()
|
||||
prefs = MagicMock()
|
||||
prefs.get_advanced_settings.return_value = MagicMock(
|
||||
cache_ttl_local_files_recently_added=120,
|
||||
cache_ttl_local_files_storage_stats=300,
|
||||
)
|
||||
prefs.get_local_files_connection.return_value = MagicMock(
|
||||
music_path="/music", lidarr_root_path="/music"
|
||||
)
|
||||
cache = cache or AsyncMock()
|
||||
return LocalFilesService(
|
||||
lidarr_repo=lidarr,
|
||||
preferences_service=prefs,
|
||||
cache=cache,
|
||||
)
|
||||
|
||||
|
||||
class TestStaleWhileError:
|
||||
@pytest.mark.asyncio
|
||||
async def test_serves_stale_data_when_lidarr_down(self):
|
||||
stale_albums = [{"id": 1, "title": "Old Album"}]
|
||||
|
||||
cache = AsyncMock()
|
||||
# Primary cache miss, then stale cache hit
|
||||
cache.get = AsyncMock(side_effect=lambda key: (
|
||||
None if key == "local_files_all_albums" else stale_albums
|
||||
))
|
||||
|
||||
lidarr = AsyncMock()
|
||||
lidarr.get_all_albums = AsyncMock(side_effect=ExternalServiceError("Lidarr down"))
|
||||
|
||||
svc = _make_local_files_service(lidarr=lidarr, cache=cache)
|
||||
result = await svc._fetch_all_albums()
|
||||
|
||||
assert result == stale_albums
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_raises_when_no_stale_data(self):
|
||||
cache = AsyncMock()
|
||||
cache.get = AsyncMock(return_value=None) # Both caches miss
|
||||
|
||||
lidarr = AsyncMock()
|
||||
lidarr.get_all_albums = AsyncMock(side_effect=ExternalServiceError("Lidarr down"))
|
||||
|
||||
svc = _make_local_files_service(lidarr=lidarr, cache=cache)
|
||||
with pytest.raises(ExternalServiceError, match="Lidarr down"):
|
||||
await svc._fetch_all_albums()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_successful_fetch_updates_stale_cache(self):
|
||||
fresh_albums = [{"id": 2, "title": "Fresh Album"}]
|
||||
|
||||
cache = AsyncMock()
|
||||
cache.get = AsyncMock(return_value=None)
|
||||
cache.set = AsyncMock()
|
||||
|
||||
lidarr = AsyncMock()
|
||||
lidarr.get_all_albums = AsyncMock(return_value=fresh_albums)
|
||||
|
||||
svc = _make_local_files_service(lidarr=lidarr, cache=cache)
|
||||
result = await svc._fetch_all_albums()
|
||||
|
||||
assert result == fresh_albums
|
||||
# Should have set both primary and stale caches
|
||||
assert cache.set.call_count == 2
|
||||
calls = {call.args[0] for call in cache.set.call_args_list}
|
||||
assert "local_files_all_albums" in calls
|
||||
assert "local_files_all_albums:stale" in calls
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cache_hit_returns_without_lidarr_call(self):
|
||||
cached = [{"id": 3, "title": "Cached"}]
|
||||
|
||||
cache = AsyncMock()
|
||||
cache.get = AsyncMock(return_value=cached)
|
||||
|
||||
lidarr = AsyncMock()
|
||||
lidarr.get_all_albums = AsyncMock()
|
||||
|
||||
svc = _make_local_files_service(lidarr=lidarr, cache=cache)
|
||||
result = await svc._fetch_all_albums()
|
||||
|
||||
assert result == cached
|
||||
lidarr.get_all_albums.assert_not_called()
|
||||
141
backend/tests/test_sync_coordinator.py
Normal file
141
backend/tests/test_sync_coordinator.py
Normal file
|
|
@ -0,0 +1,141 @@
|
|||
"""Tests for sync coordinator: cooldown on success only, future dedup, race safety."""
|
||||
import asyncio
|
||||
import time
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
|
||||
def _make_library_service(**overrides):
|
||||
"""Build a minimal LibraryService with mocked deps."""
|
||||
from services.library_service import LibraryService
|
||||
|
||||
lidarr = overrides.get("lidarr", AsyncMock())
|
||||
lidarr.is_configured = MagicMock(return_value=True)
|
||||
lidarr.get_library = overrides.get("get_library", AsyncMock(return_value=[]))
|
||||
lidarr.get_artists_from_library = overrides.get(
|
||||
"get_artists_from_library", AsyncMock(return_value=[])
|
||||
)
|
||||
|
||||
library_db = overrides.get("library_db", AsyncMock())
|
||||
library_db.save_library = AsyncMock()
|
||||
|
||||
prefs = overrides.get("prefs", MagicMock())
|
||||
prefs.get_advanced_settings.return_value = MagicMock(
|
||||
cache_ttl_library_sync=30,
|
||||
)
|
||||
|
||||
svc = LibraryService(
|
||||
lidarr_repo=lidarr,
|
||||
library_db=library_db,
|
||||
cover_repo=MagicMock(),
|
||||
preferences_service=prefs,
|
||||
)
|
||||
return svc
|
||||
|
||||
|
||||
class TestCooldownOnlyOnSuccess:
|
||||
@pytest.mark.asyncio
|
||||
async def test_failed_sync_does_not_set_cooldown(self):
|
||||
svc = _make_library_service()
|
||||
svc._lidarr_repo.get_library = AsyncMock(side_effect=RuntimeError("DNS fail"))
|
||||
|
||||
with patch("services.library_service.CacheStatusService") as mock_css:
|
||||
mock_css.return_value.is_syncing.return_value = False
|
||||
|
||||
with pytest.raises(Exception, match="DNS fail"):
|
||||
await svc.sync_library()
|
||||
|
||||
assert svc._last_sync_time == 0.0, "cooldown must NOT activate on failure"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_successful_sync_sets_cooldown(self):
|
||||
svc = _make_library_service()
|
||||
|
||||
with patch("services.library_service.CacheStatusService") as mock_css:
|
||||
mock_css.return_value.is_syncing.return_value = False
|
||||
|
||||
before = time.time()
|
||||
result = await svc.sync_library()
|
||||
after = time.time()
|
||||
|
||||
assert result.status == "success"
|
||||
assert before <= svc._last_sync_time <= after
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_retry_after_failed_sync_is_not_cooldown_blocked(self):
|
||||
call_count = 0
|
||||
|
||||
async def fail_then_succeed():
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count == 1:
|
||||
raise RuntimeError("temporary failure")
|
||||
return []
|
||||
|
||||
svc = _make_library_service()
|
||||
svc._lidarr_repo.get_library = fail_then_succeed
|
||||
|
||||
with patch("services.library_service.CacheStatusService") as mock_css:
|
||||
mock_css.return_value.is_syncing.return_value = False
|
||||
|
||||
with pytest.raises(Exception, match="temporary failure"):
|
||||
await svc.sync_library()
|
||||
|
||||
result = await svc.sync_library()
|
||||
|
||||
assert result.status == "success"
|
||||
|
||||
|
||||
class TestSyncFutureDedup:
|
||||
@pytest.mark.asyncio
|
||||
async def test_concurrent_syncs_deduplicated(self):
|
||||
"""Two concurrent sync calls should result in exactly one Lidarr call."""
|
||||
call_count = 0
|
||||
sync_event = asyncio.Event()
|
||||
|
||||
async def slow_get_library():
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
sync_event.set()
|
||||
await asyncio.sleep(0.05)
|
||||
return []
|
||||
|
||||
svc = _make_library_service()
|
||||
svc._lidarr_repo.get_library = slow_get_library
|
||||
|
||||
with patch("services.library_service.CacheStatusService") as mock_css:
|
||||
mock_css.return_value.is_syncing.return_value = False
|
||||
|
||||
results = await asyncio.gather(
|
||||
svc.sync_library(),
|
||||
svc.sync_library(),
|
||||
)
|
||||
|
||||
assert all(r.status == "success" for r in results)
|
||||
assert call_count == 1, f"Expected 1 Lidarr call, got {call_count}"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_concurrent_sync_failure_propagates_to_waiter(self):
|
||||
"""When the producer fails, deduped waiters get the real exception."""
|
||||
async def failing_get_library():
|
||||
await asyncio.sleep(0.05)
|
||||
raise RuntimeError("Lidarr DNS failure")
|
||||
|
||||
svc = _make_library_service()
|
||||
svc._lidarr_repo.get_library = failing_get_library
|
||||
|
||||
with patch("services.library_service.CacheStatusService") as mock_css:
|
||||
mock_css.return_value.is_syncing.return_value = False
|
||||
|
||||
results = await asyncio.gather(
|
||||
svc.sync_library(),
|
||||
svc.sync_library(),
|
||||
return_exceptions=True,
|
||||
)
|
||||
|
||||
# Both should get an error, not hang or get CancelledError
|
||||
for r in results:
|
||||
assert isinstance(r, Exception)
|
||||
assert not isinstance(r, asyncio.CancelledError), \
|
||||
"Waiter got CancelledError instead of the real exception"
|
||||
|
|
@ -145,7 +145,12 @@
|
|||
{#if ctrl.fetchError}
|
||||
<div role="alert" class="alert alert-error alert-soft mb-4">
|
||||
<CircleX class="h-6 w-6 shrink-0" />
|
||||
<span>{ctrl.fetchError}</span>
|
||||
<div class="flex flex-col gap-1">
|
||||
<span>{ctrl.fetchError}</span>
|
||||
{#if ctrl.fetchErrorCode === 'CIRCUIT_BREAKER_OPEN' || /connection|DNS|not configured/i.test(ctrl.fetchError)}
|
||||
<a href="/settings" class="link link-primary text-sm">Check your settings →</a>
|
||||
{/if}
|
||||
</div>
|
||||
<button class="btn btn-sm btn-ghost" onclick={() => ctrl.fetchAlbums(true)}>Retry</button>
|
||||
</div>
|
||||
{/if}
|
||||
|
|
|
|||
|
|
@ -73,6 +73,7 @@ export function createLibraryController<TAlbum>(adapter: LibraryAdapter<TAlbum>)
|
|||
let loading = $state(true);
|
||||
let loadingMore = $state(false);
|
||||
let fetchError = $state('');
|
||||
let fetchErrorCode = $state('');
|
||||
|
||||
let sortBy = $state(adapter.defaultSortBy);
|
||||
let sortOrder = $state(adapter.ascValue);
|
||||
|
|
@ -98,6 +99,7 @@ export function createLibraryController<TAlbum>(adapter: LibraryAdapter<TAlbum>)
|
|||
async function fetchAlbums(reset = false): Promise<void> {
|
||||
const id = ++fetchId;
|
||||
fetchError = '';
|
||||
fetchErrorCode = '';
|
||||
|
||||
if (albumsAbortController) albumsAbortController.abort();
|
||||
albumsAbortController = new AbortController();
|
||||
|
|
@ -145,6 +147,7 @@ export function createLibraryController<TAlbum>(adapter: LibraryAdapter<TAlbum>)
|
|||
if (isAbortError(e)) return;
|
||||
if (id === fetchId) {
|
||||
fetchError = e instanceof ApiError ? e.message : adapter.errorMessage;
|
||||
fetchErrorCode = e instanceof ApiError ? e.code : '';
|
||||
}
|
||||
} finally {
|
||||
if (id === fetchId) {
|
||||
|
|
@ -364,6 +367,9 @@ export function createLibraryController<TAlbum>(adapter: LibraryAdapter<TAlbum>)
|
|||
get fetchError() {
|
||||
return fetchError;
|
||||
},
|
||||
get fetchErrorCode() {
|
||||
return fetchErrorCode;
|
||||
},
|
||||
get sortBy() {
|
||||
return sortBy;
|
||||
},
|
||||
|
|
|
|||
|
|
@ -9,12 +9,14 @@
|
|||
import Pagination from '$lib/components/Pagination.svelte';
|
||||
import { recentlyAddedStore } from '$lib/stores/recentlyAdded';
|
||||
import { syncStatus } from '$lib/stores/syncStatus.svelte';
|
||||
import { api } from '$lib/api/client';
|
||||
import { api, ApiError } from '$lib/api/client';
|
||||
import { API } from '$lib/constants';
|
||||
import { isAbortError } from '$lib/utils/errorHandling';
|
||||
import type { Artist, Album } from '$lib/types';
|
||||
import { CircleX, X, RefreshCw, ChevronRight, Search, Loader2 } from 'lucide-svelte';
|
||||
|
||||
const CIRCUIT_BREAKER_CODE = 'CIRCUIT_BREAKER_OPEN';
|
||||
|
||||
type LibraryArtist = {
|
||||
name: string;
|
||||
mbid: string;
|
||||
|
|
@ -68,6 +70,7 @@
|
|||
let loadingStats = true;
|
||||
let syncing = false;
|
||||
let error: string | null = null;
|
||||
let errorCode: string | null = null;
|
||||
|
||||
let currentAlbumPage = 1;
|
||||
let sortBy = 'date_added';
|
||||
|
|
@ -82,6 +85,8 @@
|
|||
$: isSearching = searchQuery.trim().length > 0;
|
||||
$: totalAlbumPages = Math.ceil(albumsTotal / ALBUMS_PER_PAGE);
|
||||
$: lastSyncText = stats.last_sync ? new Date(stats.last_sync * 1000).toLocaleString() : 'Never';
|
||||
$: isConnectionError = errorCode === CIRCUIT_BREAKER_CODE ||
|
||||
(error != null && /connection|DNS|not configured/i.test(error));
|
||||
|
||||
onMount(() => {
|
||||
recentlyAddedStore.initialize();
|
||||
|
|
@ -120,7 +125,12 @@
|
|||
if (isAbortError(e)) return;
|
||||
if (id !== albumsFetchId) return;
|
||||
console.error("Couldn't load albums:", e);
|
||||
error = "Couldn't load albums";
|
||||
if (e instanceof ApiError) {
|
||||
error = e.message;
|
||||
errorCode = e.code;
|
||||
} else {
|
||||
error = "Couldn't load albums";
|
||||
}
|
||||
} finally {
|
||||
if (id === albumsFetchId) loadingAlbums = false;
|
||||
}
|
||||
|
|
@ -139,6 +149,7 @@
|
|||
|
||||
async function loadLibrary() {
|
||||
error = null;
|
||||
errorCode = null;
|
||||
loadingArtists = true;
|
||||
loadingStats = true;
|
||||
currentAlbumPage = 1;
|
||||
|
|
@ -152,13 +163,19 @@
|
|||
async function syncLibrary() {
|
||||
syncing = true;
|
||||
error = null;
|
||||
errorCode = null;
|
||||
try {
|
||||
await api.global.post('/api/v1/library/sync');
|
||||
syncStatus.checkStatus();
|
||||
await loadLibrary();
|
||||
} catch (e) {
|
||||
console.error('Sync failed:', e);
|
||||
error = e instanceof Error ? e.message : "Couldn't sync the library";
|
||||
if (e instanceof ApiError) {
|
||||
error = e.message;
|
||||
errorCode = e.code;
|
||||
} else {
|
||||
error = e instanceof Error ? e.message : "Couldn't sync the library";
|
||||
}
|
||||
} finally {
|
||||
syncing = false;
|
||||
}
|
||||
|
|
@ -213,18 +230,24 @@
|
|||
{#if error}
|
||||
<div class="alert alert-error mb-6">
|
||||
<CircleX class="h-6 w-6 shrink-0" />
|
||||
<span>{error}</span>
|
||||
<div class="flex flex-col gap-1">
|
||||
<span>{error}</span>
|
||||
{#if isConnectionError}
|
||||
<a href="/settings" class="link link-primary text-sm">Check Lidarr settings →</a>
|
||||
{/if}
|
||||
</div>
|
||||
<div class="flex gap-2">
|
||||
<button
|
||||
class="btn btn-sm"
|
||||
onclick={() => {
|
||||
error = null;
|
||||
errorCode = null;
|
||||
loadLibrary();
|
||||
}}>Retry</button
|
||||
>
|
||||
<button
|
||||
class="btn btn-sm btn-circle btn-ghost"
|
||||
onclick={() => (error = null)}
|
||||
onclick={() => { error = null; errorCode = null; }}
|
||||
aria-label="Dismiss"
|
||||
>
|
||||
<X class="h-4 w-4" />
|
||||
|
|
|
|||
Loading…
Reference in a new issue