From 1cf44e3b9ab2f1d00dbf36fdd46a4f09fde7dca9 Mon Sep 17 00:00:00 2001 From: Vlada Dusek Date: Sat, 31 Oct 2026 12:30:41 +0100 Subject: [PATCH] feat!: pass the skipped `Request` to `on_skipped_request` --- .../respect_robots_on_skipped_request.py | 6 +-- docs/upgrading/upgrading_to_v2.md | 19 ++++++++ .../_abstract_http/_abstract_http_crawler.py | 15 +++++- src/crawlee/crawlers/_basic/_basic_crawler.py | 19 ++++---- .../_playwright/_playwright_crawler.py | 15 +++++- .../crawlers/_basic/test_basic_crawler.py | 47 +++++++++++++++++++ .../test_beautifulsoup_crawler.py | 15 +++--- .../crawlers/_parsel/test_parsel_crawler.py | 15 +++--- .../_playwright/test_playwright_crawler.py | 15 +++--- 9 files changed, 131 insertions(+), 35 deletions(-) diff --git a/docs/examples/code_examples/respect_robots_on_skipped_request.py b/docs/examples/code_examples/respect_robots_on_skipped_request.py index 5c7eca173f..5084dc18dc 100644 --- a/docs/examples/code_examples/respect_robots_on_skipped_request.py +++ b/docs/examples/code_examples/respect_robots_on_skipped_request.py @@ -1,6 +1,6 @@ import asyncio -from crawlee import SkippedReason +from crawlee import Request, SkippedReason from crawlee.crawlers import ( BeautifulSoupCrawler, BeautifulSoupCrawlingContext, @@ -18,10 +18,10 @@ async def request_handler(context: BeautifulSoupCrawlingContext) -> None: # highlight-start # This handler is called when a request is skipped @crawler.on_skipped_request - async def skipped_request_handler(url: str, reason: SkippedReason) -> None: + async def skipped_request_handler(request: Request, reason: SkippedReason) -> None: # Check if the request was skipped due to robots.txt rules if reason == 'robots_txt': - crawler.log.info(f'Skipped {url} due to robots.txt rules.') + crawler.log.info(f'Skipped {request.url} due to robots.txt rules.') # highlight-end diff --git a/docs/upgrading/upgrading_to_v2.md b/docs/upgrading/upgrading_to_v2.md index f7e0bd036b..493d19918c 100644 --- a/docs/upgrading/upgrading_to_v2.md +++ b/docs/upgrading/upgrading_to_v2.md @@ -123,6 +123,25 @@ Unlike tiers, this function doesn't track blocking per domain, so every request The `Request.forefront` property is removed. Proxy tiers were the only code that set it. The crawler now reclaims a failed request for a retry at the back of the queue, and a request deferred by a `RequestThrottledError` at the front. To enqueue a request at the front, pass `forefront=True` to `add_requests` or `reclaim_request`. +## `on_skipped_request` receives the `Request` + +The callback registered with `on_skipped_request` gets the skipped `Request` as its first argument, so it can read the request's `user_data` and `label`. In v1, it got only the URL string. Update the callback's signature and read the URL from `request.url`: + +```python +# v1 +@crawler.on_skipped_request +async def on_skipped(url: str, reason: SkippedReason) -> None: + crawler.log.info(f'Skipped {url}: {reason}') + + +# v2 +@crawler.on_skipped_request +async def on_skipped(request: Request, reason: SkippedReason) -> None: + crawler.log.info(f'Skipped {request.url}: {reason}') +``` + +For links that `enqueue_links` skips because of `robots.txt`, the `Request` carries the `label` and `user_data` passed to `enqueue_links`. The `transform_request_function` isn't applied to these links. + ## Unique keys keep the URL casing The default `Request.unique_key` lowercases only the scheme and host of the URL. The path, query and fragment keep their casing, since they're case-sensitive. In v1, the whole URL was lowercased, so URLs that differed only in casing, such as `https://example.com/Product/ABC` and `https://example.com/product/abc`, got the same unique key, and the crawler visited only one of them. diff --git a/src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py b/src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py index 5f189b5b97..b085110793 100644 --- a/src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py +++ b/src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py @@ -262,8 +262,21 @@ async def extract_links( requests.append(request) + skipped_requests = list[Request]() + + for url in skipped: + try: + skipped_request = Request.from_url( + url=url, user_data={**base_user_data}, label=label, enqueue_strategy=strategy + ) + except ValidationError as exc: + context.log.debug(f'Skipping URL "{url}" disallowed by robots.txt due to invalid format: {exc}.') + continue + + skipped_requests.append(skipped_request) + skipped_tasks = [ - asyncio.create_task(self._handle_skipped_request(request, 'robots_txt')) for request in skipped + asyncio.create_task(self._handle_skipped_request(request, 'robots_txt')) for request in skipped_requests ] await asyncio.gather(*skipped_tasks) diff --git a/src/crawlee/crawlers/_basic/_basic_crawler.py b/src/crawlee/crawlers/_basic/_basic_crawler.py index 4b2dc37295..204cdf8aed 100644 --- a/src/crawlee/crawlers/_basic/_basic_crawler.py +++ b/src/crawlee/crawlers/_basic/_basic_crawler.py @@ -113,7 +113,7 @@ ErrorHandler = Callable[[TCrawlingContext, Exception], Awaitable[Request | None]] FailedRequestHandler = Callable[[TCrawlingContext, Exception], Awaitable[None]] -SkippedRequestCallback = Callable[[str, SkippedReason], Awaitable[None]] +SkippedRequestCallback = Callable[[Request, SkippedReason], Awaitable[None]] class _BasicCrawlerOptions(TypedDict): @@ -713,7 +713,8 @@ def failed_request_handler( def on_skipped_request(self, callback: SkippedRequestCallback) -> SkippedRequestCallback: """Register a function to handle skipped requests. - The skipped request handler is invoked when a request is skipped due to a collision or other reasons. + The skipped request handler is invoked when a request is skipped due to a collision or other reasons. It + receives the skipped `Request`, including its `user_data` and `label`, and the reason it was skipped. """ self._on_skipped_request = callback return callback @@ -926,14 +927,16 @@ async def add_requests( wait_for_all_requests_to_be_added_timeout: Timeout for waiting for all requests to be added. """ allowed_requests = [] - skipped = [] + skipped = list[Request]() for request in requests: check_url = request.url if isinstance(request, Request) else request if await self._is_allowed_based_on_robots_txt_file(check_url): allowed_requests.append(request) - else: + elif isinstance(request, Request): skipped.append(request) + else: + skipped.append(Request.from_url(request)) if skipped: skipped_tasks = [ @@ -1358,17 +1361,15 @@ async def _handle_failed_request(self, context: TCrawlingContext | BasicCrawling raise UserDefinedErrorHandlerError('Exception thrown in user-defined failed request handler') from e async def _handle_skipped_request( - self, request: Request | str, reason: SkippedReason, *, need_mark: bool = False + self, request: Request, reason: SkippedReason, *, need_mark: bool = False ) -> None: - if need_mark and isinstance(request, Request): + if need_mark: request.state = RequestState.SKIPPED await self._mark_request_as_handled(request) - url = request.url if isinstance(request, Request) else request - if self._on_skipped_request: try: - await self._on_skipped_request(url, reason) + await self._on_skipped_request(request, reason) except Exception as e: raise UserDefinedErrorHandlerError('Exception thrown in user-defined skipped request callback') from e diff --git a/src/crawlee/crawlers/_playwright/_playwright_crawler.py b/src/crawlee/crawlers/_playwright/_playwright_crawler.py index 3069845b98..30309d340e 100644 --- a/src/crawlee/crawlers/_playwright/_playwright_crawler.py +++ b/src/crawlee/crawlers/_playwright/_playwright_crawler.py @@ -488,8 +488,21 @@ async def extract_links( requests.append(request) + skipped_requests = list[Request]() + + for url in skipped: + try: + skipped_request = Request.from_url( + url=url, user_data={**base_user_data}, label=label, enqueue_strategy=strategy + ) + except ValidationError as exc: + context.log.debug(f'Skipping URL "{url}" disallowed by robots.txt due to invalid format: {exc}.') + continue + + skipped_requests.append(skipped_request) + skipped_tasks = [ - asyncio.create_task(self._handle_skipped_request(request, 'robots_txt')) for request in skipped + asyncio.create_task(self._handle_skipped_request(request, 'robots_txt')) for request in skipped_requests ] await asyncio.gather(*skipped_tasks) diff --git a/tests/unit/crawlers/_basic/test_basic_crawler.py b/tests/unit/crawlers/_basic/test_basic_crawler.py index f4b614b557..7a2cf26ae9 100644 --- a/tests/unit/crawlers/_basic/test_basic_crawler.py +++ b/tests/unit/crawlers/_basic/test_basic_crawler.py @@ -2360,6 +2360,53 @@ async def failed_request_handler(context: BasicCrawlingContext, error: Exception assert set(requests) == handler_requests +@pytest.mark.parametrize( + 'skipped_at', + [ + pytest.param('add_requests', id='skipped when added'), + pytest.param('fetch', id='skipped when fetched from RQ'), + ], +) +async def test_on_skipped_request_receives_request( + server_url: URL, skipped_at: Literal['add_requests', 'fetch'] +) -> None: + """A request disallowed by robots.txt reaches the skipped request callback with its user data.""" + queue = await RequestQueue.open() + crawler = BasicCrawler(request_manager=queue, respect_robots_txt_file=True, request_handler=AsyncMock()) + skipped_hook = AsyncMock() + crawler.on_skipped_request(skipped_hook) + request = Request.from_url(str(server_url / 'page_1'), user_data={'source': 'test'}) + + if skipped_at == 'add_requests': + await crawler.add_requests([request]) + else: + await queue.add_request(request) + await crawler.run() + + skipped_hook.assert_awaited_once() + skipped_request, reason = skipped_hook.call_args.args + assert isinstance(skipped_request, Request) + assert skipped_request.url == request.url + assert skipped_request.user_data['source'] == 'test' + assert reason == 'robots_txt' + + +async def test_on_skipped_request_receives_request_for_url_string(server_url: URL) -> None: + """A URL string passed to `add_requests` and disallowed by robots.txt reaches the callback as a `Request`.""" + crawler = BasicCrawler(respect_robots_txt_file=True) + skipped_hook = AsyncMock() + crawler.on_skipped_request(skipped_hook) + url = str(server_url / 'page_1') + + await crawler.add_requests([url]) + + skipped_hook.assert_awaited_once() + skipped_request, reason = skipped_hook.call_args.args + assert isinstance(skipped_request, Request) + assert skipped_request.url == url + assert reason == 'robots_txt' + + async def test_lock_with_get_robots_txt_file_for_url(server_url: URL) -> None: crawler = BasicCrawler(respect_robots_txt_file=True) diff --git a/tests/unit/crawlers/_beautifulsoup/test_beautifulsoup_crawler.py b/tests/unit/crawlers/_beautifulsoup/test_beautifulsoup_crawler.py index 7f5ee1130b..66c62cc9e2 100644 --- a/tests/unit/crawlers/_beautifulsoup/test_beautifulsoup_crawler.py +++ b/tests/unit/crawlers/_beautifulsoup/test_beautifulsoup_crawler.py @@ -253,24 +253,25 @@ async def error_handler(context: BasicCrawlingContext, _error: Exception) -> Non async def test_on_skipped_request(server_url: URL, http_client: HttpClient) -> None: + """Links disallowed by robots.txt reach the skipped request callback as requests with their label and user data.""" crawler = BeautifulSoupCrawler(http_client=http_client, respect_robots_txt_file=True) skip = mock.Mock() @crawler.router.default_handler async def request_handler(context: BeautifulSoupCrawlingContext) -> None: - await context.enqueue_links() + await context.enqueue_links(label='DETAIL', user_data={'source': 'start'}) @crawler.on_skipped_request - async def skipped_hook(url: str, _reason: SkippedReason) -> None: - skip(url) + async def skipped_hook(request: Request, reason: SkippedReason) -> None: + skip(request.url, request.label, request.user_data.get('source'), reason) await crawler.run([str(server_url / 'start_enqueue')]) expected_skip_calls = [ - mock.call(str(server_url / 'page_1')), - mock.call(str(server_url / 'page_2')), - mock.call(str(server_url / 'page_3')), - mock.call(str(server_url / 'page_4')), + mock.call(str(server_url / 'page_1'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_2'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_3'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_4'), 'DETAIL', 'start', 'robots_txt'), ] skip.assert_has_calls(expected_skip_calls, any_order=True) diff --git a/tests/unit/crawlers/_parsel/test_parsel_crawler.py b/tests/unit/crawlers/_parsel/test_parsel_crawler.py index 2b26138336..1a96ae3339 100644 --- a/tests/unit/crawlers/_parsel/test_parsel_crawler.py +++ b/tests/unit/crawlers/_parsel/test_parsel_crawler.py @@ -340,24 +340,25 @@ async def error_handler(context: BasicCrawlingContext, _error: Exception) -> Non async def test_on_skipped_request(server_url: URL, http_client: HttpClient) -> None: + """Links disallowed by robots.txt reach the skipped request callback as requests with their label and user data.""" crawler = ParselCrawler(http_client=http_client, respect_robots_txt_file=True) skip = mock.Mock() @crawler.router.default_handler async def request_handler(context: ParselCrawlingContext) -> None: - await context.enqueue_links() + await context.enqueue_links(label='DETAIL', user_data={'source': 'start'}) @crawler.on_skipped_request - async def skipped_hook(url: str, _reason: SkippedReason) -> None: - skip(url) + async def skipped_hook(request: Request, reason: SkippedReason) -> None: + skip(request.url, request.label, request.user_data.get('source'), reason) await crawler.run([str(server_url / 'start_enqueue')]) expected_skip_calls = [ - mock.call(str(server_url / 'page_1')), - mock.call(str(server_url / 'page_2')), - mock.call(str(server_url / 'page_3')), - mock.call(str(server_url / 'page_4')), + mock.call(str(server_url / 'page_1'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_2'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_3'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_4'), 'DETAIL', 'start', 'robots_txt'), ] skip.assert_has_calls(expected_skip_calls, any_order=True) diff --git a/tests/unit/crawlers/_playwright/test_playwright_crawler.py b/tests/unit/crawlers/_playwright/test_playwright_crawler.py index 0c8578ce13..5670f65a80 100644 --- a/tests/unit/crawlers/_playwright/test_playwright_crawler.py +++ b/tests/unit/crawlers/_playwright/test_playwright_crawler.py @@ -871,24 +871,25 @@ async def error_handler(context: BasicCrawlingContext, _error: Exception) -> Non async def test_on_skipped_request(server_url: URL) -> None: + """Links disallowed by robots.txt reach the skipped request callback as requests with their label and user data.""" crawler = PlaywrightCrawler(respect_robots_txt_file=True) skip = mock.Mock() @crawler.router.default_handler async def request_handler(context: PlaywrightCrawlingContext) -> None: - await context.enqueue_links() + await context.enqueue_links(label='DETAIL', user_data={'source': 'start'}) @crawler.on_skipped_request - async def skipped_hook(url: str, _reason: SkippedReason) -> None: - skip(url) + async def skipped_hook(request: Request, reason: SkippedReason) -> None: + skip(request.url, request.label, request.user_data.get('source'), reason) await crawler.run([str(server_url / 'start_enqueue')]) expected_skip_calls = [ - mock.call(str(server_url / 'page_1')), - mock.call(str(server_url / 'page_2')), - mock.call(str(server_url / 'page_3')), - mock.call(str(server_url / 'page_4')), + mock.call(str(server_url / 'page_1'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_2'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_3'), 'DETAIL', 'start', 'robots_txt'), + mock.call(str(server_url / 'page_4'), 'DETAIL', 'start', 'robots_txt'), ] skip.assert_has_calls(expected_skip_calls, any_order=True)