Skip to content

Commit d22ef78

Browse files
authored
test: Deflake throttled-request deferral tests with a stopped throttler clock (#2275)
`test_static_handler_raised_throttled_error_is_deferred` failed on macOS ([run](https://github.com/apify/crawlee-python/actions/runs/37274209355/job/111647535443)). The test put the domain into a 50 ms backoff and raised `RequestThrottledError`. The crawler defers the request only if `is_throttled()` still holds when it checks, and on a loaded runner the backoff ran out first. The request then went down the ordinary-error path and fell back to the browser. A new helper, `open_throttler_held_until_reclaim()` in `tests/unit/utils.py`, opens the throttler on a stopped `Mock` clock with a 1-minute backoff. The domain is always still held back when the crawler checks. When the crawler reclaims the deferred request, the clock moves past the backoff, so the retry is dispatched right away. The same race also failed `test_handler_raised_throttled_error_is_deferred` in the basic crawler suite ([run](https://github.com/apify/crawlee-python/actions/runs/37325111977/job/111813759733), on #2274). Six deferral tests in the basic, HTTP, Playwright and adaptive crawler suites now use the helper. Their assertions are unchanged, and all of them fail when `_is_held_back` is forced to `False`. The stall tests still use the 50 ms backoff, since they need real time to pass through `max_domain_stall`. Closes: #2271 *✍️ Drafted by Claude Code*
1 parent fa95976 commit d22ef78

5 files changed

Lines changed: 56 additions & 48 deletions

File tree

‎tests/unit/crawlers/_adaptive_playwright/test_adaptive_playwright_crawler.py‎

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@
3232
from crawlee.crawlers._adaptive_playwright._adaptive_playwright_crawling_context import AdaptiveContextError
3333
from crawlee.errors import RequestThrottledError
3434
from crawlee.http_clients import HttpClient, HttpCrawlingResult
35-
from crawlee.request_loaders import ThrottlingRequestManager
3635
from crawlee.sessions import SessionPool
3736
from crawlee.statistics import Statistics
3837
from crawlee.storage_clients import SqlStorageClient
3938
from crawlee.storages import KeyValueStore, RequestQueue
39+
from tests.unit.utils import open_throttler_held_until_reclaim
4040

4141
if TYPE_CHECKING:
4242
from collections.abc import AsyncGenerator, Iterator
@@ -941,16 +941,10 @@ async def request_handler(_context: AdaptivePlaywrightCrawlingContext) -> None:
941941
mocked_handler.assert_called()
942942

943943

944-
async def test_throttled_429_is_deferred() -> None:
944+
async def test_throttled_429_is_deferred(monkeypatch: pytest.MonkeyPatch) -> None:
945945
"""A throttled 429 from the browser sub crawler is retried later without spending a retry."""
946946
url = 'https://throttled.placeholder.com/page'
947-
throttler = ThrottlingRequestManager(
948-
await RequestQueue.open(),
949-
domains=['throttled.placeholder.com'],
950-
request_manager_opener=RequestQueue.open,
951-
base_delay=timedelta(milliseconds=50),
952-
max_delay=timedelta(milliseconds=100),
953-
)
947+
throttler = await open_throttler_held_until_reclaim(monkeypatch)
954948
failed_request_handler = AsyncMock()
955949
crawler = AdaptivePlaywrightCrawler.with_beautifulsoup_static_parser(
956950
rendering_type_predictor=_SimpleRenderingTypePredictor(
@@ -980,15 +974,9 @@ async def handler(context: AdaptivePlaywrightCrawlingContext) -> None:
980974
assert handled[0].request.retry_count == 0
981975

982976

983-
async def test_throttled_static_429_is_deferred() -> None:
977+
async def test_throttled_static_429_is_deferred(monkeypatch: pytest.MonkeyPatch) -> None:
984978
"""A throttled 429 from the static sub crawler defers the request without falling back to the browser."""
985-
throttler = ThrottlingRequestManager(
986-
await RequestQueue.open(),
987-
domains=['throttled.placeholder.com'],
988-
request_manager_opener=RequestQueue.open,
989-
base_delay=timedelta(milliseconds=50),
990-
max_delay=timedelta(milliseconds=100),
991-
)
979+
throttler = await open_throttler_held_until_reclaim(monkeypatch)
992980
http_client = AsyncMock(spec=HttpClient)
993981
http_client.crawl.side_effect = [
994982
HttpCrawlingResult(http_response=Mock(status_code=status, headers={}, read=AsyncMock(return_value=b'')))
@@ -1025,16 +1013,10 @@ async def handler(context: AdaptivePlaywrightCrawlingContext) -> None:
10251013
assert handled[0].request.retry_count == 0
10261014

10271015

1028-
async def test_static_handler_raised_throttled_error_is_deferred() -> None:
1016+
async def test_static_handler_raised_throttled_error_is_deferred(monkeypatch: pytest.MonkeyPatch) -> None:
10291017
"""A `RequestThrottledError` from the static handler defers the request without falling back to the browser."""
10301018
url = 'https://throttled.placeholder.com/page'
1031-
throttler = ThrottlingRequestManager(
1032-
await RequestQueue.open(),
1033-
domains=['throttled.placeholder.com'],
1034-
request_manager_opener=RequestQueue.open,
1035-
base_delay=timedelta(milliseconds=50),
1036-
max_delay=timedelta(milliseconds=100),
1037-
)
1019+
throttler = await open_throttler_held_until_reclaim(monkeypatch)
10381020
http_client = AsyncMock(spec=HttpClient)
10391021
http_client.crawl.return_value = HttpCrawlingResult(
10401022
http_response=Mock(status_code=200, headers={}, read=AsyncMock(return_value=b''))

‎tests/unit/crawlers/_basic/test_basic_crawler.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
from crawlee.statistics import FinalStatistics, StatisticsState
4141
from crawlee.storage_clients import FileSystemStorageClient, MemoryStorageClient
4242
from crawlee.storages import Dataset, KeyValueStore, RequestQueue
43-
from tests.unit.utils import poll_until_condition
43+
from tests.unit.utils import open_throttler_held_until_reclaim, poll_until_condition
4444

4545
if TYPE_CHECKING:
4646
from collections.abc import Callable, Sequence
@@ -2745,9 +2745,9 @@ async def open_throttler() -> ThrottlingRequestManager[RequestQueue]:
27452745
)
27462746

27472747

2748-
async def test_handler_raised_throttled_error_is_deferred() -> None:
2748+
async def test_handler_raised_throttled_error_is_deferred(monkeypatch: pytest.MonkeyPatch) -> None:
27492749
"""A `RequestThrottledError` from the handler costs neither a retry nor session reputation."""
2750-
throttler = await open_throttler()
2750+
throttler = await open_throttler_held_until_reclaim(monkeypatch)
27512751
failed_request_handler = AsyncMock()
27522752
crawler = BasicCrawler(
27532753
request_manager=throttler,
@@ -2960,9 +2960,11 @@ async def handler(context: BasicCrawlingContext) -> None:
29602960
pytest.param(['error', 'error', '429', 'ok'], [0, 0, 1], id='after-retries'),
29612961
],
29622962
)
2963-
async def test_deferral_not_counted_as_retry(outcomes: list[str], expected_histogram: list[int]) -> None:
2963+
async def test_deferral_not_counted_as_retry(
2964+
outcomes: list[str], expected_histogram: list[int], monkeypatch: pytest.MonkeyPatch
2965+
) -> None:
29642966
"""A deferral doesn't show up in the retry histogram, while earlier retries still do."""
2965-
throttler = await open_throttler()
2967+
throttler = await open_throttler_held_until_reclaim(monkeypatch)
29662968
crawler = BasicCrawler(request_manager=throttler, max_request_retries=3)
29672969

29682970
@crawler.router.default_handler

‎tests/unit/crawlers/_http/test_http_crawler.py‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from crawlee.statistics import Statistics
1919
from crawlee.storages import RequestQueue
2020
from tests.unit.server_endpoints import HELLO_WORLD
21+
from tests.unit.utils import open_throttler_held_until_reclaim
2122

2223
if TYPE_CHECKING:
2324
from collections.abc import Awaitable, Callable
@@ -705,15 +706,9 @@ def crawl_result(status_code: int) -> HttpCrawlingResult:
705706
pytest.param({'ignore_http_error_status_codes': {429}}, id='ignored_429'),
706707
],
707708
)
708-
async def test_throttled_429_is_deferred(crawler_kwargs: dict[str, Any]) -> None:
709+
async def test_throttled_429_is_deferred(crawler_kwargs: dict[str, Any], monkeypatch: pytest.MonkeyPatch) -> None:
709710
"""A throttled 429 is retried later without spending a retry, a session rotation or session reputation."""
710-
throttler = ThrottlingRequestManager(
711-
await RequestQueue.open(),
712-
domains=['throttled.placeholder.com'],
713-
request_manager_opener=RequestQueue.open,
714-
base_delay=timedelta(milliseconds=50),
715-
max_delay=timedelta(milliseconds=100),
716-
)
711+
throttler = await open_throttler_held_until_reclaim(monkeypatch)
717712
http_client = AsyncMock(spec=HttpClient)
718713
http_client.crawl.side_effect = [crawl_result(429), crawl_result(200)]
719714
failed_request_handler = AsyncMock()

‎tests/unit/crawlers/_playwright/test_playwright_crawler.py‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@
3838
from crawlee.fingerprint_suite._header_generator import fingerprint_browser_type_from_playwright_browser_type
3939
from crawlee.http_clients import ImpitHttpClient
4040
from crawlee.proxy_configuration import ProxyConfiguration
41-
from crawlee.request_loaders import ThrottlingRequestManager
4241
from crawlee.sessions import Session, SessionPool
4342
from crawlee.statistics import Statistics
4443
from crawlee.statistics._error_snapshotter import ErrorSnapshotter
4544
from crawlee.storages import RequestQueue
4645
from tests.unit.server_endpoints import GENERIC_RESPONSE, HELLO_WORLD
46+
from tests.unit.utils import open_throttler_held_until_reclaim
4747

4848
if TYPE_CHECKING:
4949
from pathlib import Path
@@ -1362,16 +1362,10 @@ async def failed_handler(context: BasicCrawlingContext | PlaywrightCrawlingConte
13621362
assert set(failed_handler_calls) <= {HELLO_WORLD.decode()}
13631363

13641364

1365-
async def test_throttled_429_is_deferred() -> None:
1365+
async def test_throttled_429_is_deferred(monkeypatch: pytest.MonkeyPatch) -> None:
13661366
"""A throttled 429 is retried later without spending a retry, a session rotation or session reputation."""
13671367
url = 'https://throttled.placeholder.com/page'
1368-
throttler = ThrottlingRequestManager(
1369-
await RequestQueue.open(),
1370-
domains=['throttled.placeholder.com'],
1371-
request_manager_opener=RequestQueue.open,
1372-
base_delay=timedelta(milliseconds=50),
1373-
max_delay=timedelta(milliseconds=100),
1374-
)
1368+
throttler = await open_throttler_held_until_reclaim(monkeypatch)
13751369
failed_request_handler = AsyncMock()
13761370
crawler = PlaywrightCrawler(
13771371
request_manager=throttler,

‎tests/unit/utils.py‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,24 @@
55
import sys
66
import time
77
from contextlib import asynccontextmanager
8+
from datetime import datetime, timedelta, timezone
89
from typing import TYPE_CHECKING, Any, TypeVar, cast, overload
9-
from unittest.mock import AsyncMock, MagicMock
10+
from unittest.mock import AsyncMock, MagicMock, Mock
1011

1112
import pytest
1213

1314
from crawlee.http_clients._base import HttpClient, HttpResponse
15+
from crawlee.request_loaders import ThrottlingRequestManager, _throttling_request_manager
16+
from crawlee.storages import RequestQueue
1417

1518
if TYPE_CHECKING:
1619
from collections.abc import AsyncIterator, Awaitable, Callable
1720

1821
from yarl import URL
1922

23+
from crawlee import Request
24+
from crawlee.storage_clients.models import ProcessedRequest
25+
2026
T = TypeVar('T')
2127

2228
run_alone_on_mac = pytest.mark.run_alone if sys.platform == 'darwin' else lambda x: x
@@ -60,6 +66,35 @@ async def read_stream() -> AsyncIterator[bytes]:
6066
return client, attempts
6167

6268

69+
async def open_throttler_held_until_reclaim(
70+
monkeypatch: pytest.MonkeyPatch,
71+
) -> ThrottlingRequestManager[RequestQueue]:
72+
"""Open a throttler for `throttled.placeholder.com` whose backoff lasts until the crawler gives a request back.
73+
74+
The throttler runs on a stopped clock, so a backoff can't run out before the crawler checks whether the domain is
75+
held back. Each reclaimed request moves the clock past the backoff, so a deferred request is dispatched right away.
76+
"""
77+
backoff = timedelta(minutes=1)
78+
clock = Mock()
79+
clock.now.return_value = datetime.now(timezone.utc)
80+
monkeypatch.setattr(_throttling_request_manager, 'datetime', clock)
81+
throttler = ThrottlingRequestManager(
82+
await RequestQueue.open(),
83+
domains=['throttled.placeholder.com'],
84+
request_manager_opener=RequestQueue.open,
85+
base_delay=backoff,
86+
max_delay=backoff,
87+
)
88+
reclaim_request = throttler.reclaim_request
89+
90+
async def reclaim_after_backoff(request: Request, *, forefront: bool = False) -> ProcessedRequest | None:
91+
clock.now.return_value += backoff
92+
return await reclaim_request(request, forefront=forefront)
93+
94+
monkeypatch.setattr(throttler, 'reclaim_request', reclaim_after_backoff)
95+
return throttler
96+
97+
6398
async def maybe_await(value: Awaitable[T] | T) -> T:
6499
"""Await `value` if it is awaitable, otherwise return it unchanged.
65100

0 commit comments

Comments
 (0)