diff --git a/streamrip/client/qobuz.py b/streamrip/client/qobuz.py index 734e2b82..f2a33cf3 100644 --- a/streamrip/client/qobuz.py +++ b/streamrip/client/qobuz.py @@ -11,6 +11,7 @@ from ..config import Config from ..exceptions import ( + APIError, AuthenticationError, IneligibleError, InvalidAppIdError, @@ -23,6 +24,15 @@ logger = logging.getLogger("streamrip") +# Qobuz takes credentials as URL query parameters, so they must be masked +# anywhere params reach a log line or an exception message. +_SENSITIVE_PARAMS = ("user_auth_token", "password", "email", "user_id", "request_sig") + + +def _redacted(params: dict) -> dict: + """params with credentials masked, for logs and error messages.""" + return {k: ("" if k in _SENSITIVE_PARAMS else v) for k, v in params.items()} + QOBUZ_BASE_URL = "https://www.qobuz.com/api.json/0.2" QOBUZ_FEATURED_KEYS = { @@ -193,14 +203,22 @@ async def login(self): "app_id": str(c.app_id), } - logger.debug("Request params %s", params) + logger.debug("Request params %s", _redacted(params)) status, resp = await self._api_request("user/login", params) - logger.debug("Login resp: %s", resp) + # The response carries the user_auth_token and the account profile. + logger.debug("Login response keys: %s", sorted(resp)) if status == 401: - raise AuthenticationError(f"Invalid credentials from params {params}") + raise AuthenticationError( + f"Invalid credentials from params {_redacted(params)}" + ) elif status == 400: - raise InvalidAppIdError(f"Invalid app id from params {params}") + raise InvalidAppIdError(f"Invalid app id from params {_redacted(params)}") + elif status != 200: + raise APIError( + f"Qobuz login failed (HTTP {status}): " + f"{resp.get('message') or 'no message'}" + ) logger.debug("Logged in to Qobuz") @@ -339,6 +357,33 @@ async def get_downloadable(self, item: str, quality: int) -> Downloadable: self.session, stream_url, "flac" if quality > 1 else "mp3", source="qobuz" ) + async def _request_ok(self, epoint: str, params: dict) -> dict: + """_api_request that insists on HTTP 200, retrying once if Qobuz blips. + + Qobuz's search backend fails intermittently -- a 400 reading + "Impossible to connect, please check your Algolia Application Id." + that succeeds moments later -- and its edge sometimes answers with a + 502 HTML page. One short retry absorbs those; anything else is raised + with Qobuz's own message rather than a bare AssertionError. + """ + for attempt in (1, 2): + status, page = await self._api_request(epoint, params) + if status == 200: + return page + message = (page.get("message") if isinstance(page, dict) else None) or "" + transient = status >= 500 or "Algolia" in message + if attempt == 1 and transient: + logger.warning( + "Qobuz %s failed (HTTP %d: %s) -- retrying once", + epoint, + status, + message or "no message", + ) + await asyncio.sleep(3) + continue + break + raise APIError(f"Qobuz {epoint} failed (HTTP {status}): {message or 'no message'}") + async def _paginate( self, epoint: str, @@ -356,9 +401,8 @@ async def _paginate( Generator that yields (status code, response) tuples """ params.update({"limit": limit}) - status, page = await self._api_request(epoint, params) - assert status == 200, status - logger.debug("paginate: initial request made with status %d", status) + page = await self._request_ok(epoint, params) + logger.debug("paginate: initial request succeeded") # albums, tracks, etc. key = epoint.split("/")[0] + "s" items = page.get(key, {}) @@ -380,17 +424,13 @@ async def _paginate( pages = [] requests = [] - assert status == 200, status pages.append(page) while (offset + limit) < total: offset += limit params.update({"offset": offset}) - requests.append(self._api_request(epoint, params.copy())) - - for status, resp in await asyncio.gather(*requests): - assert status == 200 - pages.append(resp) + requests.append(self._request_ok(epoint, params.copy())) + pages.extend(await asyncio.gather(*requests)) return pages async def _get_app_id_and_secrets(self) -> tuple[str, list[str]]: @@ -444,9 +484,17 @@ async def _api_request(self, epoint: str, params: dict) -> tuple[int, dict]: returns: status code, json parsed response """ url = f"{QOBUZ_BASE_URL}/{epoint}" - logger.debug("api_request: endpoint=%s, params=%s", epoint, params) + logger.debug("api_request: endpoint=%s, params=%s", epoint, _redacted(params)) async with self.rate_limiter: async with self.session.get(url, params=params) as response: + if "json" not in (response.content_type or ""): + # An HTML error page, such as a 502 from Qobuz's edge. + # aiohttp's ContentTypeError would quote the full request + # URL, which carries user_auth_token -- so report the + # status instead of letting that propagate. + return response.status, { + "message": f"non-JSON response ({response.content_type})" + } return response.status, await response.json() @staticmethod diff --git a/streamrip/exceptions.py b/streamrip/exceptions.py index 687b1a14..a13d359e 100644 --- a/streamrip/exceptions.py +++ b/streamrip/exceptions.py @@ -68,3 +68,10 @@ def print_msg(self, item) -> str: class ConversionError(Exception): """ConversionError.""" + + +class APIError(Exception): + """A streaming service answered a request with an error. + + The message carries the service's own explanation where it gave one. + """ diff --git a/streamrip/rip/main.py b/streamrip/rip/main.py index 562773fc..da5f38b0 100644 --- a/streamrip/rip/main.py +++ b/streamrip/rip/main.py @@ -9,6 +9,7 @@ from ..client import Client, DeezerClient, QobuzClient, SoundcloudClient, TidalClient from ..config import Config from ..console import console +from ..exceptions import APIError from ..media import ( Media, Pending, @@ -185,7 +186,11 @@ async def search_interactive(self, source: str, media_type: str, query: str): client = await self.get_logged_in_client(source) with console.status(f"[bold]Searching {source}", spinner="dots"): - pages = await client.search(media_type, query, limit=100) + try: + pages = await client.search(media_type, query, limit=100) + except APIError as e: + console.print(f"[red]Search failed: {e}") + return if len(pages) == 0: console.print(f"[red]No search results found for query {query}") return @@ -236,7 +241,11 @@ async def search_interactive(self, source: str, media_type: str, query: str): async def search_take_first(self, source: str, media_type: str, query: str): client = await self.get_logged_in_client(source) with console.status(f"[bold]Searching {source}", spinner="dots"): - pages = await client.search(media_type, query, limit=1) + try: + pages = await client.search(media_type, query, limit=1) + except APIError as e: + console.print(f"[red]Search failed: {e}") + return if len(pages) == 0: console.print(f"[red]No search results found for query {query}") @@ -252,7 +261,11 @@ async def search_output_file( ): client = await self.get_logged_in_client(source) with console.status(f"[bold]Searching {source}", spinner="dots"): - pages = await client.search(media_type, query, limit=limit) + try: + pages = await client.search(media_type, query, limit=limit) + except APIError as e: + console.print(f"[red]Search failed: {e}") + return if len(pages) == 0: console.print(f"[red]No search results found for query {query}") diff --git a/tests/test_qobuz_errors.py b/tests/test_qobuz_errors.py new file mode 100644 index 00000000..3a295d30 --- /dev/null +++ b/tests/test_qobuz_errors.py @@ -0,0 +1,118 @@ +"""Qobuz failures should explain themselves, and never quote credentials. + +`rip search qobuz ...` crashed with a bare `AssertionError: 400`. The 400 was +Qobuz's search backend failing transiently -- "Impossible to connect, please +check your Algolia Application Id." -- and the same search succeeded seconds +later. Separately, Qobuz takes credentials as URL query parameters, and both an +HTML error page and a failed login used to put them into error output. +""" + +import contextlib +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from streamrip.client.qobuz import QobuzClient +from streamrip.exceptions import APIError, AuthenticationError + +ALGOLIA = {"message": "Impossible to connect, please check your Algolia Application Id."} +TOKEN = "SECRET-TOKEN-abcdefghij" + + +@pytest.fixture(autouse=True) +def no_wait(): + with patch("streamrip.client.qobuz.asyncio.sleep", new=AsyncMock()): + yield + + +def _client(*responses): + c = QobuzClient.__new__(QobuzClient) + c._api_request = AsyncMock(side_effect=list(responses)) + return c + + +@pytest.mark.asyncio +async def test_transient_failure_is_retried_once(): + ok = {"artists": {"items": [], "total": 0}} + c = _client((400, ALGOLIA), (200, ok)) + assert await c._request_ok("artist/search", {}) == ok + assert c._api_request.await_count == 2 + + +@pytest.mark.asyncio +async def test_persistent_failure_raises_with_qobuz_message(): + c = _client((400, ALGOLIA), (400, ALGOLIA)) + with pytest.raises(APIError, match="Algolia"): + await c.search("artist", "Radioaktivists", limit=100) + assert c._api_request.await_count == 2 + + +@pytest.mark.asyncio +async def test_other_errors_are_not_retried(): + c = _client((400, {"message": "Invalid parameter"})) + with pytest.raises(APIError, match="Invalid parameter"): + await c._request_ok("artist/search", {}) + assert c._api_request.await_count == 1 + + +class _HtmlErrorPage: + status = 502 + content_type = "text/html" + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + async def json(self): + raise AssertionError("must not parse an HTML page as JSON") + + +@pytest.mark.asyncio +async def test_html_error_page_is_reported_by_status_without_the_url(): + c = QobuzClient.__new__(QobuzClient) + c.rate_limiter = contextlib.nullcontext() + c.session = MagicMock() + c.session.get = MagicMock(return_value=_HtmlErrorPage()) + status, page = await c._api_request( + "user/login", {"user_id": "123456789", "user_auth_token": TOKEN} + ) + assert status == 502 + assert "non-JSON" in page["message"] + assert TOKEN not in str(page) + + +@pytest.mark.asyncio +async def test_failed_login_does_not_quote_the_token(): + c = QobuzClient.__new__(QobuzClient) + c.logged_in = False + c.config = MagicMock() + q = c.config.session.qobuz + q.use_auth_token = True + q.email_or_userid = "123456789" + q.password_or_token = TOKEN + q.app_id = "987654321" + q.secrets = ["s1"] + c._api_request = AsyncMock(return_value=(401, {})) + with patch.object(QobuzClient, "get_session", new=AsyncMock(return_value=MagicMock())): + with pytest.raises(AuthenticationError) as err: + await c.login() + assert TOKEN not in str(err.value) + assert "" in str(err.value) + + +@pytest.mark.asyncio +async def test_search_command_reports_failure_instead_of_crashing(): + from streamrip.rip.main import Main + + m = Main.__new__(Main) + client = MagicMock() + client.search = AsyncMock( + side_effect=APIError("Qobuz artist/search failed (HTTP 400): Algolia") + ) + m.get_logged_in_client = AsyncMock(return_value=client) + with patch("streamrip.rip.main.console") as console: + await m.search_interactive("qobuz", "artist", "Radioaktivists") + printed = " ".join(str(c.args[0]) for c in console.print.call_args_list) + assert "Search failed" in printed and "Algolia" in printed