fix: Fail fast on transport errors a retry cannot fix

src/apify_client/_utils/errors.py

@@ -2,9 +2,7 @@
 
 from typing import TYPE_CHECKING
 
-import impit
-
-from apify_client.errors import InvalidResponseBodyError, NotFoundError
+from apify_client.errors import NotFoundError
 
 if TYPE_CHECKING:
     from apify_client.errors import ApifyApiError
@@ -33,19 +31,3 @@ def catch_not_found_for_resource_or_throw(exc: ApifyApiError, resource_id: str |
     if resource_id is None:
         raise exc
     catch_not_found_or_throw(exc)
-
-
-def is_retryable_error(exc: Exception) -> bool:
-    """Check if the given error is retryable.
-
-    All `impit.HTTPError` subclasses are considered retryable because they represent transport-level failures
-    (network issues, timeouts, protocol errors, body decoding errors) that are typically transient. HTTP status
-    code errors are handled separately in `_make_request` based on the response status code, not here.
-    """
-    return isinstance(
-        exc,
-        (
-            InvalidResponseBodyError,
-            impit.HTTPError,
-        ),
-    )

src/apify_client/errors.py

@@ -137,9 +137,9 @@ class ServerError(ApifyApiError):
 class InvalidResponseBodyError(ApifyClientError):
     """Error raised when a response body cannot be parsed.
 
-    This typically occurs when the API returns a partial or malformed JSON response, for example due to a network
-    interruption. The client retries such requests automatically, so this error is only raised after all retry
-    attempts have been exhausted.
+    This occurs when the API returns a body that does not match its content type, for example a malformed JSON
+    document. It is raised on a response the client already accepted, so it is not retried - a transfer that breaks
+    mid-body surfaces as a transport error inside the retry loop instead.
     """
 
     def __init__(self, response: HttpResponse) -> None:

src/apify_client/http_clients/_impit.py

@@ -21,7 +21,7 @@
 from apify_client._docs import docs_group
 from apify_client._logging import log_context, logger_name
 from apify_client._utils.time import to_seconds
-from apify_client.errors import ApifyApiError, InvalidResponseBodyError
+from apify_client.errors import ApifyApiError
 from apify_client.http_clients._base import HttpClient, HttpClientAsync
 
 if TYPE_CHECKING:
@@ -37,20 +37,31 @@
 logger = logging.getLogger(logger_name)
 
 
+_PERMANENT_ERRORS = (
+    # A request Impit rejects before sending it, e.g. one carrying an invalid header value.
+    impit.LocalProtocolError,
+    # The class Impit declares for a scheme it refuses to speak, but does not raise today - it reports an unsupported
+    # scheme as `impit.InvalidURL`, which sits outside the `impit.HTTPError` tree and is non-retryable anyway. Listed
+    # so the classifier stays right if Impit switches over.
+    impit.UnsupportedProtocol,
+    # An over-long redirect chain is a routing loop, which repeating the request cannot break.
+    impit.TooManyRedirects,
+    # Only `Response.raise_for_status()` raises this, and the client never calls it - `_make_request` decides on
+    # status codes from the response itself.
+    impit.HTTPStatusError,
+)
+
+
 def _is_retryable_error(exc: Exception) -> bool:
-    """Check if an exception represents a transient error that should be retried.
+    """Check if an exception represents a transient transport failure that should be retried.
 
-    All `impit.HTTPError` subclasses are considered retryable because they represent transport-level failures
-    (network issues, timeouts, protocol errors, body decoding errors) that are typically transient. HTTP status
-    code errors are handled separately in `_make_request` based on the response status code, not here.
+    Every error from Impit's own hierarchy counts as transient except the permanently-failing types listed in
+    `_PERMANENT_ERRORS`. Retrying is the default because Impit also reports genuinely transient failures through
+    its generic base class, e.g. a bare `impit.HTTPError` wrapping a failure its internal HTTP library did not
+    classify. HTTP status code errors are handled separately in `_make_request` based on the response status code,
+    not here.
     """
-    return isinstance(
-        exc,
-        (
-            InvalidResponseBodyError,
-            impit.HTTPError,
-        ),
-    )
+    return isinstance(exc, impit.HTTPError) and not isinstance(exc, _PERMANENT_ERRORS)
 
 
 @docs_group('HTTP clients')

tests/unit/test_http_clients.py

@@ -309,18 +309,82 @@ def test_parse_params_mixed() -> None:
     }
 
 
-def test_is_retryable_error() -> None:
-    """Test _is_retryable_error correctly identifies retryable errors."""
-    mock_response = Mock()
-    assert _is_retryable_error(InvalidResponseBodyError(mock_response))
-    assert _is_retryable_error(impit.NetworkError('test'))
-    assert _is_retryable_error(impit.TimeoutException('test'))
-    assert _is_retryable_error(impit.RemoteProtocolError('test'))
-
-    # Non-retryable errors
-    assert not _is_retryable_error(ValueError('test'))
-    assert not _is_retryable_error(RuntimeError('test'))
-    assert not _is_retryable_error(Exception('test'))
+@pytest.mark.parametrize(
+    'exc',
+    [
+        # Impit wraps a failure its internal HTTP library did not classify in a bare `HTTPError`, so even the
+        # generic base class is transient.
+        pytest.param(impit.HTTPError('unclassified failure'), id='bare HTTPError'),
+        pytest.param(impit.TimeoutException('timeout'), id='TimeoutException'),
+        pytest.param(impit.NetworkError('network error'), id='NetworkError'),
+        pytest.param(impit.RemoteProtocolError('remote protocol error'), id='RemoteProtocolError'),
+        pytest.param(impit.DecodingError('decoding error'), id='DecodingError'),
+        # One `ProxyError` covers both a proxy rejecting the CONNECT tunnel and a 407, so a transient case cannot
+        # be told from a permanent one - retrying is the safer default.
+        pytest.param(impit.ProxyError('proxy error'), id='ProxyError'),
+    ],
+)
+def test_is_retryable_error(exc: Exception) -> None:
+    """A transient transport failure is retried."""
+    assert _is_retryable_error(exc)
+
+
+@pytest.mark.parametrize(
+    'exc',
+    [
+        pytest.param(impit.LocalProtocolError('invalid header value'), id='LocalProtocolError'),
+        pytest.param(impit.UnsupportedProtocol('unsupported scheme'), id='UnsupportedProtocol'),
+        pytest.param(impit.TooManyRedirects('too many redirects'), id='TooManyRedirects'),
+        pytest.param(impit.HTTPStatusError('status error'), id='HTTPStatusError'),
+        # Impit reports a bad URL outside the `impit.HTTPError` tree entirely.
+        pytest.param(impit.InvalidURL('unsupported scheme'), id='InvalidURL'),
+        # `InvalidResponseBodyError` is raised by a resource client once `call` has returned, never in the retry loop.
+        pytest.param(InvalidResponseBodyError(Mock()), id='InvalidResponseBodyError'),
+        pytest.param(ValueError('value error'), id='ValueError'),
+        pytest.param(RuntimeError('runtime error'), id='RuntimeError'),
+        pytest.param(Exception('generic exception'), id='Exception'),
+    ],
+)
+def test_is_not_retryable_error(exc: Exception) -> None:
+    """A transport failure a retry cannot fix, and anything outside Impit's hierarchy, is not retried."""
+    assert not _is_retryable_error(exc)
+
+
+def test_permanent_transport_error_is_not_retried() -> None:
+    """A transport error a retry cannot fix fails on the first attempt instead of burning the whole backoff."""
+    client = ImpitHttpClient(token='test_token', min_delay_between_retries=timedelta(0))
+    request = Mock(side_effect=impit.LocalProtocolError('invalid header value'))
+    client._impit_client = Mock(request=request)
+
+    with pytest.raises(impit.LocalProtocolError):
+        client.call(method='GET', url='https://api.test.com/endpoint')
+
+    request.assert_called_once()
+
+
+async def test_permanent_transport_error_is_not_retried_async() -> None:
+    """The async client applies the same policy, failing on the first attempt."""
+    client = ImpitHttpClientAsync(token='test_token', min_delay_between_retries=timedelta(0))
+    request = AsyncMock(side_effect=impit.LocalProtocolError('invalid header value'))
+    client._impit_async_client = Mock(request=request)
+
+    with pytest.raises(impit.LocalProtocolError):
+        await client.call(method='GET', url='https://api.test.com/endpoint')
+
+    request.assert_awaited_once()
+
+
+def test_transient_transport_error_is_retried() -> None:
+    """A transient transport failure keeps being retried until the attempts run out."""
+    client = ImpitHttpClient(token='test_token', max_retries=2, min_delay_between_retries=timedelta(0))
+    request = Mock(side_effect=impit.TimeoutException('timeout'))
+    client._impit_client = Mock(request=request)
+
+    with pytest.raises(impit.TimeoutException):
+        client.call(method='GET', url='https://api.test.com/endpoint')
+
+    # `max_retries` attempts inside the backoff loop, plus the final one it makes after the last delay.
+    assert request.call_count == 3
 
 
 @pytest.fixture(

tests/unit/test_utils.py

@@ -11,22 +11,21 @@
 from typing import TYPE_CHECKING, Any
 from unittest.mock import Mock
 
-import impit
 import pytest
 
 from apify_client._models import WebhookCondition, WebhookCreate
 from apify_client._resource_clients._resource_client import ResourceClientBase
 from apify_client._utils.crypto import create_hmac_signature, create_storage_content_signature, encode_base62
 from apify_client._utils.encoding import encode_key_value_store_record_value, encode_webhooks_to_base64
-from apify_client._utils.errors import catch_not_found_or_throw, is_retryable_error
+from apify_client._utils.errors import catch_not_found_or_throw
 from apify_client._utils.http import (
     is_compressible_content_type,
     response_to_dict,
     response_to_list,
     to_safe_id,
 )
 from apify_client._utils.try_import import FailedImport, try_import
-from apify_client.errors import ApifyApiError, InvalidResponseBodyError
+from apify_client.errors import ApifyApiError
 
 if TYPE_CHECKING:
     from apify_client._typeddicts import WebhookRepresentationDict
@@ -187,36 +186,6 @@ def test_encode_webhooks_to_base64_from_dicts() -> None:
     assert result == result_from_models
 
 
-@pytest.mark.parametrize(
-    'exc',
-    [
-        InvalidResponseBodyError(impit.Response(status_code=200)),
-        impit.HTTPError('generic http error'),
-        impit.NetworkError('network error'),
-        impit.TimeoutException('timeout'),
-        impit.RemoteProtocolError('remote protocol error'),
-        impit.ReadError('read error'),
-        impit.ConnectError('connect error'),
-        impit.WriteError('write error'),
-        impit.DecodingError('decoding error'),
-    ],
-)
-def test__is_retryable_error(exc: Exception) -> None:
-    assert is_retryable_error(exc) is True
-
-
-@pytest.mark.parametrize(
-    'exc',
-    [
-        Exception('generic exception'),
-        ValueError('value error'),
-        RuntimeError('runtime error'),
-    ],
-)
-def test__is_not_retryable_error(exc: Exception) -> None:
-    assert is_retryable_error(exc) is False
-
-
 @pytest.mark.parametrize(
     ('status_code', 'error_type', 'should_suppress'),
     [