Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/apify_client/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
import time
import warnings
from base64 import urlsafe_b64encode
from http import HTTPStatus
from typing import TYPE_CHECKING, Any, Literal, TypeVar, overload

import impit

from apify_client._consts import OVERRIDABLE_DEFAULT_HEADERS
from apify_client.errors import InvalidResponseBodyError
from apify_client.errors import InvalidResponseBodyError, NotFoundError

if TYPE_CHECKING:
from datetime import timedelta
Expand Down Expand Up @@ -63,9 +62,7 @@ def catch_not_found_or_throw(exc: ApifyApiError) -> None:
Raises:
ApifyApiError: If the error is not a 404 Not Found error.
"""
is_not_found_status = exc.status_code == HTTPStatus.NOT_FOUND
is_not_found_type = exc.type in ['record-not-found', 'record-or-token-not-found']
if not (is_not_found_status and is_not_found_type):
if not isinstance(exc, NotFoundError):

@Pijukatel Pijukatel Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this (but the previous variant had the same issues.)

The problem of swallowing 404s is that you lose the information about what was not found. For example

dataset_1 = client.run(run_id="RUN DOES NOT EXIST").dataset().get() # Run does not exist
dataset_2 = client.run(run_id="RUN EXISTS").dataset().get() # Dataset does not exist

In both cases it returns None and user has no idea which of the cases happened.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably just swallow 404 related to the last resource in the chain and throw all the others?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this up.

We should probably just swallow 404 related to the last resource in the chain and throw all the others?

I implemented it this way. It's certainly better, but I'm not sure in terms of consistency - now for the get we for some cases return None and for some other raise exception, hmm?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the last commit, merge it as it is, and do this in a dedicated one if there is a consensus.

raise exc


Expand Down
117 changes: 98 additions & 19 deletions src/apify_client/errors.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
from __future__ import annotations

from typing import TYPE_CHECKING
from http import HTTPStatus
from typing import TYPE_CHECKING, Any

from apify_client._docs import docs_group

if TYPE_CHECKING:
from typing import Self

from apify_client._http_clients import HttpResponse


Expand All @@ -26,6 +29,15 @@ class ApifyApiError(ApifyClientError):
are retried automatically before this error is raised, while client errors (HTTP 4xx) are raised
immediately.

Instantiating `ApifyApiError` directly dispatches to a more specific subclass based on the
HTTP status code of the response (e.g. a 404 response produces a `NotFoundError`). Existing
`except ApifyApiError` handlers continue to match because every subclass inherits from this
class. Statuses without a dedicated subclass fall back to `ApifyApiError` itself.

The `type`, `message` and `data` fields from the API response body are exposed as attributes
for inspection, but they are treated as non-authoritative metadata — dispatch is driven by the
status code only, which is more stable than the per-endpoint `error.type` strings.

Attributes:
message: The error message from the API response.
type: The error type identifier from the API response (e.g. `record-not-found`).
Expand All @@ -35,6 +47,18 @@ class ApifyApiError(ApifyClientError):
data: Additional error data from the API response.
"""

def __new__(cls, response: HttpResponse, attempt: int, method: str = 'GET') -> Self: # noqa: ARG004
"""Dispatch to the subclass matching the response's HTTP status code, if any."""
target_cls: type[ApifyApiError] = cls
if cls is ApifyApiError:
status = response.status_code
mapped = _STATUS_TO_CLASS.get(status)
if mapped is None and status >= HTTPStatus.INTERNAL_SERVER_ERROR:
mapped = ServerError
if mapped is not None:
target_cls = mapped
return super().__new__(target_cls)
Comment thread
vdusek marked this conversation as resolved.

def __init__(self, response: HttpResponse, attempt: int, method: str = 'GET') -> None:
"""Initialize the API error from a failed response.

Expand All @@ -43,27 +67,17 @@ def __init__(self, response: HttpResponse, attempt: int, method: str = 'GET') ->
attempt: The attempt number when the request failed (1-indexed).
method: The HTTP method of the failed request.
"""
self.message: str | None = None
payload = _extract_error_payload(response)

