diff --git a/src/apify_client/errors.py b/src/apify_client/errors.py index 009f9958..85b27e08 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 `(response, attempt, *, method='GET', url=None)` constructor + # signature - Python calls the dispatched subclass's `__init__` with the original arguments. - 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..c1b5b839 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 error response with an empty body names the HTTP status and the failed 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.""" @@ -293,6 +340,30 @@ def test_apify_api_error_subclass_constructed_directly_keeps_its_class() -> None assert error.type == 'record-not-found' +def test_apify_api_error_without_url_keeps_plain_message() -> None: + """An error constructed without a URL has no `path` and its text is just the message.""" + response = Mock() + response.status_code = 404 + response.json.return_value = {'error': {'type': 'record-not-found', 'message': 'nope'}} + + error = ApifyApiError(response, 1, method='GET') + + assert error.path is None + assert str(error) == 'nope' + + +def test_apify_api_error_with_empty_message_falls_back_to_status() -> None: + """An error payload with an empty `message` falls back to the HTTP status message.""" + response = Mock() + response.status_code = 400 + response.text = '{"error": {"type": "invalid-input", "message": ""}}' + response.json.return_value = {'error': {'type': 'invalid-input', 'message': ''}} + + error = ApifyApiError(response, 1, method='GET') + + assert error.message == f'Unexpected error (HTTP 400): {response.text}' + + def test_apify_api_error_falls_back_for_unparsable_body( httpserver: HTTPServer, http_client_class: type[HttpClient] ) -> None: @@ -305,6 +376,7 @@ def test_apify_api_error_falls_back_for_unparsable_body( assert type(exc.value) is ApifyApiError assert exc.value.type is None + assert exc.value.message == 'Unexpected error (HTTP 418): ' def test_get_raises_invalid_response_body_error_on_malformed_json(