chore: keep local design assets out of the repo

.gitignore

@@ -1,6 +1,9 @@
 # secrets
 .env
 
+# local working material (design mocks, scratch), never shipped
+.local/
+
 # python
 __pycache__/
 *.pyc

backend/app/adapters/spotify/client.py

@@ -76,47 +76,114 @@ class SpotifyClient:
         params: dict[str, str | int] | None = None,
         json: dict[str, object] | None = None,
     ) -> httpx2.Response:
-        has_retried_authentication = False
-        has_retried_rate_limit = False
+        access_token = await self._access_token()
+        response = await self._send(
+            method,
+            path,
+            access_token,
+            params=params,
+            json=json,
+        )
+        response = await self._retry_once_if_unauthorized(
+            response,
+            method,
+            path,
+            access_token,
+            params=params,
+            json=json,
+        )
+        response = await self._retry_once_if_rate_limited(
+            response,
+            method,
+            path,
+            params=params,
+            json=json,
+        )
+        return self._raise_for_error(response)
 
-        while True:
-            access_token = await self._access_token()
-            response = await self.http.request(
-                method,
-                f"{self.settings.spotify_api_base_url.rstrip('/')}{path}",
-                params=params,
-                json=json,
-                headers={"Authorization": f"Bearer {access_token}"},
-            )
+    async def _send(
+        self,
+        method: str,
+        path: str,
+        access_token: str,
+        *,
+        params: dict[str, str | int] | None,
+        json: dict[str, object] | None,
+    ) -> httpx2.Response:
+        return await self.http.request(
+            method,
+            f"{self.settings.spotify_api_base_url.rstrip('/')}{path}",
+            params=params,
+            json=json,
+            headers={"Authorization": f"Bearer {access_token}"},
+        )
 
-            if response.status_code == 401:
-                if has_retried_authentication:
-                    raise SpotifyAuthenticationError("Spotify rejected refreshed authentication")
-                await self._refresh_if_current(access_token)
-                has_retried_authentication = True
-                continue
-
-            if response.status_code == 429:
-                retry_after_seconds = _parse_retry_after(response)
-                if (
-                    method == "GET"
-                    and not has_retried_rate_limit
-                    and retry_after_seconds is not None
-                    and retry_after_seconds <= self.settings.spotify_retry_after_cap_seconds
-                ):
-                    await asyncio.sleep(retry_after_seconds)
-                    has_retried_rate_limit = True
-                    continue
-                raise SpotifyRateLimitedError(retry_after_seconds)
-
-            if response.status_code >= 500:
-                raise SpotifyUnavailableError(
-                    f"Spotify is unavailable with status {response.status_code}"
-                )
-            if response.status_code >= 400:
-                raise SpotifyRequestError(response.status_code)
+    async def _retry_once_if_unauthorized(
+        self,
+        response: httpx2.Response,
+        method: str,
+        path: str,
+        access_token: str,
+        *,
+        params: dict[str, str | int] | None,
+        json: dict[str, object] | None,
+    ) -> httpx2.Response:
+        if response.status_code != 401:
             return response
 
+        await self._refresh_if_current(access_token)
+        return await self._send(
+            method,
+            path,
+            self.session.tokens.access_token,
+            params=params,
+            json=json,
+        )
+
+    async def _retry_once_if_rate_limited(
+        self,
+        response: httpx2.Response,
+        method: str,
+        path: str,
+        *,
+        params: dict[str, str | int] | None,
+        json: dict[str, object] | None,
+    ) -> httpx2.Response:
+        if response.status_code != 429:
+            return response
+
+        retry_after_seconds = _parse_retry_after(response)
+        _, reason = _parse_error_details(response)
+        if (
+            method != "GET"
+            or reason == "QUOTA_EXCEEDED"
+            or retry_after_seconds is None
+            or retry_after_seconds > self.settings.spotify_retry_after_cap_seconds
+        ):
+            return response
+
+        await asyncio.sleep(retry_after_seconds)
+        return await self._send(
+            method,
+            path,
+            self.session.tokens.access_token,
+            params=params,
+            json=json,
+        )
+
+    def _raise_for_error(self, response: httpx2.Response) -> httpx2.Response:
+        if response.status_code < 400:
+            return response
+
+        message, reason = _parse_error_details(response)
+        if response.status_code == 401:
+            raise SpotifyAuthenticationError("Spotify rejected refreshed authentication")
+        if response.status_code == 429:
+            raise SpotifyRateLimitedError(_parse_retry_after(response), reason)
+        if response.status_code >= 500:
+            raise SpotifyUnavailableError(response.status_code, message)
+        raise SpotifyRequestError(response.status_code, message)
+
     async def _access_token(self) -> str:
         access_token = self.session.tokens.access_token
         if self.session.tokens.is_expired:
