Compare commits
4 commits
1e1c40f103
...
1fe7349952
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1fe7349952 | ||
|
|
4bc48663c8 | ||
|
|
6769833f7e | ||
|
|
cdab1b4dd5 |
7 changed files with 320 additions and 88 deletions
3
.gitignore
vendored
3
.gitignore
vendored
|
|
@ -1,6 +1,9 @@
|
|||
# secrets
|
||||
.env
|
||||
|
||||
# local working material (design mocks, scratch), never shipped
|
||||
.local/
|
||||
|
||||
# python
|
||||
__pycache__/
|
||||
*.pyc
|
||||
|
|
|
|||
|
|
@ -76,12 +76,41 @@ 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
|
||||
|
||||
while True:
|
||||
access_token = await self._access_token()
|
||||
response = await self.http.request(
|
||||
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)
|
||||
|
||||
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,
|
||||
|
|
@ -89,34 +118,72 @@ class SpotifyClient:
|
|||
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,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
66
backend/app/adapters/spotify/login.py
Normal file
66
backend/app/adapters/spotify/login.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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(
|
||||
pending_logins = cast(PendingLogins, request.app.state.pending_logins)
|
||||
session_store = cast(SessionStore, request.app.state.session_store)
|
||||
session_id = await complete_login(
|
||||
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,
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue