Re-raise error from urllib as ScryfallError and fix tests

scrython/base.py

@@ -16,8 +16,8 @@ def __init__(self, scryfall_data: dict[str, Any], *args: Any, **kwargs: Any) ->
         self._status: int = scryfall_data["status"]
         self._code: str = scryfall_data["code"]
         self._details: str = scryfall_data["details"]
-        self._type: str | None = scryfall_data["type"]
-        self._warnings: list[str] | None = scryfall_data["warnings"]
+        self._type: str | None = scryfall_data.get("type")
+        self._warnings: list[str] | None = scryfall_data.get("warnings")
 
     @property
     def status(self) -> int:
@@ -220,6 +220,20 @@ def _fetch_raw(self, url: str, cache_key: str | None = None, **kwargs: Any) -> d
 
                 return response_data
         except urllib.error.HTTPError as exc:
+            # Scryfall returns JSON error bodies on 4xx/5xx responses.
+            # urllib raises HTTPError before we can read the body normally,
+            # but the HTTPError itself is a file-like object containing it.
+            try:
+                charset = exc.headers.get_param("charset")
+                if not isinstance(charset, str):
+                    charset = "utf-8"
+                error_data = json.loads(exc.read().decode(charset))
+            except (json.JSONDecodeError, UnicodeDecodeError):
+                raise Exception(f"{exc}: {request.get_full_url()}") from exc
+
+            if error_data.get("object") == "error":
+                raise ScryfallError(error_data, error_data["details"]) from exc
+
             raise Exception(f"{exc}: {request.get_full_url()}") from exc
 
     def _fetch(self, **kwargs: Any) -> None:

tests/conftest.py

@@ -1,6 +1,9 @@
 """Pytest configuration and shared fixtures for Scrython tests."""
 
+import http.client
+import io
 import json
+import urllib.error
 from pathlib import Path
 from unittest.mock import Mock, patch
 
@@ -138,11 +141,10 @@ def set_error_response(self, error_data):
 
         def __call__(self, request):
             # Record the call for assertion purposes
+            url = request.get_full_url() if hasattr(request, "get_full_url") else str(request)
             self.calls.append(
                 {
-                    "url": (
-                        request.get_full_url() if hasattr(request, "get_full_url") else str(request)
-                    ),
+                    "url": url,
                     "method": request.get_method() if hasattr(request, "get_method") else "GET",
                     "headers": dict(request.headers) if hasattr(request, "headers") else {},
                 }
@@ -151,6 +153,23 @@ def __call__(self, request):
             if self.response_data is None:
                 raise ValueError("No response data set. Call set_response() first.")
 
+            # Real urlopen raises HTTPError for 4xx/5xx status codes
+            if self.status >= 400:
+                body = (
+                    self.response_data.encode("utf-8")
+                    if isinstance(self.response_data, str)
+                    else self.response_data
+                )
+                headers = http.client.HTTPMessage()
+                headers["Content-Type"] = "application/json; charset=utf-8"
+                raise urllib.error.HTTPError(
+                    url=url,
+                    code=self.status,
+                    msg=f"HTTP Error {self.status}",
+                    hdrs=headers,
+                    fp=io.BytesIO(body),
+                )
+
             return MockURLResponse(self.response_data, self.status)
 
     mock = MockURLOpen()

tests/test_caching.py

@@ -3,7 +3,7 @@
 import contextlib
 import time
 
-from scrython.base import ScrythonRequestHandler
+from scrython.base import ScryfallError, ScrythonRequestHandler
 from scrython.cache import MemoryCache, generate_cache_key, get_global_cache, reset_global_cache
 
 
@@ -271,7 +271,7 @@ def test_cache_doesnt_store_errors(self, mock_urlopen):
         class TestHandler(ScrythonRequestHandler):
             _endpoint = "cards/named"
 
-        with contextlib.suppress(Exception):
+        with contextlib.suppress(ScryfallError):
             _handler = TestHandler(fuzzy="Nonexistent", cache=True)
 
         # Cache should be empty (errors not cached)

tests/test_rate_limiting.py

@@ -5,7 +5,7 @@
 
 import pytest
 
-from scrython.base import ScrythonRequestHandler
+from scrython.base import ScryfallError, ScrythonRequestHandler
 from scrython.rate_limiter import RateLimiter
 
 
@@ -144,7 +144,10 @@ class TestRequestHandlerRateLimiting:
     @pytest.fixture
     def mock_urlopen_with_rate_limit(self):
         """Mock urlopen without disabling rate limiting."""
+        import http.client
+        import io
         import json
+        import urllib.error
         from unittest.mock import Mock, patch
 
         class MockURLResponse:
@@ -193,13 +196,10 @@ def set_error_response(self, error_data):
                 self.status = error_data.get("status", 404)
 
             def __call__(self, request):
+                url = request.get_full_url() if hasattr(request, "get_full_url") else str(request)
                 self.calls.append(
                     {
-                        "url": (
-                            request.get_full_url()
-                            if hasattr(request, "get_full_url")
-                            else str(request)
-                        ),
+                        "url": url,
                         "method": request.get_method() if hasattr(request, "get_method") else "GET",
                         "headers": dict(request.headers) if hasattr(request, "headers") else {},
                     }
@@ -208,6 +208,23 @@ def __call__(self, request):
                 if self.response_data is None:
                     raise ValueError("No response data set. Call set_response() first.")
 
+                # Real urlopen raises HTTPError for 4xx/5xx status codes
+                if self.status >= 400:
+                    body = (
+                        self.response_data.encode("utf-8")
+                        if isinstance(self.response_data, str)
+                        else self.response_data
+                    )
+                    headers = http.client.HTTPMessage()
+                    headers["Content-Type"] = "application/json; charset=utf-8"
+                    raise urllib.error.HTTPError(
+                        url=url,
+                        code=self.status,
+                        msg=f"HTTP Error {self.status}",
+                        hdrs=headers,
+                        fp=io.BytesIO(body),
+                    )
+
                 return MockURLResponse(self.response_data, self.status)
 
         mock = MockURLOpen()
@@ -418,10 +435,10 @@ class TestHandler(ScrythonRequestHandler):
 
         # Make two calls that will error
         start = time.time()
-        with contextlib.suppress(Exception):
+        with contextlib.suppress(ScryfallError):
             _handler1 = TestHandler(fuzzy="Nonexistent 1")
 
-        with contextlib.suppress(Exception):
+        with contextlib.suppress(ScryfallError):
             _handler2 = TestHandler(fuzzy="Nonexistent 2")
 
         elapsed = time.time() - start