@@ -143,3 +210,22 @@ def _parse_retry_after(response: httpx2.Response) -> float | None:
     except ValueError:
         return None
     return retry_after_seconds if retry_after_seconds >= 0 else None
+
+
+def _parse_error_details(response: httpx2.Response) -> tuple[str | None, str | None]:
+    try:
+        payload: object = response.json()
+    except ValueError:
+        return None, None
+    if not isinstance(payload, dict):
+        return None, None
+
+    error = payload.get("error")
+    if not isinstance(error, dict):
+        return None, None
+    message = error.get("message")
+    reason = error.get("reason")
+    return (
+        message if isinstance(message, str) else None,
+        reason if isinstance(reason, str) else None,
+    )

backend/app/adapters/spotify/errors.py

@@ -12,20 +12,30 @@ class SpotifyAuthenticationError(SpotifyError):
 class SpotifyRateLimitedError(SpotifyError):
     """Spotify rate limited a request that could not be retried."""
 
-    def __init__(self, retry_after_seconds: float | None) -> None:
+    def __init__(self, retry_after_seconds: float | None, reason: str | None = None) -> None:
         """Record Spotify's requested delay without exposing request data."""
         super().__init__("Spotify rate limit exceeded")
         self.retry_after_seconds = retry_after_seconds
+        self.reason = reason
 
 
 class SpotifyUnavailableError(SpotifyError):
     """Spotify returned a server-side failure."""
 
+    def __init__(self, status_code: int, message: str | None = None) -> None:
+        """Record safe details from a Spotify server failure."""
+        generic_message = f"Spotify is unavailable with status {status_code}"
+        super().__init__(message if message is not None else generic_message)
+        self.status_code = status_code
+        self.message = message
+
 
 class SpotifyRequestError(SpotifyError):
     """Spotify rejected a non-authenticated API request."""
 
-    def __init__(self, status_code: int) -> None:
-        """Record the response status without exposing response content."""
-        super().__init__(f"Spotify request failed with status {status_code}")
+    def __init__(self, status_code: int, message: str | None = None) -> None:
+        """Record safe details from a rejected Spotify request."""
+        generic_message = f"Spotify request failed with status {status_code}"
+        super().__init__(message if message is not None else generic_message)
         self.status_code = status_code
+        self.message = message

backend/app/adapters/spotify/login.py

@@ -0,0 +1,66 @@
+"""Spotify login workflow independent of HTTP routing."""
+
+import secrets
+
+import httpx2
+
+from app.adapters.spotify.auth import (
+    build_authorize_url,
+    derive_code_challenge,
+    exchange_authorization_code,
+    generate_code_verifier,
+)
+from app.adapters.spotify.client import SpotifyClient
+from app.adapters.spotify.errors import SpotifyError
+from app.adapters.spotify.session import PendingLogins, SessionStore, SpotifySession
+from app.config import Settings
+
+
+def begin_login(settings: Settings, pending_logins: PendingLogins) -> str:
+    """Store a pending PKCE login and return its Spotify authorization URL."""
+    state = secrets.token_urlsafe(32)
+    code_verifier = generate_code_verifier()
+    pending_logins.add(state, code_verifier)
+    return build_authorize_url(
+        settings.spotify_client_id,
+        settings.spotify_redirect_uri,
+        state,
+        derive_code_challenge(code_verifier),
+    )
+
+
+async def complete_login(
+    http: httpx2.AsyncClient,
+    settings: Settings,
+    pending_logins: PendingLogins,
+    session_store: SessionStore,
+    code: str,
+    state: str,
+) -> str | None:
+    """Complete a pending Spotify login and return its application session ID."""
+    code_verifier = pending_logins.pop(state)
+    if code_verifier is None:
+        return None
+
+    try:
+        tokens = await exchange_authorization_code(
+            http,
+            client_id=settings.spotify_client_id,
+            redirect_uri=settings.spotify_redirect_uri,
+            code=code,
+            code_verifier=code_verifier,
+        )
+        bootstrap_session = SpotifySession(tokens=tokens, account_id="", display_name="")
+        current_user = await SpotifyClient(
+            http,
+            bootstrap_session,
+            settings,
+        ).fetch_current_user()
+        session = SpotifySession(
+            tokens=bootstrap_session.tokens,
+            account_id=current_user.account_id,
+            display_name=current_user.display_name,
+        )
+        return session_store.create(session)
+    except (SpotifyError, ValueError):
+        return None