self.message: str | None = f'Unexpected error: {response.text}'
self.type: str | None = None
self.data = dict[str, str]()
self.message = f'Unexpected error: {response.text}'

try:
response_data = response.json()

if (
isinstance(response_data, dict)
and 'error' in response_data
and isinstance(response_data['error'], dict)
):
self.message = response_data['error']['message']
self.type = response_data['error']['type']

if 'data' in response_data['error']:
self.data = response_data['error']['data']

except ValueError:
pass
if payload is not None:
self.message = payload['message']
self.type = payload['type']
if 'data' in payload:
self.data = payload['data']

Comment thread
vdusek marked this conversation as resolved.
Outdated
super().__init__(self.message)

Expand All @@ -73,6 +87,71 @@ def __init__(self, response: HttpResponse, attempt: int, method: str = 'GET') ->
self.http_method = method


def _extract_error_payload(response: HttpResponse) -> dict[str, Any] | None:
"""Return the `error` dict from the response body, or None if absent or unparsable."""
try:
data = response.json()
except ValueError:
return None
if not isinstance(data, dict):
return None
error = data.get('error')
return error if isinstance(error, dict) else None


@docs_group('Errors')
class InvalidRequestError(ApifyApiError):
"""Raised when the Apify API returns an HTTP 400 Bad Request response."""


@docs_group('Errors')
class UnauthorizedError(ApifyApiError):
"""Raised when the Apify API returns an HTTP 401 Unauthorized response."""


@docs_group('Errors')
class ForbiddenError(ApifyApiError):
"""Raised when the Apify API returns an HTTP 403 Forbidden response."""


@docs_group('Errors')
class NotFoundError(ApifyApiError):
"""Raised when the Apify API returns an HTTP 404 Not Found response."""


@docs_group('Errors')
class ConflictError(ApifyApiError):
"""Raised when the Apify API returns an HTTP 409 Conflict response."""


@docs_group('Errors')
class RateLimitError(ApifyApiError):
"""Raised when the Apify API returns an HTTP 429 Too Many Requests response.

Rate-limited requests are retried automatically; this error is only raised after all
retry attempts have been exhausted.
"""


@docs_group('Errors')
class ServerError(ApifyApiError):
"""Raised when the Apify API returns an HTTP 5xx response.

Server errors are retried automatically; this error is only raised after all
retry attempts have been exhausted.
"""


_STATUS_TO_CLASS: dict[int, type[ApifyApiError]] = {
400: InvalidRequestError,
401: UnauthorizedError,
403: ForbiddenError,
404: NotFoundError,
409: ConflictError,
429: RateLimitError,
}


