Skip to content

Commit 9c23101

Browse files
authored
fix(throttling-manager): Apply per-domain backoff when retry_on_blocked is disabled (#2158)
### Description - Recording a 429 is now separate from the session blocking check and runs regardless of `retry_on_blocked`. That configuration previously dropped the 429 backoff and the robots.txt crawl-delay, while warning that no throttler was in use. ### Issues - Closes: #2126 ### Testing - Added new unit tests
1 parent dd5c5e6 commit 9c23101

5 files changed

Lines changed: 94 additions & 53 deletions

File tree

src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ async def _make_http_request(self, context: BasicCrawlingContext) -> AsyncGenera
289289
async def _handle_status_code_response(
290290
self, context: HttpCrawlingContext
291291
) -> AsyncGenerator[HttpCrawlingContext, None]:
292-
"""Validate the HTTP status code and raise appropriate exceptions if needed.
292+
"""Record rate limiting and validate the HTTP status code, raising appropriate exceptions if needed.
293293
294294
Args:
295295
context: The current crawling context containing the HTTP response.
@@ -303,13 +303,13 @@ async def _handle_status_code_response(
303303
The original crawling context if no errors are detected.
304304
"""
305305
status_code = context.http_response.status_code
306+
self._record_rate_limit_status_code(
307+
status_code,
308+
request_url=context.request.url,
309+
retry_after_header=context.http_response.headers.get('retry-after'),
310+
)
306311
if self._retry_on_blocked:
307-
self._raise_for_session_blocked_status_code(
308-
context.session,
309-
status_code,
310-
request_url=context.request.url,
311-
retry_after_header=context.http_response.headers.get('retry-after'),
312-
)
312+
self._raise_for_session_blocked_status_code(context.session, status_code)
313313
self._raise_for_error_status_code(status_code)
314314
yield context
315315

src/crawlee/crawlers/_basic/_basic_crawler.py

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,51 +1621,59 @@ def _raise_for_error_status_code(self, status_code: int) -> None:
16211621
if is_status_code_server_error(status_code) and not is_ignored_status:
16221622
raise HttpStatusCodeError('Error status code returned', status_code)
16231623

1624-
def _raise_for_session_blocked_status_code(
1624+
def _record_rate_limit_status_code(
16251625
self,
1626-
session: Session | None,
16271626
status_code: int,
16281627
*,
16291628
request_url: str,
16301629
retry_after_header: str | None = None,
16311630
) -> None:
1632-
"""Raise an exception if the given status code indicates the session is blocked.
1631+
"""Record a 429 Too Many Requests response so the request's domain gets a backoff.
16331632
1634-
If the status code is 429 (Too Many Requests), the domain is recorded as rate-limited in the
1635-
`ThrottlingRequestManager` for per-domain backoff.
1633+
Rate limiting is independent of session blocking, so this runs for every response regardless of
1634+
`retry_on_blocked`.
16361635
16371636
Args:
1638-
session: The session used for the request. If `None`, no check is performed.
16391637
status_code: The HTTP status code to check.
16401638
request_url: The request URL, used for per-domain rate limit tracking.
16411639
retry_after_header: The value of the `Retry-After` response header, if present.
1642-
1643-
Raises:
1644-
SessionError: If the status code indicates the session is blocked.
16451640
"""
1646-
if status_code == HTTPStatus.TOO_MANY_REQUESTS:
1647-
if isinstance(self._request_manager, ThrottlingRequestManager):
1648-
retry_after = parse_retry_after_header(retry_after_header)
1649-
if not self._request_manager.record_domain_delay(request_url, retry_after=retry_after):
1650-
domain = (URL(request_url).host or '').lower().removesuffix('.')
1651-
if domain:
1652-
self._logger_once.log(
1653-
f'Received an HTTP 429 (Too Many Requests) response from domain "{domain}", but it is '
1654-
f'not in the `ThrottlingRequestManager.domains` list. Per-domain backoff will not be '
1655-
f'applied for this domain. Add it to `domains=` to enable throttling.',
1656-
key=f'unconfigured_throttle_domain:{domain}',
1657-
level=logging.WARNING,
1658-
)
1659-
else:
1641+
if status_code != HTTPStatus.TOO_MANY_REQUESTS:
1642+
return
1643+
1644+
if not isinstance(self._request_manager, ThrottlingRequestManager):
1645+
self._logger_once.log(
1646+
'Received an HTTP 429 (Too Many Requests) response, but the crawler is not using '
1647+
'`ThrottlingRequestManager`. Per-domain backoff and `Retry-After` headers will not be honored. '
1648+
'To enable per-domain rate limiting, configure the crawler to use `ThrottlingRequestManager` '
1649+
'as the request manager.',
1650+
key='no_throttling_manager_on_429',
1651+
level=logging.WARNING,
1652+
)
1653+
return
1654+
1655+
retry_after = parse_retry_after_header(retry_after_header)
1656+
if not self._request_manager.record_domain_delay(request_url, retry_after=retry_after):
1657+
domain = (URL(request_url).host or '').lower().removesuffix('.')
1658+
if domain:
16601659
self._logger_once.log(
1661-
'Received an HTTP 429 (Too Many Requests) response, but the crawler is not using '
1662-
'`ThrottlingRequestManager`. Per-domain backoff and `Retry-After` headers will not be honored. '
1663-
'To enable per-domain rate limiting, configure the crawler to use `ThrottlingRequestManager` '
1664-
'as the request manager.',
1665-
key='no_throttling_manager_on_429',
1660+
f'Received an HTTP 429 (Too Many Requests) response from domain "{domain}", but it is '
1661+
f'not in the `ThrottlingRequestManager.domains` list. Per-domain backoff will not be '
1662+
f'applied for this domain. Add it to `domains=` to enable throttling.',
1663+
key=f'unconfigured_throttle_domain:{domain}',
16661664
level=logging.WARNING,
16671665
)
16681666

1667+
def _raise_for_session_blocked_status_code(self, session: Session | None, status_code: int) -> None:
1668+
"""Raise an exception if the given status code indicates the session is blocked.
1669+
1670+
Args:
1671+
session: The session used for the request. If `None`, no check is performed.
1672+
status_code: The HTTP status code to check.
1673+
1674+
Raises:
1675+
SessionError: If the status code indicates the session is blocked.
1676+
"""
16691677
if session is not None and session.is_blocked_status_code(
16701678
status_code=status_code,
16711679
ignore_http_error_status_codes=self._ignore_http_error_status_codes,

src/crawlee/crawlers/_playwright/_playwright_crawler.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ async def extract_links(
494494
return extract_links
495495

496496
async def _handle_status_code_response(self, context: TPostNavContext) -> AsyncGenerator[TPostNavContext, None]:
497-
"""Validate the HTTP status code and raise appropriate exceptions if needed.
497+
"""Record rate limiting and validate the HTTP status code, raising appropriate exceptions if needed.
498498
499499
Args:
500500
context: The current crawling context containing the response.
@@ -508,13 +508,13 @@ async def _handle_status_code_response(self, context: TPostNavContext) -> AsyncG
508508
The original crawling context if no errors are detected.
509509
"""
510510
status_code = context.response.status
511+
self._record_rate_limit_status_code(
512+
status_code,
513+
request_url=context.request.url,
514+
retry_after_header=context.response.headers.get('retry-after'),
515+
)
511516
if self._retry_on_blocked:
512-
self._raise_for_session_blocked_status_code(
513-
context.session,
514-
status_code,
515-
request_url=context.request.url,
516-
retry_after_header=context.response.headers.get('retry-after'),
517-
)
517+
self._raise_for_session_blocked_status_code(context.session, status_code)
518518
self._raise_for_error_status_code(status_code)
519519
yield context
520520

tests/unit/crawlers/_basic/test_basic_crawler.py

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2517,8 +2517,8 @@ async def test_warn_no_throttling_manager_once_on_429(caplog: pytest.LogCaptureF
25172517
"""A 429 from a crawler without ThrottlingRequestManager logs a recommendation, only once per instance."""
25182518
crawler = BasicCrawler(configure_logging=False)
25192519
with caplog.at_level(logging.WARNING, logger='crawlee'):
2520-
crawler._raise_for_session_blocked_status_code(session=None, status_code=429, request_url='https://a.test/')
2521-
crawler._raise_for_session_blocked_status_code(session=None, status_code=429, request_url='https://b.test/')
2520+
crawler._record_rate_limit_status_code(429, request_url='https://a.test/')
2521+
crawler._record_rate_limit_status_code(429, request_url='https://b.test/')
25222522

25232523
matching = [
25242524
r for r in caplog.records if 'ThrottlingRequestManager' in r.getMessage() and 'HTTP 429' in r.getMessage()
@@ -2537,15 +2537,9 @@ async def test_warn_unconfigured_throttle_domain_once_per_domain(caplog: pytest.
25372537
crawler = BasicCrawler(configure_logging=False, request_manager=throttler)
25382538

25392539
with caplog.at_level(logging.WARNING, logger='crawlee'):
2540-
crawler._raise_for_session_blocked_status_code(
2541-
session=None, status_code=429, request_url='https://A.example.com/page1'
2542-
)
2543-
crawler._raise_for_session_blocked_status_code(
2544-
session=None, status_code=429, request_url='https://a.example.com/page2'
2545-
)
2546-
crawler._raise_for_session_blocked_status_code(
2547-
session=None, status_code=429, request_url='https://other.example.com/page1'
2548-
)
2540+
crawler._record_rate_limit_status_code(429, request_url='https://A.example.com/page1')
2541+
crawler._record_rate_limit_status_code(429, request_url='https://a.example.com/page2')
2542+
crawler._record_rate_limit_status_code(429, request_url='https://other.example.com/page1')
25492543

25502544
matching = [
25512545
r

tests/unit/crawlers/_http/test_http_crawler.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

33
import json
4+
from datetime import timedelta
45
from typing import TYPE_CHECKING
56
from unittest.mock import AsyncMock, Mock
67
from urllib.parse import parse_qs, urlencode
@@ -9,6 +10,7 @@
910

1011
from crawlee import ConcurrencySettings, Request, RequestState
1112
from crawlee.crawlers import HttpCrawler
13+
from crawlee.request_loaders import ThrottlingRequestManager
1214
from crawlee.sessions import SessionPool
1315
from crawlee.statistics import Statistics
1416
from crawlee.storages import RequestQueue
@@ -686,3 +688,40 @@ async def failed_request_handler(context: BasicCrawlingContext, _error: Exceptio
686688
}
687689

688690
await queue.drop()
691+
692+
693+
@pytest.mark.parametrize(
694+
'retry_on_blocked',
695+
[
696+
pytest.param(True, id='retry_on_blocked'),
697+
pytest.param(False, id='no_retry_on_blocked'),
698+
],
699+
)
700+
async def test_records_429_regardless_of_retry_on_blocked(
701+
mock_request_handler: AsyncMock,
702+
server_url: URL,
703+
*,
704+
retry_on_blocked: bool,
705+
) -> None:
706+
"""Rate limiting is a separate concern from session blocking, so a 429 must be recorded either way."""
707+
domain = server_url.host or ''
708+
inner = await RequestQueue.open(alias='throttle-429-inner')
709+
throttler = ThrottlingRequestManager(
710+
inner,
711+
domains=[domain],
712+
request_manager_opener=RequestQueue.open,
713+
# Long enough that the assertion below cannot race the backoff expiring.
714+
base_delay=timedelta(seconds=30),
715+
)
716+
crawler = HttpCrawler(
717+
request_handler=mock_request_handler,
718+
request_manager=throttler,
719+
retry_on_blocked=retry_on_blocked,
720+
max_request_retries=0,
721+
# Without this, a 429 retires the session and the rotation retries walk the backoff up to `max_delay`.
722+
max_session_rotations=0,
723+
)
724+
725+
await crawler.run([str(server_url / 'status/429')])
726+
727+
assert throttler._is_domain_throttled(domain)

0 commit comments

Comments
 (0)