From edba1ad5726f87a76bd36555e99e058f040f4af1 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 18 Aug 2025 21:40:46 +0500 Subject: [PATCH] Refactor RobotsTxtMiddleware to use async def process_request(). (#6802) --- docs/news.rst | 8 +++ scrapy/downloadermiddlewares/robotstxt.py | 64 ++++++++------------ tests/test_downloadermiddleware_robotstxt.py | 55 ++++++++++++----- 3 files changed, 72 insertions(+), 55 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 1bdd0a267..92320ac6a 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -44,6 +44,14 @@ Backward-incompatible changes - :meth:`~scrapy.spidermiddlewares.referer.ReferrerPolicy.referrer` +- :meth:`scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware.process_request` + now returns a coroutine, previously it returned a + :class:`~twisted.internet.defer.Deferred` object or ``None``. The + ``robot_parser()`` method was also changed to return a coroutine. This + change only impacts code that subclasses + :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware` or + calls its methods directly. + .. _release-2.13.3: diff --git a/scrapy/downloadermiddlewares/robotstxt.py b/scrapy/downloadermiddlewares/robotstxt.py index 48e4875a7..83af0f7bf 100644 --- a/scrapy/downloadermiddlewares/robotstxt.py +++ b/scrapy/downloadermiddlewares/robotstxt.py @@ -9,19 +9,16 @@ from __future__ import annotations import logging from typing import TYPE_CHECKING -from twisted.internet.defer import Deferred, maybeDeferred +from twisted.internet.defer import Deferred from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request, Response from scrapy.http.request import NO_CALLBACK -from scrapy.utils.defer import deferred_from_coro +from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.httpobj import urlparse_cached -from scrapy.utils.log import failure_to_exc_info from scrapy.utils.misc import load_object if TYPE_CHECKING: - from twisted.python.failure import Failure - # typing.Self requires Python 3.11 from typing_extensions import Self @@ -54,20 +51,13 @@ class RobotsTxtMiddleware: def from_crawler(cls, crawler: Crawler) -> Self: return cls(crawler) - def process_request( - self, request: Request, spider: Spider - ) -> Deferred[None] | None: + async def process_request(self, request: Request, spider: Spider) -> None: if request.meta.get("dont_obey_robotstxt"): - return None + return if request.url.startswith("data:") or request.url.startswith("file:"): - return None - d: Deferred[RobotParser | None] = maybeDeferred( - self.robot_parser, - request, - spider, # type: ignore[call-overload] - ) - d2: Deferred[None] = d.addCallback(self.process_request_2, request, spider) - return d2 + return + rp = await self.robot_parser(request, spider) + self.process_request_2(rp, request, spider) def process_request_2( self, rp: RobotParser | None, request: Request, spider: Spider @@ -89,9 +79,9 @@ class RobotsTxtMiddleware: self.crawler.stats.inc_value("robotstxt/forbidden") raise IgnoreRequest("Forbidden by robots.txt") - def robot_parser( + async def robot_parser( self, request: Request, spider: Spider - ) -> RobotParser | Deferred[RobotParser | None] | None: + ) -> RobotParser | None: url = urlparse_cached(request) netloc = url.netloc @@ -106,35 +96,29 @@ class RobotsTxtMiddleware: ) assert self.crawler.engine assert self.crawler.stats - dfd = deferred_from_coro(self.crawler.engine.download_async(robotsreq)) - dfd.addCallback(self._parse_robots, netloc, spider) - dfd.addErrback(self._logerror, robotsreq, spider) - dfd.addErrback(self._robots_error, netloc) + try: + resp = await self.crawler.engine.download_async(robotsreq) + self._parse_robots(resp, netloc) + except Exception as e: + self._logerror(e, robotsreq, spider) + self._robots_error(e, netloc) self.crawler.stats.inc_value("robotstxt/request_count") parser = self._parsers[netloc] if isinstance(parser, Deferred): - d: Deferred[RobotParser | None] = Deferred() - - def cb(result: RobotParser | None) -> RobotParser | None: - d.callback(result) - return result - - parser.addCallback(cb) - return d + return await maybe_deferred_to_future(parser) return parser - def _logerror(self, failure: Failure, request: Request, spider: Spider) -> Failure: - if failure.type is not IgnoreRequest: + def _logerror(self, exc: Exception, request: Request, spider: Spider) -> None: + if not isinstance(exc, IgnoreRequest): logger.error( "Error downloading %(request)s: %(f_exception)s", - {"request": request, "f_exception": failure.value}, - exc_info=failure_to_exc_info(failure), + {"request": request, "f_exception": exc}, + exc_info=True, # noqa: LOG014 extra={"spider": spider}, ) - return failure - def _parse_robots(self, response: Response, netloc: str, spider: Spider) -> None: + def _parse_robots(self, response: Response, netloc: str) -> None: assert self.crawler.stats self.crawler.stats.inc_value("robotstxt/response_count") self.crawler.stats.inc_value( @@ -146,9 +130,9 @@ class RobotsTxtMiddleware: self._parsers[netloc] = rp rp_dfd.callback(rp) - def _robots_error(self, failure: Failure, netloc: str) -> None: - if failure.type is not IgnoreRequest: - key = f"robotstxt/exception_count/{failure.type}" + def _robots_error(self, exc: Exception, netloc: str) -> None: + if not isinstance(exc, IgnoreRequest): + key = f"robotstxt/exception_count/{type(exc)}" assert self.crawler.stats self.crawler.stats.inc_value(key) rp_dfd = self._parsers[netloc] diff --git a/tests/test_downloadermiddleware_robotstxt.py b/tests/test_downloadermiddleware_robotstxt.py index ed229e558..3cf570180 100644 --- a/tests/test_downloadermiddleware_robotstxt.py +++ b/tests/test_downloadermiddleware_robotstxt.py @@ -1,11 +1,12 @@ from __future__ import annotations +import asyncio from typing import TYPE_CHECKING from unittest import mock import pytest from twisted.internet import error -from twisted.internet.defer import Deferred +from twisted.internet.defer import Deferred, DeferredList from twisted.python import failure from scrapy.downloadermiddlewares.robotstxt import RobotsTxtMiddleware @@ -17,7 +18,7 @@ from scrapy.settings import Settings from scrapy.utils.asyncio import call_later from scrapy.utils.defer import ( deferred_f_from_coro_f, - ensure_awaitable, + deferred_from_coro, maybe_deferred_to_future, ) from tests.test_robotstxt_interface import rerp_available @@ -78,6 +79,25 @@ Disallow: /some/randome/page.html Request("http://site.local/wiki/Käyttäjä:"), middleware ) + @deferred_f_from_coro_f + async def test_robotstxt_multiple_reqs(self) -> None: + middleware = RobotsTxtMiddleware(self._get_successful_crawler()) + d1 = deferred_from_coro( + middleware.process_request(Request("http://site.local/allowed1"), None) # type: ignore[arg-type] + ) + d2 = deferred_from_coro( + middleware.process_request(Request("http://site.local/allowed2"), None) # type: ignore[arg-type] + ) + await maybe_deferred_to_future(DeferredList([d1, d2], fireOnOneErrback=True)) + + @pytest.mark.only_asyncio + @deferred_f_from_coro_f + async def test_robotstxt_multiple_reqs_asyncio(self) -> None: + middleware = RobotsTxtMiddleware(self._get_successful_crawler()) + c1 = middleware.process_request(Request("http://site.local/allowed1"), None) # type: ignore[arg-type] + c2 = middleware.process_request(Request("http://site.local/allowed2"), None) # type: ignore[arg-type] + await asyncio.gather(c1, c2) + @deferred_f_from_coro_f async def test_robotstxt_ready_parser(self): middleware = RobotsTxtMiddleware(self._get_successful_crawler()) @@ -157,9 +177,7 @@ Disallow: /some/randome/page.html middleware = RobotsTxtMiddleware(self.crawler) middleware._logerror = mock.MagicMock(side_effect=middleware._logerror) - await maybe_deferred_to_future( - middleware.process_request(Request("http://site.local"), None) - ) + await middleware.process_request(Request("http://site.local"), None) assert middleware._logerror.called @deferred_f_from_coro_f @@ -201,32 +219,39 @@ Disallow: /some/randome/page.html middleware.process_request_2(rp, Request("http://site.local/allowed"), None) rp.allowed.assert_called_once_with("http://site.local/allowed", "Examplebot") - def test_robotstxt_local_file(self): + @deferred_f_from_coro_f + async def test_robotstxt_local_file(self): middleware = RobotsTxtMiddleware(self._get_emptybody_crawler()) - assert not middleware.process_request( + middleware.process_request_2 = mock.MagicMock() + + await middleware.process_request( Request("data:text/plain,Hello World data"), None ) - assert not middleware.process_request( + assert not middleware.process_request_2.called + + await middleware.process_request( Request("file:///tests/sample_data/test_site/nothinghere.html"), None ) - assert isinstance( - middleware.process_request(Request("http://site.local/allowed"), None), - Deferred, - ) + assert not middleware.process_request_2.called + + await middleware.process_request(Request("http://site.local/allowed"), None) + assert middleware.process_request_2.called async def assertNotIgnored( self, request: Request, middleware: RobotsTxtMiddleware ) -> None: spider = None # not actually used - result = await ensure_awaitable(middleware.process_request(request, spider)) # type: ignore[arg-type] - assert result is None + try: + await middleware.process_request(request, spider) # type: ignore[arg-type] + except IgnoreRequest: + pytest.fail("IgnoreRequest was raised unexpectedly") async def assertIgnored( self, request: Request, middleware: RobotsTxtMiddleware ) -> None: spider = None # not actually used with pytest.raises(IgnoreRequest): - await ensure_awaitable(middleware.process_request(request, spider)) # type: ignore[arg-type] + await middleware.process_request(request, spider) # type: ignore[arg-type] def assertRobotsTxtRequested(self, base_url: str) -> None: calls = self.crawler.engine.download_async.call_args_list