From 0bbb2f043de5c3e221a596b20dadd010b5177ebc Mon Sep 17 00:00:00 2001 From: Leonardo Santiago Date: Fri, 16 May 2025 09:44:27 -0300 Subject: [PATCH 1/3] chore(API): validate JSON input for APIError.__init__() directly passing `r.json()` into APIError.__init__() is incorrect, as it expects a `Dict[str, str]`. instead, it should first validate that the json object is in fact the correct schema, by using a pydantic model --- postgrest/_async/request_builder.py | 12 +++++------- postgrest/exceptions.py | 15 +++++++++++++++ tests/_async/test_client.py | 27 +++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/postgrest/_async/request_builder.py b/postgrest/_async/request_builder.py index 2892fa36..c58e3697 100644 --- a/postgrest/_async/request_builder.py +++ b/postgrest/_async/request_builder.py @@ -19,7 +19,7 @@ pre_update, pre_upsert, ) -from ..exceptions import APIError, generate_default_error_message +from ..exceptions import APIError, APIErrorFromJSON, generate_default_error_message from ..types import ReturnMethod from ..utils import AsyncClient, get_origin_and_cast @@ -75,10 +75,9 @@ async def execute(self) -> APIResponse[_ReturnT]: return body return APIResponse[_ReturnT].from_http_request_response(r) else: - raise APIError(r.json()) + json_obj = APIErrorFromJSON.model_validate_json(r.content) + raise APIError(dict(json_obj)) except ValidationError as e: - raise APIError(r.json()) from e - except JSONDecodeError: raise APIError(generate_default_error_message(r)) @@ -124,10 +123,9 @@ async def execute(self) -> SingleAPIResponse[_ReturnT]: ): # Response.ok from JS (https://developer.mozilla.org/en-US/docs/Web/API/Response/ok) return SingleAPIResponse[_ReturnT].from_http_request_response(r) else: - raise APIError(r.json()) + json_obj = APIErrorFromJSON.model_validate_json(r.content) + raise APIError(dict(json_obj)) except ValidationError as e: - raise APIError(r.json()) from e - except JSONDecodeError: raise APIError(generate_default_error_message(r)) diff --git a/postgrest/exceptions.py b/postgrest/exceptions.py index 303c5705..4207aad4 100644 --- a/postgrest/exceptions.py +++ b/postgrest/exceptions.py @@ -1,6 +1,21 @@ from typing import Dict, Optional +from pydantic import BaseModel +class APIErrorFromJSON(BaseModel): + """ + A pydantic object to validate an error info object + from a json string. + """ + message: Optional[str] + """The error message.""" + code: Optional[str] + """The error code.""" + hint: Optional[str] + """The error hint.""" + details: Optional[str] + """The error details.""" + class APIError(Exception): """ Base exception for all API errors. diff --git a/tests/_async/test_client.py b/tests/_async/test_client.py index fb7881c5..411ffe11 100644 --- a/tests/_async/test_client.py +++ b/tests/_async/test_client.py @@ -1,7 +1,7 @@ from unittest.mock import patch import pytest -from httpx import BasicAuth, Headers +from httpx import BasicAuth, Headers, Response, Request from postgrest import AsyncPostgrestClient from postgrest.exceptions import APIError @@ -107,7 +107,6 @@ async def test_response_status_code_outside_ok(postgrest_client: AsyncPostgrestC ) assert exc_response["errors"][0].get("code") == 400 - @pytest.mark.asyncio async def test_response_maybe_single(postgrest_client: AsyncPostgrestClient): with patch( @@ -127,3 +126,27 @@ async def test_response_maybe_single(postgrest_client: AsyncPostgrestClient): exc_response = exc_info.value.json() assert isinstance(exc_response.get("message"), str) assert "code" in exc_response and int(exc_response["code"]) == 204 + +# https://github.com/supabase/postgrest-py/issues/595 +@pytest.mark.asyncio +async def test_response_client_invalid_response_but_valid_json(postgrest_client: AsyncPostgrestClient): + with patch( + "httpx._client.AsyncClient.request", + return_value=Response( + status_code=502, + text='"gateway error: Error: Network connection lost."', # quotes makes this text a valid non-dict JSON object + request=Request(method="GET", url="http://example.com") + ) + ): + client = ( + postgrest_client.from_("test").select("a", "b").eq("c", "d").single() + ) + assert "Accept" in client.headers + assert client.headers.get("Accept") == "application/vnd.pgrst.object+json" + with pytest.raises(APIError) as exc_info: + await client.execute() + assert isinstance(exc_info, pytest.ExceptionInfo) + exc_response = exc_info.value.json() + assert isinstance(exc_response.get("message"), str) + assert exc_response.get("message") == "JSON could not be generated" + assert "code" in exc_response and int(exc_response["code"]) == 502 From 72b09a0e0f88091c1f7062faaef7d325113de740 Mon Sep 17 00:00:00 2001 From: Andrew Smith Date: Sat, 17 May 2025 00:05:31 +0100 Subject: [PATCH 2/3] chore(format): fix formatting of code --- postgrest/_async/request_builder.py | 1 - postgrest/_sync/request_builder.py | 23 ++++++++++------------- postgrest/exceptions.py | 3 +++ tests/_async/test_client.py | 18 ++++++++++-------- tests/_sync/test_client.py | 28 +++++++++++++++++++++++++++- 5 files changed, 50 insertions(+), 23 deletions(-) diff --git a/postgrest/_async/request_builder.py b/postgrest/_async/request_builder.py index c58e3697..fa5b856f 100644 --- a/postgrest/_async/request_builder.py +++ b/postgrest/_async/request_builder.py @@ -1,6 +1,5 @@ from __future__ import annotations -from json import JSONDecodeError from typing import Any, Generic, Optional, TypeVar, Union from httpx import Headers, QueryParams diff --git a/postgrest/_sync/request_builder.py b/postgrest/_sync/request_builder.py index 9683b1ac..4f8af88e 100644 --- a/postgrest/_sync/request_builder.py +++ b/postgrest/_sync/request_builder.py @@ -1,6 +1,5 @@ from __future__ import annotations -from json import JSONDecodeError from typing import Any, Generic, Optional, TypeVar, Union from httpx import Headers, QueryParams @@ -19,7 +18,7 @@ pre_update, pre_upsert, ) -from ..exceptions import APIError, generate_default_error_message +from ..exceptions import APIError, APIErrorFromJSON, generate_default_error_message from ..types import ReturnMethod from ..utils import SyncClient, get_origin_and_cast @@ -75,10 +74,9 @@ def execute(self) -> APIResponse[_ReturnT]: return body return APIResponse[_ReturnT].from_http_request_response(r) else: - raise APIError(r.json()) + json_obj = APIErrorFromJSON.model_validate_json(r.content) + raise APIError(dict(json_obj)) except ValidationError as e: - raise APIError(r.json()) from e - except JSONDecodeError: raise APIError(generate_default_error_message(r)) @@ -124,10 +122,9 @@ def execute(self) -> SingleAPIResponse[_ReturnT]: ): # Response.ok from JS (https://developer.mozilla.org/en-US/docs/Web/API/Response/ok) return SingleAPIResponse[_ReturnT].from_http_request_response(r) else: - raise APIError(r.json()) + json_obj = APIErrorFromJSON.model_validate_json(r.content) + raise APIError(dict(json_obj)) except ValidationError as e: - raise APIError(r.json()) from e - except JSONDecodeError: raise APIError(generate_default_error_message(r)) @@ -290,7 +287,7 @@ def select( *columns: The names of the columns to fetch. count: The method to use to get the count of rows returned. Returns: - :class:`SyncSelectRequestBuilder` + :class:`AsyncSelectRequestBuilder` """ method, params, headers, json = pre_select(*columns, count=count, head=head) return SyncSelectRequestBuilder[_ReturnT]( @@ -317,7 +314,7 @@ def insert( Otherwise, use the default value for the column. Only applies for bulk inserts. Returns: - :class:`SyncQueryRequestBuilder` + :class:`AsyncQueryRequestBuilder` """ method, params, headers, json = pre_insert( json, @@ -353,7 +350,7 @@ def upsert( not when merging with existing rows under `ignoreDuplicates: false`. This also only applies when doing bulk upserts. Returns: - :class:`SyncQueryRequestBuilder` + :class:`AsyncQueryRequestBuilder` """ method, params, headers, json = pre_upsert( json, @@ -381,7 +378,7 @@ def update( count: The method to use to get the count of rows returned. returning: Either 'minimal' or 'representation' Returns: - :class:`SyncFilterRequestBuilder` + :class:`AsyncFilterRequestBuilder` """ method, params, headers, json = pre_update( json, @@ -404,7 +401,7 @@ def delete( count: The method to use to get the count of rows returned. returning: Either 'minimal' or 'representation' Returns: - :class:`SyncFilterRequestBuilder` + :class:`AsyncFilterRequestBuilder` """ method, params, headers, json = pre_delete( count=count, diff --git a/postgrest/exceptions.py b/postgrest/exceptions.py index 4207aad4..203153ea 100644 --- a/postgrest/exceptions.py +++ b/postgrest/exceptions.py @@ -1,4 +1,5 @@ from typing import Dict, Optional + from pydantic import BaseModel @@ -7,6 +8,7 @@ class APIErrorFromJSON(BaseModel): A pydantic object to validate an error info object from a json string. """ + message: Optional[str] """The error message.""" code: Optional[str] @@ -16,6 +18,7 @@ class APIErrorFromJSON(BaseModel): details: Optional[str] """The error details.""" + class APIError(Exception): """ Base exception for all API errors. diff --git a/tests/_async/test_client.py b/tests/_async/test_client.py index 411ffe11..fa32fcaa 100644 --- a/tests/_async/test_client.py +++ b/tests/_async/test_client.py @@ -1,7 +1,7 @@ from unittest.mock import patch import pytest -from httpx import BasicAuth, Headers, Response, Request +from httpx import BasicAuth, Headers, Request, Response from postgrest import AsyncPostgrestClient from postgrest.exceptions import APIError @@ -107,6 +107,7 @@ async def test_response_status_code_outside_ok(postgrest_client: AsyncPostgrestC ) assert exc_response["errors"][0].get("code") == 400 + @pytest.mark.asyncio async def test_response_maybe_single(postgrest_client: AsyncPostgrestClient): with patch( @@ -127,20 +128,21 @@ async def test_response_maybe_single(postgrest_client: AsyncPostgrestClient): assert isinstance(exc_response.get("message"), str) assert "code" in exc_response and int(exc_response["code"]) == 204 + # https://github.com/supabase/postgrest-py/issues/595 @pytest.mark.asyncio -async def test_response_client_invalid_response_but_valid_json(postgrest_client: AsyncPostgrestClient): +async def test_response_client_invalid_response_but_valid_json( + postgrest_client: AsyncPostgrestClient, +): with patch( "httpx._client.AsyncClient.request", return_value=Response( status_code=502, - text='"gateway error: Error: Network connection lost."', # quotes makes this text a valid non-dict JSON object - request=Request(method="GET", url="http://example.com") - ) + text='"gateway error: Error: Network connection lost."', # quotes makes this text a valid non-dict JSON object + request=Request(method="GET", url="http://example.com"), + ), ): - client = ( - postgrest_client.from_("test").select("a", "b").eq("c", "d").single() - ) + client = postgrest_client.from_("test").select("a", "b").eq("c", "d").single() assert "Accept" in client.headers assert client.headers.get("Accept") == "application/vnd.pgrst.object+json" with pytest.raises(APIError) as exc_info: diff --git a/tests/_sync/test_client.py b/tests/_sync/test_client.py index 930fb7be..4558a8a4 100644 --- a/tests/_sync/test_client.py +++ b/tests/_sync/test_client.py @@ -1,7 +1,7 @@ from unittest.mock import patch import pytest -from httpx import BasicAuth, Headers +from httpx import BasicAuth, Headers, Request, Response from postgrest import SyncPostgrestClient from postgrest.exceptions import APIError @@ -123,3 +123,29 @@ def test_response_maybe_single(postgrest_client: SyncPostgrestClient): exc_response = exc_info.value.json() assert isinstance(exc_response.get("message"), str) assert "code" in exc_response and int(exc_response["code"]) == 204 + + +# https://github.com/supabase/postgrest-py/issues/595 + + +def test_response_client_invalid_response_but_valid_json( + postgrest_client: SyncPostgrestClient, +): + with patch( + "httpx._client.SyncClient.request", + return_value=Response( + status_code=502, + text='"gateway error: Error: Network connection lost."', # quotes makes this text a valid non-dict JSON object + request=Request(method="GET", url="http://example.com"), + ), + ): + client = postgrest_client.from_("test").select("a", "b").eq("c", "d").single() + assert "Accept" in client.headers + assert client.headers.get("Accept") == "application/vnd.pgrst.object+json" + with pytest.raises(APIError) as exc_info: + client.execute() + assert isinstance(exc_info, pytest.ExceptionInfo) + exc_response = exc_info.value.json() + assert isinstance(exc_response.get("message"), str) + assert exc_response.get("message") == "JSON could not be generated" + assert "code" in exc_response and int(exc_response["code"]) == 502 From 41f6d5689737ba4416f0dcab6231982f37ed9d79 Mon Sep 17 00:00:00 2001 From: Andrew Smith Date: Sat, 17 May 2025 00:24:53 +0100 Subject: [PATCH 3/3] chore: fix make file to replace httpx syncclient with client --- Makefile | 1 + tests/_sync/test_client.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 652e0e63..fc3c5ee8 100644 --- a/Makefile +++ b/Makefile @@ -37,6 +37,7 @@ remove_pytest_asyncio_from_sync: sed -i 's/@pytest.mark.asyncio//g' tests/_sync/test_client.py sed -i 's/_async/_sync/g' tests/_sync/test_client.py sed -i 's/Async/Sync/g' tests/_sync/test_client.py + sed -i 's/_client\.SyncClient/_client\.Client/g' tests/_sync/test_client.py sleep: sleep 2 diff --git a/tests/_sync/test_client.py b/tests/_sync/test_client.py index 4558a8a4..0e70a63e 100644 --- a/tests/_sync/test_client.py +++ b/tests/_sync/test_client.py @@ -132,7 +132,7 @@ def test_response_client_invalid_response_but_valid_json( postgrest_client: SyncPostgrestClient, ): with patch( - "httpx._client.SyncClient.request", + "httpx._client.Client.request", return_value=Response( status_code=502, text='"gateway error: Error: Network connection lost."', # quotes makes this text a valid non-dict JSON object