backend/app/api/routes.py

@@ -1,21 +1,13 @@
 """HTTP routes for Spotify login and session management."""
 
-import secrets
 from typing import cast
 
 import httpx2
 from fastapi import APIRouter, Request
 from fastapi.responses import JSONResponse, RedirectResponse, Response
 
-from app.adapters.spotify.auth import (
-    build_authorize_url,
-    derive_code_challenge,
-    exchange_authorization_code,
-    generate_code_verifier,
-)
-from app.adapters.spotify.client import SpotifyClient
-from app.adapters.spotify.errors import SpotifyError
-from app.adapters.spotify.session import PendingLogins, SessionStore, SpotifySession
+from app.adapters.spotify.login import begin_login, complete_login
+from app.adapters.spotify.session import PendingLogins, SessionStore
 from app.config import Settings
 
 SESSION_COOKIE_NAME = "discovery_session"
@@ -28,15 +20,7 @@ def login(request: Request) -> RedirectResponse:
     """Start Spotify Authorization Code with PKCE login."""
     application_settings = cast(Settings, request.app.state.settings)
     pending_logins = cast(PendingLogins, request.app.state.pending_logins)
-    state = secrets.token_urlsafe(32)
-    code_verifier = generate_code_verifier()
-    pending_logins.add(state, code_verifier)
-    authorize_url = build_authorize_url(
-        application_settings.spotify_client_id,
-        application_settings.spotify_redirect_uri,
-        state,
-        derive_code_challenge(code_verifier),
-    )
+    authorize_url = begin_login(application_settings, pending_logins)
     return RedirectResponse(authorize_url, status_code=307)
 
 
