diff --git a/src/storage/src/storage3/exceptions.py b/src/storage/src/storage3/exceptions.py index 34cb7316..47625824 100644 --- a/src/storage/src/storage3/exceptions.py +++ b/src/storage/src/storage3/exceptions.py @@ -14,13 +14,19 @@ def __init__(self, msg: str) -> None: self.msg = msg -class VectorBucketErrorMessage(BaseModel): +class StorageApiErrorMessage(BaseModel): + """Wire format of a storage API error body.""" + statusCode: str | int error: str message: str code: str | None = None +# Kept as an alias: the vector endpoints return the same error body. +VectorBucketErrorMessage = StorageApiErrorMessage + + @dataclass class StorageApiError(StorageException): message: str @@ -34,15 +40,24 @@ def __str__(self) -> str: return f"StorageApiError(message='{self.message}', code={self.code}, status='{self.status}')" -StorageApiErrorParser = TypeAdapter(StorageApiError) +StorageApiErrorParser = TypeAdapter(StorageApiErrorMessage) def parse_api_error(response: Response) -> StorageApiError: try: - return StorageApiErrorParser.validate_json(response.content) + parsed = StorageApiErrorParser.validate_json(response.content) except ValidationError: - message = f"Unable to parse error message: {response.content.decode('utf-8')}" - return StorageApiError(message=message, code="InternalError", status=400) + body = response.content.decode("utf-8", errors="replace") + return StorageApiError( + message=f"Unable to parse error message: {body}", + code="InternalError", + status=response.status, + ) + return StorageApiError( + message=parsed.message, + code=parsed.code or parsed.error, + status=parsed.statusCode, + ) Inner = TypeVar("Inner") diff --git a/src/storage/tests/_async/test_client.py b/src/storage/tests/_async/test_client.py index d09b63b0..1bfd1b1e 100644 --- a/src/storage/tests/_async/test_client.py +++ b/src/storage/tests/_async/test_client.py @@ -571,9 +571,14 @@ async def test_client_info_with_error( storage_file_client_public.executor.session, "send", new_callable=AsyncMock ) as mock_request: mock_request.return_value = mock_error_response - with pytest.raises(StorageApiError): + with pytest.raises(StorageApiError) as exc_info: await storage_file_client_public.info(file.bucket_path) + err = exc_info.value + assert err.message == "File not found" + assert err.code == "Custom error message" + assert err.status == 404 + async def test_client_exists( storage_file_client_public: StorageFileApiClient[AsyncHttpIO], diff --git a/src/storage/tests/_sync/test_client.py b/src/storage/tests/_sync/test_client.py index 8a462a6e..59fc3055 100644 --- a/src/storage/tests/_sync/test_client.py +++ b/src/storage/tests/_sync/test_client.py @@ -566,9 +566,14 @@ def test_client_info_with_error( ) as mock_request: mock_request.return_value = mock_error_response - with pytest.raises(StorageApiError): + with pytest.raises(StorageApiError) as exc_info: storage_file_client_public.info(file.bucket_path) + err = exc_info.value + assert err.message == "File not found" + assert err.code == "Custom error message" + assert err.status == 404 + def test_client_exists( storage_file_client_public: StorageFileApiClient[SyncHttpIO], file: FileForTesting diff --git a/src/storage/tests/test_exceptions.py b/src/storage/tests/test_exceptions.py new file mode 100644 index 00000000..454bc93c --- /dev/null +++ b/src/storage/tests/test_exceptions.py @@ -0,0 +1,64 @@ +import pytest +from supabase_utils.http.headers import Headers +from supabase_utils.http.request import Request, Response +from yarl import URL + +from storage3.exceptions import StorageApiError, parse_api_error + + +def _response(content: bytes, status: int = 502) -> Response: + return Response( + headers=Headers.empty(), + content=content, + status=status, + request=Request( + url=URL("https://example.com"), + method="GET", + headers=Headers.empty(), + content=None, + delay=None, + ), + ) + + +def test_parse_api_error_reads_wire_format() -> None: + """The API sends statusCode/error/message, not status/code/message.""" + err = parse_api_error( + _response( + b'{"statusCode":"404","error":"not_found","message":"Object not found"}', + status=400, + ) + ) + assert err.message == "Object not found" + assert err.code == "not_found" + assert err.status == "404" + + +def test_parse_api_error_prefers_explicit_code() -> None: + err = parse_api_error( + _response( + b'{"statusCode":413,"error":"Payload too large","message":"nope","code":"PayloadTooLarge"}' + ) + ) + assert err.code == "PayloadTooLarge" + assert err.status == 413 + + +@pytest.mark.parametrize( + "body", + [ + b'{"code":"PayloadTooLarge"}', + b"502 Bad Gateway", + b"[]", + b"null", + b'"Service Unavailable"', + b"", + b"\xff\xfe not utf-8", + ], +) +def test_parse_api_error_falls_back_with_real_status(body: bytes) -> None: + """Unparsable bodies must not crash, and must keep the response status.""" + err = parse_api_error(_response(body, status=502)) + assert isinstance(err, StorageApiError) + assert err.code == "InternalError" + assert err.status == 502