Report Qobuz API failures cleanly instead of crashing; keep credentials out of errors

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: ("<redacted>" 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

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.
+    """

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}")

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 "<redacted>" 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