From 9079ba17988445faf7e7027c4d7985eeaa4c75ad Mon Sep 17 00:00:00 2001 From: Antonio Yuen Date: Mon, 2 Mar 2026 12:40:35 -0500 Subject: [PATCH 1/2] git comment + tests --- pyiceberg/catalog/rest/response.py | 11 +- tests/catalog/test_rest_response.py | 151 ++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 tests/catalog/test_rest_response.py diff --git a/pyiceberg/catalog/rest/response.py b/pyiceberg/catalog/rest/response.py index 9c1c33dacb..5e727bc82b 100644 --- a/pyiceberg/catalog/rest/response.py +++ b/pyiceberg/catalog/rest/response.py @@ -17,6 +17,8 @@ from json import JSONDecodeError from typing import Literal +from http import HTTPStatus + from pydantic import Field, ValidationError from requests import HTTPError @@ -101,8 +103,13 @@ def _handle_non_200_response(exc: HTTPError, error_handler: dict[int, type[Excep if uri := error.error_uri: response += f" ({uri})" else: - error = ErrorResponse.model_validate_json(exc.response.text).error - response = f"{error.type}: {error.message}" + # Handle empty response bodies (Specifically HEAD requests via exist requests) + if not exc.response.text: + http_status = HTTPStatus(code) + response = f"{exception.__name__}: RestError: {http_status.phrase}" + else: + error = ErrorResponse.model_validate_json(exc.response.text).error + response = f"{error.type}: {error.message}" except JSONDecodeError: # In the case we don't have a proper response response = f"RESTError {exc.response.status_code}: Could not decode json payload: {exc.response.text}" diff --git a/tests/catalog/test_rest_response.py b/tests/catalog/test_rest_response.py new file mode 100644 index 0000000000..d78890f4ba --- /dev/null +++ b/tests/catalog/test_rest_response.py @@ -0,0 +1,151 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import json + +import pytest +from requests import HTTPError, Response + +from pyiceberg.catalog.rest.response import _handle_non_200_response +from pyiceberg.exceptions import ( + AuthorizationExpiredError, + BadRequestError, + ForbiddenError, + NoSuchTableError, + OAuthError, + RESTError, + ServerError, + ServiceUnavailableError, + TooManyRequestsError, + UnauthorizedError, +) + + +def _make_http_error(status_code: int, body: str = "", reason: str | None = None) -> HTTPError: + response = Response() + response.status_code = status_code + response._content = body.encode("utf-8") if body else b"" + if reason is not None: + response.reason = reason + return HTTPError(response=response) + + +def _error_body(message: str, error_type: str, code: int) -> str: + return json.dumps({"error": {"message": message, "type": error_type, "code": code}}) + + +@pytest.mark.parametrize( + "status_code, expected_exception", + [ + (400, BadRequestError), + (401, UnauthorizedError), + (403, ForbiddenError), + (419, AuthorizationExpiredError), + (422, RESTError), + (429, TooManyRequestsError), + (501, NotImplementedError), + (503, ServiceUnavailableError), + (500, ServerError), + (502, ServerError), + (504, ServerError), + (999, RESTError), + ], +) +def test_status_code_maps_to_exception(status_code: int, expected_exception: type[Exception]) -> None: + body = _error_body("something went wrong", "SomeError", status_code) + exc = _make_http_error(status_code, body=body) + + with pytest.raises(expected_exception, match="SomeError: something went wrong"): + _handle_non_200_response(exc, {}) + + +def test_error_handler_overrides_default_mapping() -> None: + body = _error_body("Table does not exist: ns.tbl", "NoSuchTableException", 404) + exc = _make_http_error(404, body=body) + + with pytest.raises(NoSuchTableError, match="NoSuchTableException: Table does not exist: ns.tbl"): + _handle_non_200_response(exc, {404: NoSuchTableError}) + + +@pytest.mark.parametrize( + "status_code, body, expected_exception", + [ + (500, "not json at all", ServerError), + (400, '{"unexpected": "structure"}', BadRequestError), + ], +) +def test_unparseable_body_falls_back_to_validation_error( + status_code: int, body: str, expected_exception: type[Exception] +) -> None: + exc = _make_http_error(status_code, body=body) + + with pytest.raises(expected_exception, match="Received unexpected JSON Payload"): + _handle_non_200_response(exc, {}) + + +def test_empty_body_bypasses_pydantic() -> None: + exc = _make_http_error(403, body="", reason="Forbidden") + + with pytest.raises(ForbiddenError, match="ForbiddenError: RestError: Forbidden"): + _handle_non_200_response(exc, {}) + + +def test_empty_body_falls_back_to_http_status_phrase() -> None: + exc = _make_http_error(503, body="") + exc.response.reason = None + + with pytest.raises(ServiceUnavailableError, match="ServiceUnavailableError: RestError: Service Unavailable"): + _handle_non_200_response(exc, {}) + + +def test_oauth_error_with_description() -> None: + body = json.dumps({ + "error": "invalid_client", + "error_description": "Client authentication failed", + }) + exc = _make_http_error(401, body=body) + + with pytest.raises(OAuthError, match="invalid_client: Client authentication failed"): + _handle_non_200_response(exc, {401: OAuthError}) + + +def test_oauth_error_with_uri() -> None: + body = json.dumps({ + "error": "invalid_scope", + "error_description": "scope not allowed", + "error_uri": "https://example.com/help", + }) + exc = _make_http_error(400, body=body) + + with pytest.raises(OAuthError, match=r"invalid_scope: scope not allowed \(https://example.com/help\)"): + _handle_non_200_response(exc, {400: OAuthError}) + + +def test_oauth_error_without_description() -> None: + body = json.dumps({"error": "invalid_grant"}) + exc = _make_http_error(401, body=body) + + with pytest.raises(OAuthError, match="^invalid_grant$"): + _handle_non_200_response(exc, {401: OAuthError}) + + +def test_none_response_raises_value_error() -> None: + exc = HTTPError() + exc.response = None + + with pytest.raises(ValueError, match="Did not receive a response"): + _handle_non_200_response(exc, {}) From 19c89e7f42731f01ba994951c09ea6e3f74d55d6 Mon Sep 17 00:00:00 2001 From: Antonio Yuen Date: Mon, 2 Mar 2026 12:42:57 -0500 Subject: [PATCH 2/2] linting --- pyiceberg/catalog/rest/response.py | 3 +-- tests/catalog/test_rest_response.py | 22 +++++++++++++--------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/pyiceberg/catalog/rest/response.py b/pyiceberg/catalog/rest/response.py index 5e727bc82b..a7723999df 100644 --- a/pyiceberg/catalog/rest/response.py +++ b/pyiceberg/catalog/rest/response.py @@ -14,11 +14,10 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from http import HTTPStatus from json import JSONDecodeError from typing import Literal -from http import HTTPStatus - from pydantic import Field, ValidationError from requests import HTTPError diff --git a/tests/catalog/test_rest_response.py b/tests/catalog/test_rest_response.py index d78890f4ba..09828f630a 100644 --- a/tests/catalog/test_rest_response.py +++ b/tests/catalog/test_rest_response.py @@ -113,10 +113,12 @@ def test_empty_body_falls_back_to_http_status_phrase() -> None: def test_oauth_error_with_description() -> None: - body = json.dumps({ - "error": "invalid_client", - "error_description": "Client authentication failed", - }) + body = json.dumps( + { + "error": "invalid_client", + "error_description": "Client authentication failed", + } + ) exc = _make_http_error(401, body=body) with pytest.raises(OAuthError, match="invalid_client: Client authentication failed"): @@ -124,11 +126,13 @@ def test_oauth_error_with_description() -> None: def test_oauth_error_with_uri() -> None: - body = json.dumps({ - "error": "invalid_scope", - "error_description": "scope not allowed", - "error_uri": "https://example.com/help", - }) + body = json.dumps( + { + "error": "invalid_scope", + "error_description": "scope not allowed", + "error_uri": "https://example.com/help", + } + ) exc = _make_http_error(400, body=body) with pytest.raises(OAuthError, match=r"invalid_scope: scope not allowed \(https://example.com/help\)"):