@@ -51,37 +35,21 @@ async def callback(
     if error is not None or code is None or state is None:
         return _login_error_redirect()
 
-    pending_logins = cast(PendingLogins, request.app.state.pending_logins)
-    code_verifier = pending_logins.pop(state)
-    if code_verifier is None:
-        return _login_error_redirect()
-
     application_settings = cast(Settings, request.app.state.settings)
     http = cast(httpx2.AsyncClient, request.app.state.http)
-    try:
-        tokens = await exchange_authorization_code(
-            http,
-            client_id=application_settings.spotify_client_id,
-            redirect_uri=application_settings.spotify_redirect_uri,
-            code=code,
-            code_verifier=code_verifier,
-        )
-        bootstrap_session = SpotifySession(tokens=tokens, account_id="", display_name="")
-        current_user = await SpotifyClient(
-            http,
-            bootstrap_session,
-            application_settings,
-        ).fetch_current_user()
-    except (SpotifyError, ValueError):
+    pending_logins = cast(PendingLogins, request.app.state.pending_logins)
+    session_store = cast(SessionStore, request.app.state.session_store)
+    session_id = await complete_login(
+        http,
+        application_settings,
+        pending_logins,
+        session_store,
+        code,
+        state,
+    )
+    if session_id is None:
         return _login_error_redirect()
 
-    session = SpotifySession(
-        tokens=bootstrap_session.tokens,
-        account_id=current_user.account_id,
-        display_name=current_user.display_name,
-    )
-    session_store = cast(SessionStore, request.app.state.session_store)
-    session_id = session_store.create(session)
     response = RedirectResponse("/", status_code=307)
     response.set_cookie(
         SESSION_COOKIE_NAME,

backend/tests/test_auth_routes.py

@@ -70,5 +70,25 @@ def test_unknown_callback_state_redirects_to_login_error() -> None:
     assert response.headers["location"] == "/?login=error"
 
 
+def test_spotify_failure_during_callback_redirects_to_login_error() -> None:
+    async def spotify_handler(request: httpx2.Request) -> httpx2.Response:
+        return httpx2.Response(400)
+
+    app = create_app(
+        application_settings=_live_settings(),
+        http_transport=httpx2.MockTransport(spotify_handler),
+    )
+    with TestClient(app, follow_redirects=False) as client:
+        login_response = client.get("/api/auth/login")
+        authorize_query = parse_qs(urlparse(login_response.headers["location"]).query)
+        response = client.get(
+            "/callback",
+            params={"code": "code", "state": authorize_query["state"][0]},
+        )
+
+    assert response.status_code == 307
+    assert response.headers["location"] == "/?login=error"
+
+
 def _live_settings() -> Settings:
     return Settings(app_mode=AppMode.LIVE, spotify_client_id="client-id")

backend/tests/test_spotify_client.py

@@ -9,7 +9,7 @@ import pytest
 
 from app.adapters.spotify.auth import TokenSet
 from app.adapters.spotify.client import SpotifyClient
-from app.adapters.spotify.errors import SpotifyRateLimitedError
+from app.adapters.spotify.errors import SpotifyRateLimitedError, SpotifyRequestError
 from app.adapters.spotify.session import SpotifySession
 from app.config import Settings
 from app.domain.models import Track
@@ -110,6 +110,85 @@ def test_get_rate_limit_above_cap_raises_without_retry() -> None:
     asyncio.run(run())
 
 
+def test_quota_exhaustion_raises_without_retry_or_sleep(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    async def run() -> None:
+        api_calls = 0
+        sleep_calls = 0
+
+        async def fake_sleep(delay: float) -> None:
+            nonlocal sleep_calls
+            sleep_calls += 1
+
+        async def handler(request: httpx2.Request) -> httpx2.Response:
+            nonlocal api_calls
+            api_calls += 1
+            return httpx2.Response(
+                429,
+                headers={"Retry-After": "0"},
+                json={
+                    "error": {
+                        "message": "Quota exhausted",
+                        "reason": "QUOTA_EXCEEDED",
+                    }
+                },
+            )
+
+        monkeypatch.setattr("app.adapters.spotify.client.asyncio.sleep", fake_sleep)
+        with pytest.raises(SpotifyRateLimitedError) as error:
+            await _search_with_handler(handler)
+
+        assert error.value.reason == "QUOTA_EXCEEDED"
+        assert api_calls == 1
+        assert sleep_calls == 0
+
+    asyncio.run(run())
+
+
+def test_authentication_then_rate_limit_retries_each_policy_once() -> None:
+    async def run() -> None:
+        token_calls = 0
+        api_calls = 0
+
+        async def handler(request: httpx2.Request) -> httpx2.Response:
+            nonlocal api_calls, token_calls
+            if request.url.host == "accounts.spotify.com":
+                token_calls += 1
+                return _token_response()
+            api_calls += 1
+            if api_calls == 1:
+                return httpx2.Response(401)
+            if api_calls == 2:
+                return httpx2.Response(429, headers={"Retry-After": "0"})
+            return httpx2.Response(200, json=_search_payload())
+
+        tracks = await _search_with_handler(handler)
+
+        assert len(tracks) == 1
+        assert token_calls == 1
+        assert api_calls == 3
+
+    asyncio.run(run())
+
+
+def test_request_error_includes_parsed_spotify_message() -> None:
+    async def run() -> None:
+        async def handler(request: httpx2.Request) -> httpx2.Response:
+            return httpx2.Response(
+                400,
+                json={"error": {"message": "Invalid search request"}},
+            )
+
+        with pytest.raises(SpotifyRequestError) as error:
+            await _search_with_handler(handler)
+
+        assert error.value.message == "Invalid search request"
+        assert str(error.value) == "Invalid search request"
+
+    asyncio.run(run())
+
+
 def test_playlist_write_rate_limit_is_not_retried() -> None:
     async def run() -> None:
         api_calls = 0