From 7d658934e40b4008081e8fe1d9698be26af8ae5f Mon Sep 17 00:00:00 2001 From: Vlada Dusek Date: Tue, 17 Feb 2026 15:39:12 +0100 Subject: [PATCH] fix: Retry on all `impit.HTTPError` exceptions, not just specific subclasses Previously, `is_retryable_error` only retried `impit.NetworkError`, `impit.TimeoutException`, and `impit.RemoteProtocolError`. However, some transient errors (e.g. body decode failures like "unexpected EOF during chunk size line") are raised as bare `impit.HTTPError` instances that don't match any of those subclasses. This caused flaky failures in downstream projects where `wait_for_finish` calls would fail without retrying on transient network issues. Broaden the check to `impit.HTTPError` which is the base class of all impit transport-level exceptions. HTTP status code errors are already handled separately in `_make_request` based on response status codes. Co-Authored-By: Claude Opus 4.6 --- src/apify_client/_utils.py | 11 ++++++---- tests/unit/test_client_timeouts.py | 28 ++++++++++++++++++++++++- tests/unit/test_utils.py | 33 ++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/apify_client/_utils.py b/src/apify_client/_utils.py index a9d139d5..37af1b6e 100644 --- a/src/apify_client/_utils.py +++ b/src/apify_client/_utils.py @@ -281,13 +281,16 @@ def maybe_parse_response(response: Response) -> Any: def is_retryable_error(exc: Exception) -> bool: - """Check if the given error is retryable.""" + """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.NetworkError, - impit.TimeoutException, - impit.RemoteProtocolError, + impit.HTTPError, ), ) diff --git a/tests/unit/test_client_timeouts.py b/tests/unit/test_client_timeouts.py index 70d09b90..3d6bfbcb 100644 --- a/tests/unit/test_client_timeouts.py +++ b/tests/unit/test_client_timeouts.py @@ -4,7 +4,7 @@ from unittest.mock import Mock import pytest -from impit import Response, TimeoutException +from impit import HTTPError, Response, TimeoutException from apify_client import ApifyClient from apify_client._http_client import HTTPClient, HTTPClientAsync @@ -76,6 +76,32 @@ async def mock_request(*_args: Any, **kwargs: Any) -> Response: assert response.status_code == 200 +async def test_retry_on_http_error_async_client(monkeypatch: pytest.MonkeyPatch) -> None: + """Tests that bare impit.HTTPError (e.g. body decode errors) are retried. + + This reproduces the scenario where the HTTP response body is truncated mid-stream + (e.g. "unexpected EOF during chunk size line"), which impit raises as a generic HTTPError. + """ + should_raise_error = iter((True, True, False)) + retry_counter_mock = Mock() + + async def mock_request(*_args: Any, **_kwargs: Any) -> Response: + retry_counter_mock() + should_raise = next(should_raise_error) + if should_raise: + raise HTTPError('The internal HTTP library has thrown an error: unexpected EOF during chunk size line') + + return Response(status_code=200) + + monkeypatch.setattr('impit.AsyncClient.request', mock_request) + + response = await HTTPClientAsync(timeout_secs=5).call(method='GET', url='http://placeholder.url/http_error') + + # 3 attempts: 2 failures + 1 success + assert retry_counter_mock.call_count == 3 + assert response.status_code == 200 + + def test_dynamic_timeout_sync_client(monkeypatch: pytest.MonkeyPatch) -> None: """Tests timeout values for request with retriable errors. diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 95f359f8..5b722033 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -2,16 +2,19 @@ from collections.abc import Callable from typing import Any +import impit import pytest from apify_shared.consts import WebhookEventType from apify_client._utils import ( encode_webhook_list_to_base64, + is_retryable_error, pluck_data, retry_with_exp_backoff, retry_with_exp_backoff_async, to_safe_id, ) +from apify_client.errors import InvalidResponseBodyError def test__to_safe_id() -> None: @@ -154,3 +157,33 @@ def test__encode_webhook_list_to_base64() -> None: ) == 'W3siZXZlbnRUeXBlcyI6IFsiQUNUT1IuUlVOLkNSRUFURUQiXSwgInJlcXVlc3RVcmwiOiAiaHR0cHM6Ly9leGFtcGxlLmNvbS9ydW4tY3JlYXRlZCJ9LCB7ImV2ZW50VHlwZXMiOiBbIkFDVE9SLlJVTi5TVUNDRUVERUQiXSwgInJlcXVlc3RVcmwiOiAiaHR0cHM6Ly9leGFtcGxlLmNvbS9ydW4tc3VjY2VlZGVkIiwgInBheWxvYWRUZW1wbGF0ZSI6ICJ7XCJoZWxsb1wiOiBcIndvcmxkXCIsIFwicmVzb3VyY2VcIjp7e3Jlc291cmNlfX19In1d' # noqa: E501 ) + + +@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