From 6e6952b7a8033485c7b7aaa57e15b39fea8e6cad Mon Sep 17 00:00:00 2001 From: Emre K <110906681+kocaemre@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:56:06 +0100 Subject: [PATCH] fix: identify failed API requests without exposing query tokens Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com> --- src/apify_client/errors.py | 20 +++++++---- src/apify_client/http_clients/_base.py | 4 +-- tests/unit/test_client_errors.py | 47 ++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/apify_client/errors.py b/src/apify_client/errors.py index 009f9958..d99ca06b 100644 --- a/src/apify_client/errors.py +++ b/src/apify_client/errors.py @@ -2,6 +2,7 @@ from http import HTTPStatus from typing import TYPE_CHECKING, Any +from urllib.parse import urlsplit from apify_client._docs import docs_group @@ -33,13 +34,14 @@ class ApifyApiError(ApifyClientError): status_code: The HTTP status code of the error response. attempt: The attempt number when the error was raised. http_method: The HTTP method of the failed request. + path: The failed request path, without query parameters or credentials (if available). data: Additional error data from the API response. """ - # Subclasses in `_STATUS_TO_CLASS` must keep the `(response, attempt, *, method='GET')` constructor signature — - # `__new__` forwards those arguments verbatim. + # Subclasses in `_STATUS_TO_CLASS` must keep the `__new__` constructor signature; + # the response, attempt, method, and url are forwarded verbatim. - def __new__(cls, response: HttpResponse, attempt: int, *, method: str = 'GET') -> Self: # noqa: ARG004 + def __new__(cls, response: HttpResponse, attempt: int, *, method: str = 'GET', url: str | None = None) -> Self: # noqa: ARG004 """Dispatch to the subclass matching the response's HTTP status code, if any.""" if cls is ApifyApiError: status = response.status_code @@ -52,27 +54,31 @@ def __new__(cls, response: HttpResponse, attempt: int, *, method: str = 'GET') - return super().__new__(mapped) return super().__new__(cls) - def __init__(self, response: HttpResponse, attempt: int, *, method: str = 'GET') -> None: + def __init__(self, response: HttpResponse, attempt: int, *, method: str = 'GET', url: str | None = None) -> None: """Initialize the API error from a failed response. Args: response: The failed HTTP response from the Apify API. attempt: The attempt number when the request failed (1-indexed). method: The HTTP method of the failed request. + url: The failed request URL, if available. """ payload = self._extract_error_payload(response) - self.message: str | None = f'Unexpected error: {response.text}' + self.message: str | None = f'Unexpected error (HTTP {response.status_code})' + if response.text: + self.message += f': {response.text}' self.type: str | None = None self.data = dict[str, str]() if payload is not None: - self.message = payload.get('message', self.message) + self.message = payload.get('message') or self.message self.type = payload.get('type') if 'data' in payload: self.data = payload['data'] - super().__init__(self.message) + self.path = urlsplit(url).path if url is not None else None + super().__init__(f'{method} {self.path}: {self.message}' if self.path is not None else self.message) self.status_code = response.status_code self.attempt = attempt diff --git a/src/apify_client/http_clients/_base.py b/src/apify_client/http_clients/_base.py index cad2efdc..378c570d 100644 --- a/src/apify_client/http_clients/_base.py +++ b/src/apify_client/http_clients/_base.py @@ -686,7 +686,7 @@ def _make_request( stop_retrying() raise - raise ApifyApiError(response, attempt, method=method) + raise ApifyApiError(response, attempt, method=method, url=url) @docs_group('HTTP clients') @@ -911,4 +911,4 @@ async def _make_request( stop_retrying() raise - raise ApifyApiError(response, attempt, method=method) + raise ApifyApiError(response, attempt, method=method, url=url) diff --git a/tests/unit/test_client_errors.py b/tests/unit/test_client_errors.py index 1536ff2d..8fb44893 100644 --- a/tests/unit/test_client_errors.py +++ b/tests/unit/test_client_errors.py @@ -121,6 +121,53 @@ async def test_async_client_apify_api_error_with_data( assert exc.value.data == _EXPECTED_DATA +def test_api_error_identifies_request_without_exposing_query_token( + httpserver: HTTPServer, http_client_class: type[HttpClient] +) -> None: + """A failed request reports its method and path but not a token in the query string.""" + httpserver.expect_request('/errors', query_string='token=private-value&limit=1').respond_with_json( + {'error': {'message': 'not found', 'type': 'record-not-found'}}, status=404 + ) + client = http_client_class() + with pytest.raises(NotFoundError) as exc: + client.call( + method='POST', url=str(httpserver.url_for('/errors')), params={'token': 'private-value', 'limit': 1} + ) + + assert exc.value.path == '/errors' + assert str(exc.value) == 'POST /errors: not found' + assert 'private-value' not in str(exc.value) + + +async def test_async_api_error_identifies_request_without_exposing_query_token( + httpserver: HTTPServer, http_client_async_class: type[HttpClientAsync] +) -> None: + """The async request path is available without leaking query credentials.""" + httpserver.expect_request('/errors', query_string='token=private-value').respond_with_json( + {'error': {'message': 'not found', 'type': 'record-not-found'}}, status=404 + ) + client = http_client_async_class() + with pytest.raises(NotFoundError) as exc: + await client.call(method='GET', url=str(httpserver.url_for('/errors')), params={'token': 'private-value'}) + + assert exc.value.path == '/errors' + assert str(exc.value) == 'GET /errors: not found' + assert 'private-value' not in str(exc.value) + + +def test_api_error_with_empty_body_names_http_status( + httpserver: HTTPServer, http_client_class: type[HttpClient] +) -> None: + """An empty proxy error still identifies the failed status and request.""" + httpserver.expect_request('/empty-error').respond_with_data(b'', status=502) + client = http_client_class(max_retries=0) + with pytest.raises(ServerError) as exc: + client.call(method='GET', url=str(httpserver.url_for('/empty-error'))) + + assert exc.value.message == 'Unexpected error (HTTP 502)' + assert str(exc.value) == 'GET /empty-error: Unexpected error (HTTP 502)' + + def test_client_apify_api_error_streamed(httpserver: HTTPServer, http_client_class: type[HttpClient]) -> None: """Test that client correctly throws ApifyApiError when the response has stream."""