Skip to content

Commit 03b0e9c

Browse files
vdusekclaude
andauthored
fix: Retry on all impit.HTTPError exceptions, not just specific subclasses (#624)
## Summary - Broadened `is_retryable_error()` to catch `impit.HTTPError` (base class) instead of only `impit.NetworkError`, `impit.TimeoutException`, and `impit.RemoteProtocolError` - Some transient errors (e.g. body decode failures like "unexpected EOF during chunk size line") are raised by `impit` as bare `HTTPError` instances that don't match any of the specific subclasses, causing requests to fail without retrying - HTTP status code errors are already handled separately in `_make_request` based on response status codes, so this change only affects transport-level failures **Triggered by**: [flaky e2e test failure in apify-sdk-python](https://github.com/apify/apify-sdk-python/actions/runs/22101349916/attempts/1) where `wait_for_finish` failed with: ``` impit.HTTPError: The internal HTTP library has thrown an error: reqwest::Error { kind: Decode, source: reqwest::Error { kind: Body, source: hyper::Error( Body, Custom { kind: UnexpectedEof, error: "unexpected EOF during chunk size line", }, ), }, } ``` Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 308ddf3 commit 03b0e9c

File tree

3 files changed

+67
-5
lines changed

3 files changed

+67
-5
lines changed

src/apify_client/_utils.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,16 @@ def maybe_parse_response(response: Response) -> Any:
281281

282282

283283
def is_retryable_error(exc: Exception) -> bool:
284-
"""Check if the given error is retryable."""
284+
"""Check if the given error is retryable.
285+
286+
All ``impit.HTTPError`` subclasses are considered retryable because they represent transport-level failures
287+
(network issues, timeouts, protocol errors, body decoding errors) that are typically transient. HTTP status
288+
code errors are handled separately in ``_make_request`` based on the response status code, not here.
289+
"""
285290
return isinstance(
286291
exc,
287292
(
288293
InvalidResponseBodyError,
289-
impit.NetworkError,
290-
impit.TimeoutException,
291-
impit.RemoteProtocolError,
294+
impit.HTTPError,
292295
),
293296
)

tests/unit/test_client_timeouts.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from unittest.mock import Mock
55

66
import pytest
7-
from impit import Response, TimeoutException
7+
from impit import HTTPError, Response, TimeoutException
88

99
from apify_client import ApifyClient
1010
from apify_client._http_client import HTTPClient, HTTPClientAsync
@@ -76,6 +76,32 @@ async def mock_request(*_args: Any, **kwargs: Any) -> Response:
7676
assert response.status_code == 200
7777

7878

79+
async def test_retry_on_http_error_async_client(monkeypatch: pytest.MonkeyPatch) -> None:
80+
"""Tests that bare impit.HTTPError (e.g. body decode errors) are retried.
81+
82+
This reproduces the scenario where the HTTP response body is truncated mid-stream
83+
(e.g. "unexpected EOF during chunk size line"), which impit raises as a generic HTTPError.
84+
"""
85+
should_raise_error = iter((True, True, False))
86+
retry_counter_mock = Mock()
87+
88+
async def mock_request(*_args: Any, **_kwargs: Any) -> Response:
89+
retry_counter_mock()
90+
should_raise = next(should_raise_error)
91+
if should_raise:
92+
raise HTTPError('The internal HTTP library has thrown an error: unexpected EOF during chunk size line')
93+
94+
return Response(status_code=200)
95+
96+
monkeypatch.setattr('impit.AsyncClient.request', mock_request)
97+
98+
response = await HTTPClientAsync(timeout_secs=5).call(method='GET', url='http://placeholder.url/http_error')
99+
100+
# 3 attempts: 2 failures + 1 success
101+
assert retry_counter_mock.call_count == 3
102+
assert response.status_code == 200
103+
104+
79105
def test_dynamic_timeout_sync_client(monkeypatch: pytest.MonkeyPatch) -> None:
80106
"""Tests timeout values for request with retriable errors.
81107

tests/unit/test_utils.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,19 @@
22
from collections.abc import Callable
33
from typing import Any
44

5+
import impit
56
import pytest
67
from apify_shared.consts import WebhookEventType
78

89
from apify_client._utils import (
910
encode_webhook_list_to_base64,
11+
is_retryable_error,
1012
pluck_data,
1113
retry_with_exp_backoff,
1214
retry_with_exp_backoff_async,
1315
to_safe_id,
1416
)
17+
from apify_client.errors import InvalidResponseBodyError
1518

1619

1720
def test__to_safe_id() -> None:
@@ -154,3 +157,33 @@ def test__encode_webhook_list_to_base64() -> None:
154157
)
155158
== 'W3siZXZlbnRUeXBlcyI6IFsiQUNUT1IuUlVOLkNSRUFURUQiXSwgInJlcXVlc3RVcmwiOiAiaHR0cHM6Ly9leGFtcGxlLmNvbS9ydW4tY3JlYXRlZCJ9LCB7ImV2ZW50VHlwZXMiOiBbIkFDVE9SLlJVTi5TVUNDRUVERUQiXSwgInJlcXVlc3RVcmwiOiAiaHR0cHM6Ly9leGFtcGxlLmNvbS9ydW4tc3VjY2VlZGVkIiwgInBheWxvYWRUZW1wbGF0ZSI6ICJ7XCJoZWxsb1wiOiBcIndvcmxkXCIsIFwicmVzb3VyY2VcIjp7e3Jlc291cmNlfX19In1d' # noqa: E501
156159
)
160+
161+
162+
@pytest.mark.parametrize(
163+
'exc',
164+
[
165+
InvalidResponseBodyError(impit.Response(status_code=200)),
166+
impit.HTTPError('generic http error'),
167+
impit.NetworkError('network error'),
168+
impit.TimeoutException('timeout'),
169+
impit.RemoteProtocolError('remote protocol error'),
170+
impit.ReadError('read error'),
171+
impit.ConnectError('connect error'),
172+
impit.WriteError('write error'),
173+
impit.DecodingError('decoding error'),
174+
],
175+
)
176+
def test__is_retryable_error(exc: Exception) -> None:
177+
assert is_retryable_error(exc) is True
178+
179+
180+
@pytest.mark.parametrize(
181+
'exc',
182+
[
183+
Exception('generic exception'),
184+
ValueError('value error'),
185+
RuntimeError('runtime error'),
186+
],
187+
)
188+
def test__is_not_retryable_error(exc: Exception) -> None:
189+
assert is_retryable_error(exc) is False

0 commit comments

Comments
 (0)