@docs_group('Errors')
class InvalidResponseBodyError(ApifyClientError):
"""Error raised when a response body cannot be parsed.
Expand Down
72 changes: 71 additions & 1 deletion tests/unit/test_client_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from werkzeug import Response

from apify_client._http_clients import ImpitHttpClient, ImpitHttpClientAsync
from apify_client.errors import ApifyApiError
from apify_client.errors import ApifyApiError, ForbiddenError, NotFoundError, ServerError

if TYPE_CHECKING:
from pytest_httpserver import HTTPServer
Expand Down Expand Up @@ -103,3 +103,73 @@ async def test_async_client_apify_api_error_streamed(httpserver: HTTPServer) ->

assert exc.value.message == error['error']['message']
assert exc.value.type == error['error']['type']


def test_apify_api_error_dispatches_to_subclass_for_known_status(httpserver: HTTPServer) -> None:
"""Mapped HTTP status codes dispatch to their matching subclass."""
httpserver.expect_request('/dispatch').respond_with_json(
{'error': {'type': 'record-not-found', 'message': 'nope'}}, status=404
)
client = ImpitHttpClient()

with pytest.raises(NotFoundError) as exc:
client.call(method='GET', url=str(httpserver.url_for('/dispatch')))

# Still an ApifyApiError, so legacy `except` handlers keep working.
assert isinstance(exc.value, ApifyApiError)
assert exc.value.status_code == 404
assert exc.value.type == 'record-not-found'


def test_apify_api_error_dispatches_streamed_response(httpserver: HTTPServer) -> None:
"""Dispatch works even when the response body comes in as a stream (403 → ForbiddenError)."""
httpserver.expect_request('/stream_dispatch').respond_with_handler(streaming_handler)
client = ImpitHttpClient()

with pytest.raises(ForbiddenError) as exc:
client.call(method='GET', url=httpserver.url_for('/stream_dispatch'), stream=True)

assert isinstance(exc.value, ApifyApiError)
assert exc.value.status_code == 403
assert exc.value.type == 'insufficient-permissions'


def test_apify_api_error_dispatches_5xx_to_server_error(httpserver: HTTPServer) -> None:
"""Any 5xx status falls under the ServerError subclass."""
httpserver.expect_request('/server_error').respond_with_json(
{'error': {'type': 'internal-error', 'message': 'boom'}}, status=503
)
client = ImpitHttpClient(max_retries=1)

with pytest.raises(ServerError) as exc:
client.call(method='GET', url=str(httpserver.url_for('/server_error')))

assert isinstance(exc.value, ApifyApiError)
assert exc.value.status_code == 503


def test_apify_api_error_falls_back_for_unmapped_status(httpserver: HTTPServer) -> None:
"""Statuses without a dedicated subclass fall back to the base ApifyApiError."""
httpserver.expect_request('/unmapped').respond_with_json(
{'error': {'type': 'whatever', 'message': 'nope'}}, status=418
)
client = ImpitHttpClient()

with pytest.raises(ApifyApiError) as exc:
client.call(method='GET', url=str(httpserver.url_for('/unmapped')))

assert type(exc.value) is ApifyApiError
assert exc.value.status_code == 418
assert exc.value.type == 'whatever'


def test_apify_api_error_falls_back_for_unparsable_body(httpserver: HTTPServer) -> None:
"""When the body can't be parsed, status-based dispatch still applies and `.type` is None."""
httpserver.expect_request('/unparsable').respond_with_data('<not json>', status=418, content_type='text/html')
client = ImpitHttpClient(max_retries=1)

with pytest.raises(ApifyApiError) as exc:
client.call(method='GET', url=str(httpserver.url_for('/unparsable')))

assert type(exc.value) is ApifyApiError
assert exc.value.type is None
5 changes: 3 additions & 2 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,19 @@ def test__is_not_retryable_error(exc: Exception) -> None:
[
pytest.param(HTTPStatus.NOT_FOUND, 'record-not-found', True, id='404 record-not-found'),
pytest.param(HTTPStatus.NOT_FOUND, 'record-or-token-not-found', True, id='404 token-not-found'),
pytest.param(HTTPStatus.NOT_FOUND, 'some-other-error', False, id='404 other error type'),
pytest.param(HTTPStatus.NOT_FOUND, 'some-other-error', True, id='404 other error type'),
pytest.param(HTTPStatus.BAD_REQUEST, 'record-not-found', False, id='400 record-not-found'),
pytest.param(HTTPStatus.INTERNAL_SERVER_ERROR, 'record-not-found', False, id='500 record-not-found'),
],
)
def test_catch_not_found_or_throw(status_code: HTTPStatus, error_type: str, *, should_suppress: bool) -> None:
"""Test that catch_not_found_or_throw suppresses 404 errors correctly."""
mock_response = Mock()
mock_response.status_code = status_code
mock_response.json.return_value = {'error': {'type': error_type, 'message': 'msg'}}
mock_response.text = f'{{"error":{{"type":"{error_type}"}}}}'

error = ApifyApiError(mock_response, 1)
error.type = error_type

if should_suppress:
catch_not_found_or_throw(error)
Expand Down