From 9d2dea7a8df4f1c2cb59b71bca143148a5306c36 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 10:57:44 +0200 Subject: [PATCH 01/42] Stop skipping the IPv6 resolver tests (#7935) --- .../AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py | 5 ++++- tests/AsyncCrawlerProcess/default_name_resolver.py | 5 ++++- tests/CrawlerProcess/caching_hostname_resolver_ipv6.py | 5 ++++- tests/CrawlerProcess/default_name_resolver.py | 5 ++++- tests/test_crawler_subprocess.py | 7 +------ 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py b/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py index 55d2ef711..8181c4d17 100644 --- a/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py +++ b/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py @@ -8,7 +8,10 @@ class CachingHostnameResolverSpider(scrapy.Spider): """ name = "caching_hostname_resolver_spider" - start_urls = ["http://[::1]"] + + async def start(self): + # w3lib older than 2.4.1 strips the brackets, making the URL invalid. + yield scrapy.Request("http://[::1]", meta={"verbatim_url": True}) if __name__ == "__main__": diff --git a/tests/AsyncCrawlerProcess/default_name_resolver.py b/tests/AsyncCrawlerProcess/default_name_resolver.py index 4c8897f8f..7cc59594b 100644 --- a/tests/AsyncCrawlerProcess/default_name_resolver.py +++ b/tests/AsyncCrawlerProcess/default_name_resolver.py @@ -9,7 +9,10 @@ class IPv6Spider(scrapy.Spider): """ name = "ipv6_spider" - start_urls = ["http://[::1]"] + + async def start(self): + # w3lib older than 2.4.1 strips the brackets, making the URL invalid. + yield scrapy.Request("http://[::1]", meta={"verbatim_url": True}) if __name__ == "__main__": diff --git a/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py b/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py index da9c16cb8..f6f865e3e 100644 --- a/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py +++ b/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py @@ -8,7 +8,10 @@ class CachingHostnameResolverSpider(scrapy.Spider): """ name = "caching_hostname_resolver_spider" - start_urls = ["http://[::1]"] + + async def start(self): + # w3lib older than 2.4.1 strips the brackets, making the URL invalid. + yield scrapy.Request("http://[::1]", meta={"verbatim_url": True}) if __name__ == "__main__": diff --git a/tests/CrawlerProcess/default_name_resolver.py b/tests/CrawlerProcess/default_name_resolver.py index f4c129fdf..12b894030 100644 --- a/tests/CrawlerProcess/default_name_resolver.py +++ b/tests/CrawlerProcess/default_name_resolver.py @@ -9,7 +9,10 @@ class IPv6Spider(scrapy.Spider): """ name = "ipv6_spider" - start_urls = ["http://[::1]"] + + async def start(self): + # w3lib older than 2.4.1 strips the brackets, making the URL invalid. + yield scrapy.Request("http://[::1]", meta={"verbatim_url": True}) if __name__ == "__main__": diff --git a/tests/test_crawler_subprocess.py b/tests/test_crawler_subprocess.py index 733b6797d..240482586 100644 --- a/tests/test_crawler_subprocess.py +++ b/tests/test_crawler_subprocess.py @@ -10,9 +10,7 @@ from pathlib import Path from typing import TYPE_CHECKING import pytest -from packaging.version import parse as parse_version from pexpect.popen_spawn import PopenSpawn -from w3lib import __version__ as w3lib_version from scrapy.utils.asyncio import sleep from tests.utils import get_script_run_env @@ -97,10 +95,6 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): ) assert "RuntimeError" not in log - @pytest.mark.skipif( - parse_version(w3lib_version) >= parse_version("2.0.0"), - reason="w3lib 2.0.0 and later do not allow invalid domains.", - ) def test_ipv6_default_name_resolver(self) -> None: log = self.run_script("default_name_resolver.py") assert "Spider closed (finished)" in log @@ -116,6 +110,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): def test_caching_hostname_resolver_ipv6(self) -> None: log = self.run_script("caching_hostname_resolver_ipv6.py") assert "Spider closed (finished)" in log + assert "http://::1" not in log assert "scrapy.exceptions.CannotResolveHostError" not in log def test_caching_hostname_resolver_finite_execution( From 63485522b619e9f338c1e11dbe5a81368ee618bb Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 10:58:59 +0200 Subject: [PATCH 02/42] Remove the job directory of a download slot once it drains (#7955) --- scrapy/pqueues.py | 7 +++++++ tests/test_pqueues.py | 25 ++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py index 41411ceaf..8e9783f5a 100644 --- a/scrapy/pqueues.py +++ b/scrapy/pqueues.py @@ -2,6 +2,8 @@ from __future__ import annotations import hashlib import logging +from contextlib import suppress +from pathlib import Path from typing import TYPE_CHECKING, Protocol, cast from scrapy.utils.misc import build_from_crawler @@ -409,6 +411,11 @@ class DownloaderAwarePriorityQueue: request = queue.pop() if len(queue) == 0: del self.pqueues[slot] + if self.key: + # Reclaim the slot directory; rmdir leaves it alone if the + # downstream queues did not remove all their files. + with suppress(OSError): + Path(self.key, _path_safe(slot)).rmdir() return request def push(self, request: Request) -> None: diff --git a/tests/test_pqueues.py b/tests/test_pqueues.py index 85fefd172..6ecbb0721 100644 --- a/tests/test_pqueues.py +++ b/tests/test_pqueues.py @@ -6,7 +6,7 @@ import queuelib from scrapy.core.downloader import Downloader from scrapy.http.request import Request -from scrapy.pqueues import DownloaderAwarePriorityQueue, ScrapyPriorityQueue +from scrapy.pqueues import DownloaderAwarePriorityQueue, ScrapyPriorityQueue, _path_safe from scrapy.spiders import Spider from scrapy.squeues import FifoMemoryQueue, PickleFifoDiskQueue from scrapy.utils.misc import build_from_crawler, load_object @@ -258,6 +258,29 @@ class TestDownloaderAwarePriorityQueue: assert "other-slot" not in self.queue +def test_slot_directory_removed_when_slot_drains(tmp_path): + crawler = get_crawler(Spider) + crawler.spider = crawler._create_spider("foo") + crawler.engine = Mock(downloader=MockDownloader()) + queue = DownloaderAwarePriorityQueue.from_crawler( + crawler=crawler, + downstream_queue_cls=PickleFifoDiskQueue, + key=str(tmp_path), + ) + request = Request("https://example.org/1") + slot_dir = tmp_path / _path_safe("example.org") + + queue.push(request) + assert slot_dir.is_dir() + + assert queue.pop().url == request.url + assert not slot_dir.exists() + + queue.push(request) + assert slot_dir.is_dir() + queue.close() + + @pytest.mark.parametrize( ("input_", "output"), [ From 1e92635a188315255ae57c68eb05fa142cfcd713 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:01:06 +0200 Subject: [PATCH 03/42] Benchmark item processing and item concurrency (#7954) --- tests/benchmarks/test_crawl.py | 115 +++++++++++++++++++++++++++++++-- 1 file changed, 110 insertions(+), 5 deletions(-) diff --git a/tests/benchmarks/test_crawl.py b/tests/benchmarks/test_crawl.py index d3bf0fdd6..f79b66a5b 100644 --- a/tests/benchmarks/test_crawl.py +++ b/tests/benchmarks/test_crawl.py @@ -1,5 +1,7 @@ from __future__ import annotations +import asyncio +from collections import Counter from typing import TYPE_CHECKING, Any from urllib.parse import urlencode @@ -29,11 +31,23 @@ LINKS_PER_PAGE = 5 REQUESTS = 200 BROAD_DEEP_PAGES = 10 -# Requests per crawl and delay of the benchmark that measures delayed requests, -# where wall time, unlike in the other benchmarks, is a function of the delay. +# Requests per crawl and delay of the benchmarks that wait, where wall time, +# unlike in the other benchmarks, is a function of the delay. DELAYED_REQUESTS = 50 DELAY = 0.005 +# Requests per crawl and items per response of the benchmarks that measure item +# processing, which reaches fewer pages than the other benchmarks because every +# page costs it several items. +ITEM_REQUESTS = 20 +ITEMS_PER_RESPONSE = 100 + +# Item concurrency limits of the benchmarks that measure item processing. The +# high limit is above the number of items that a response yields in any of +# them. +HIGH_CONCURRENT_ITEMS = 1000 +DELAYED_CONCURRENT_ITEMS = 50 + NULL_SETTINGS: dict[str, Any] = { "DOWNLOAD_HANDLERS": {"http": NullDownloadHandler}, "LOG_ENABLED": False, @@ -63,7 +77,8 @@ class _FollowSpider(Spider): class _TreeSpider(Spider): - """Crawl *pages* pages on each of *domains* hostnames. + """Crawl *pages* pages on each of *domains* hostnames, yielding *items* + items from every page. Pages are numbered from 1, and page *n* links to pages *2n* and *2n+1*, so that requests also reach the scheduler from callbacks, and not only from @@ -73,6 +88,7 @@ class _TreeSpider(Spider): name = "benchmark-tree" domains: int = 1 pages: int = 1 + items: int = 0 async def start(self) -> AsyncIterator[Any]: for domain in range(self.domains): @@ -83,6 +99,8 @@ class _TreeSpider(Spider): for child in (page * 2, page * 2 + 1): if child <= self.pages: yield Request(response.urljoin(f"/{child}")) + for _ in range(self.items): + yield _Page(url=response.url) class _Pipeline: @@ -90,12 +108,48 @@ class _Pipeline: return item -def _crawl_tree(settings: dict[str, Any], *, domains: int, pages: int) -> Crawler: +class _DelayedPipeline: + """Item pipeline that waits, so that the item concurrency limit applies. + + The peak number of items of a same response in progress is tracked in the + ``benchmark/peak_items`` stat. Items are counted per response because the + limit is per response, and the items of a response are processed while + later responses are already being downloaded. + """ + + def __init__(self, crawler: Crawler): + self._crawler = crawler + self._active: Counter[str] = Counter() + + @classmethod + def from_crawler(cls, crawler: Crawler) -> _DelayedPipeline: + return cls(crawler) + + async def process_item(self, item: Any) -> Any: + url = item["url"] + self._active[url] += 1 + assert self._crawler.stats + self._crawler.stats.max_value("benchmark/peak_items", self._active[url]) + try: + await asyncio.sleep(DELAY) + return item + finally: + self._active[url] -= 1 + + +def _crawl_tree( + settings: dict[str, Any], *, domains: int, pages: int, items: int = 0 +) -> Crawler: crawler = crawl( - _TreeSpider, {**NULL_SETTINGS, **settings}, domains=domains, pages=pages + _TreeSpider, + {**NULL_SETTINGS, **settings}, + domains=domains, + pages=pages, + items=items, ) assert crawler.stats assert crawler.stats.get_value("downloader/response_count") == domains * pages + assert crawler.stats.get_value("item_scraped_count", 0) == domains * pages * items return crawler @@ -163,3 +217,54 @@ def test_overhead_delay(benchmark: BenchmarkFixture) -> None: """ settings = {"DOWNLOAD_DELAY": DELAY, "RANDOMIZE_DOWNLOAD_DELAY": False} benchmark(lambda: _crawl_tree(settings, domains=1, pages=DELAYED_REQUESTS)) + + +@pytest.mark.parametrize( + ("items", "settings"), + [ + pytest.param(1, {}, id="single"), + pytest.param(ITEMS_PER_RESPONSE, {}, id="many"), + pytest.param( + 1, + {"CONCURRENT_ITEMS": HIGH_CONCURRENT_ITEMS}, + id="high-limit", + ), + ], +) +def test_overhead_items( + benchmark: BenchmarkFixture, items: int, settings: dict[str, Any] +) -> None: + """Overhead of sending the items of a callback through the item pipeline. + + The single and many scenarios, which use the default + :setting:`CONCURRENT_ITEMS` value, measure how that overhead grows with the + number of items that a response yields. The high-limit scenario instead + raises :setting:`CONCURRENT_ITEMS` well above that number. + """ + benchmark( + lambda: _crawl_tree(settings, domains=1, pages=ITEM_REQUESTS, items=items) + ) + + +def test_overhead_item_concurrency(benchmark: BenchmarkFixture) -> None: + """Overhead of a crawl where item processing waits. + + Every response yields more items than :setting:`CONCURRENT_ITEMS` allows in + parallel, so that the item pipeline gets them in several batches, and wall + time, unlike in most of the other benchmarks, is a function of the delay. + """ + settings = { + "CONCURRENT_ITEMS": DELAYED_CONCURRENT_ITEMS, + "ITEM_PIPELINES": {_DelayedPipeline: 100}, + } + + def run() -> None: + crawler = _crawl_tree( + settings, domains=1, pages=ITEM_REQUESTS, items=ITEMS_PER_RESPONSE + ) + assert crawler.stats + assert ( + crawler.stats.get_value("benchmark/peak_items") == DELAYED_CONCURRENT_ITEMS + ) + + benchmark(run) From 56f4afd84e4a2b5d3c2957332fa79d373cdfbf52 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:03:57 +0200 Subject: [PATCH 04/42] Fix the telnet console shutdown error after a failed start (#7910) --- scrapy/extensions/telnet.py | 8 ++++++-- tests/test_extension_telnet.py | 13 +++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/scrapy/extensions/telnet.py b/scrapy/extensions/telnet.py index 3be24c53f..1506cb1ea 100644 --- a/scrapy/extensions/telnet.py +++ b/scrapy/extensions/telnet.py @@ -52,6 +52,7 @@ class TelnetConsole(protocol.ServerFactory): self.crawler: Crawler = crawler self.noisy: bool = False + self.port: Port | None = None self.portrange: list[int] = [ int(x) for x in crawler.settings.getlist("TELNETCONSOLE_PORT") ] @@ -71,7 +72,7 @@ class TelnetConsole(protocol.ServerFactory): return cls(crawler) def start_listening(self) -> None: - self.port: Port = listen_tcp(self.portrange, self.host, self) + self.port = listen_tcp(self.portrange, self.host, self) h = self.port.getHost() logger.info( "Telnet console listening on %(host)s:%(port)d", @@ -80,7 +81,10 @@ class TelnetConsole(protocol.ServerFactory): ) def stop_listening(self) -> None: - self.port.stopListening() + # The port is unset if start_listening() failed, e.g. because every + # port in TELNETCONSOLE_PORT was taken. + if self.port is not None: + self.port.stopListening() def protocol(self) -> telnet.TelnetTransport: class Portal: diff --git a/tests/test_extension_telnet.py b/tests/test_extension_telnet.py index fca0e3153..cf858e4ea 100644 --- a/tests/test_extension_telnet.py +++ b/tests/test_extension_telnet.py @@ -1,5 +1,6 @@ from __future__ import annotations +import socket from contextlib import contextmanager from typing import TYPE_CHECKING, Any @@ -91,6 +92,18 @@ def test_invalid_reversed_portrange() -> None: console.start_listening() +@coroutine_test +async def test_unavailable_port(caplog: pytest.LogCaptureFixture) -> None: + """Run a crawl where the console cannot bind any port.""" + with socket.create_server(("127.0.0.1", 0)) as sock: + port = sock.getsockname()[1] + crawler = _get_crawler(settings_dict={"TELNETCONSOLE_PORT": [port]}) + await crawler.crawl_async() + + assert "CannotListenError" in caplog.text + assert "AttributeError" not in caplog.text + + @coroutine_test async def test_telnet_vars() -> None: """Log into the console of a running crawl, which is when the telnet From 38f6e3cfd19ca844c0439b982f09a9ce04c30398 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:06:39 +0200 Subject: [PATCH 05/42] Remove the leftover KEEP_ALIVE setting from scrapy shell (#7928) --- scrapy/commands/shell.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 19138ffd0..52be1aadf 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -27,7 +27,6 @@ if TYPE_CHECKING: class Command(ScrapyCommand): default_settings: ClassVar[dict[str, Any]] = { "DUPEFILTER_CLASS": "scrapy.dupefilters.BaseDupeFilter", - "KEEP_ALIVE": True, "LOGSTATS_INTERVAL": 0, } From 5b4888a0b1e3dd5f235c3965f1a9144a868345f1 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:07:23 +0200 Subject: [PATCH 06/42] Document that signal handler order is undefined (#7941) --- docs/topics/item-pipeline.rst | 3 ++- docs/topics/signals.rst | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index c1313635c..35891ce8e 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -49,7 +49,8 @@ Additionally, they may also implement the following methods: .. method:: close_spider(self) - This method is called when the spider is closed. + This method is called when the spider is closed, before the + :signal:`spider_closed` signal is sent. Any of these methods may be defined as a coroutine function (``async def``). diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index f7f9f5cca..ceea2f7c0 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -44,6 +44,15 @@ Here is a simple example showing how you can catch signals and perform some acti def parse(self, response): pass +.. _signal-order: + +Handler order +============= + +The order in which the handlers of a signal run is undefined, and +:ref:`asynchronous handlers ` run concurrently. If two actions +must happen in a given order, run both from a single handler, in that order. + .. _signal-deferred: Asynchronous signal handlers From 81d12c6eb895e2026372dbb54a20e58597c69c5f Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:08:06 +0200 Subject: [PATCH 07/42] Document the Referer caveat of DEFAULT_REQUEST_HEADERS (#7917) --- docs/topics/settings.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 65ee77258..793d65f35 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -658,6 +658,11 @@ The default headers used for Scrapy HTTP Requests. They're populated in the :class:`Request.cookies ` parameter. This is a known current limitation that is being worked on. +.. caution:: A ``Referer`` header defined here only reaches requests for which + :class:`~scrapy.spidermiddlewares.referer.RefererMiddleware` does not set + one, such as start requests. To send it on every request, set + :setting:`REFERRER_POLICY` to ``"no-referrer"``. + .. setting:: DEPTH_LIMIT DEPTH_LIMIT From 4a69e48f0f648bd4509347f70963dd08bf3b28c8 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:08:49 +0200 Subject: [PATCH 08/42] Log the first depth-limited link only (#7916) --- docs/topics/stats.rst | 7 +++++++ scrapy/spidermiddlewares/depth.py | 14 +++++++++----- tests/test_spidermiddleware_depth.py | 17 +++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/docs/topics/stats.rst b/docs/topics/stats.rst index c702cefe7..b558c1cf2 100644 --- a/docs/topics/stats.rst +++ b/docs/topics/stats.rst @@ -121,6 +121,13 @@ one per actual value of the placeholder. :meth:`~scrapy.statscollectors.StatsCollector.get_stats` output is equivalent to a counter of 0. +.. stat:: depth/request_ignored_count + +``depth/request_ignored_count`` + Number of requests dropped for exceeding :setting:`DEPTH_LIMIT`. + + Set by :class:`~scrapy.spidermiddlewares.depth.DepthMiddleware`. + .. stat:: downloader/exception_count ``downloader/exception_count`` diff --git a/scrapy/spidermiddlewares/depth.py b/scrapy/spidermiddlewares/depth.py index 054804119..0131b62e7 100644 --- a/scrapy/spidermiddlewares/depth.py +++ b/scrapy/spidermiddlewares/depth.py @@ -41,6 +41,7 @@ class DepthMiddleware(BaseSpiderMiddleware): self.stats = stats self.verbose_stats = verbose_stats self.prio = prio + self._ignored_logged = False @classmethod def from_crawler(cls, crawler: Crawler) -> Self: @@ -94,11 +95,14 @@ class DepthMiddleware(BaseSpiderMiddleware): if self.prio: request.priority -= depth * self.prio if self.maxdepth and depth > self.maxdepth: - logger.debug( - "Ignoring link (depth > %(maxdepth)d): %(requrl)s ", - {"maxdepth": self.maxdepth, "requrl": request.url}, - extra={"spider": self.crawler.spider}, - ) + if not self._ignored_logged: + logger.debug( + f"Ignoring link (depth > {self.maxdepth}): {request.url}" + " - no more ignored links will be shown", + extra={"spider": self.crawler.spider}, + ) + self._ignored_logged = True + self.stats.inc_value("depth/request_ignored_count") return None if self.verbose_stats: self.stats.inc_value(f"request_depth_count/{depth}") diff --git a/tests/test_spidermiddleware_depth.py b/tests/test_spidermiddleware_depth.py index 32a2ea8f2..2aa76195e 100644 --- a/tests/test_spidermiddleware_depth.py +++ b/tests/test_spidermiddleware_depth.py @@ -85,6 +85,23 @@ async def test_process_spider_output_async_no_response( assert stats.get_value("request_depth_count/0") is None +def test_ignored_logged_once( + mw: DepthMiddleware, stats: StatsCollector, caplog: pytest.LogCaptureFixture +) -> None: + resp = Response("http://example.com") + resp.request = Request("http://example.com") + resp.meta["depth"] = 1 + result = [Request(f"http://example.com/{i}") for i in range(3)] + + with caplog.at_level("DEBUG", logger="scrapy.spidermiddlewares.depth"): + assert not list(mw.process_spider_output(resp, result)) + + messages = [r.getMessage() for r in caplog.records] + assert len(messages) == 1 + assert "http://example.com/0" in messages[0] + assert stats.get_value("depth/request_ignored_count") == 3 + + def test_priority_and_non_verbose_stats() -> None: crawler = get_crawler( Spider, From fc9c505e797591b45f82d0f7d918cd1eceffa5d1 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:12:44 +0200 Subject: [PATCH 09/42] Add URL benchmarks (#7914) --- tests/benchmarks/test_urls.py | 152 ++++++++++++++++++++++++++++++++++ tests/benchmarks/urls.txt | 130 +++++++++++++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 tests/benchmarks/test_urls.py create mode 100644 tests/benchmarks/urls.txt diff --git a/tests/benchmarks/test_urls.py b/tests/benchmarks/test_urls.py new file mode 100644 index 000000000..acc1d4d80 --- /dev/null +++ b/tests/benchmarks/test_urls.py @@ -0,0 +1,152 @@ +from __future__ import annotations + +from html import escape +from pathlib import Path +from typing import TYPE_CHECKING, Any + +import pytest + +from scrapy import Request +from scrapy.http import HtmlResponse +from scrapy.linkextractors import LinkExtractor +from scrapy.utils.request import fingerprint + +if TYPE_CHECKING: + from pytest_codspeed import BenchmarkFixture # type: ignore[import-not-found] + +pytest.importorskip("pytest_codspeed", reason="Benchmarks require pytest-codspeed") + +RESPONSE_URL = "https://www.example.com/catalogue/page-1.html" + +# Links that each scenario returns for the benchmark page. They are fewer than +# the anchors of the page because links to images, to other non-crawlable files +# and to non-HTTP schemes are rejected, and, except in the scenario that keeps +# duplicates, because the links that the navigation repeats are collapsed. +LINKS = 63 +DUPLICATE_LINKS = 88 +CANONICAL_LINKS = 60 +FILTERED_LINKS = 45 + +# Requests built from LINKS links that point to a different resource. +# Canonicalization maps the rest to one that another link already covers, e.g. +# two fragments of a page, or two spellings of one percent-escape. +FINGERPRINTS = 60 + + +def _read_corpus() -> tuple[list[str], list[str]]: + """Return the URLs of ``urls.txt``, and its first group of URLs. + + The first group is the site navigation, which the benchmark page repeats. + """ + groups: list[list[str]] = [[]] + for line in (Path(__file__).parent / "urls.txt").read_text().splitlines(): + line = line.strip() + if not line or line.startswith("#"): + if groups[-1]: + groups.append([]) + continue + groups[-1].append(line) + urls = [url for group in groups for url in group] + return urls, groups[0] + + +def _build_page(urls: list[str], navigation: list[str]) -> bytes: + """Return an HTML page that links to *urls*. + + Every link is surrounded by the markup of a product listing, so that + benchmarks also cover walking over the elements and attributes that a real + page puts between links. + """ + + def item(index: int, url: str) -> str: + href = escape(url) + return ( + f'
  • ' + f'Product {index}' + f'

    Product {index}

    ' + f'

    A description of product {index}.

    ' + f"
  • " + ) + + def nav(urls: list[str]) -> str: + links = "".join(f'{escape(url)}' for url in urls) + return f'' + + items = "".join(item(index, url) for index, url in enumerate(urls)) + return ( + "Catalogue" + f'' + f'{nav(navigation)}
      {items}
    {nav(navigation)}' + "" + ).encode() + + +URLS, NAVIGATION = _read_corpus() +BODY = _build_page(URLS, NAVIGATION) + + +def _response() -> HtmlResponse: + return HtmlResponse(RESPONSE_URL, body=BODY, encoding="utf-8") + + +@pytest.mark.parametrize( + ("kwargs", "links"), + [ + pytest.param({}, LINKS, id="default"), + pytest.param({"unique": False}, DUPLICATE_LINKS, id="duplicates"), + pytest.param({"canonicalize": True}, CANONICAL_LINKS, id="canonicalize"), + pytest.param( + { + "allow": r"/catalogue/", + "deny": r"/legal/", + "allow_domains": ["example.com", "www.example.com"], + }, + FILTERED_LINKS, + id="filtered", + ), + ], +) +def test_extract_links( + benchmark: BenchmarkFixture, kwargs: dict[str, Any], links: int +) -> None: + """Extraction of every link of a page. + + The scenarios cover the choices that change which work dominates: + deduplication and canonicalization both build a key for every link, and the + filters of a configured extractor reject links before the later checks, + which the default extractor reaches for every link. + """ + link_extractor = LinkExtractor(**kwargs) + + def run() -> None: + assert len(link_extractor.extract_links(_response())) == links + + benchmark(run) + + +EXTRACTED_URLS = [link.url for link in LinkExtractor().extract_links(_response())] + + +def test_requests(benchmark: BenchmarkFixture) -> None: + """Building a request for every link of a page.""" + + def run() -> None: + assert len([Request(url) for url in EXTRACTED_URLS]) == LINKS + + benchmark(run) + + +def test_fingerprints(benchmark: BenchmarkFixture) -> None: + """Fingerprinting the request of every link of a page. + + Requests are built here as well, and not once for all rounds, because + fingerprints are cached per request object. + """ + + def run() -> None: + assert ( + len({fingerprint(Request(url)) for url in EXTRACTED_URLS}) == FINGERPRINTS + ) + + benchmark(run) diff --git a/tests/benchmarks/urls.txt b/tests/benchmarks/urls.txt new file mode 100644 index 000000000..6ef6939f1 --- /dev/null +++ b/tests/benchmarks/urls.txt @@ -0,0 +1,130 @@ +# Link targets for the URL benchmarks, as they would appear in the href +# attribute of a page at https://www.example.com/catalogue/page-1.html. +# +# Cost per URL varies by shape: the number of query parameters drives the +# parsing and re-encoding of the query string, non-ASCII characters and +# unescaped characters drive percent-encoding, and non-default ports, dot +# segments and uppercase host names drive normalization. A corpus of uniform +# URLs would therefore measure one shape and miss the others, so this one +# covers each of them, in roughly the proportion of a real listing page. +# +# Blank lines and lines starting with "#" are ignored. + +# Site navigation. These also appear in a second copy of the navigation at the +# end of the page, so that deduplication has duplicates to collapse. +/ +/index.html +/about-us +/contact +/catalogue/ +/catalogue/page-2.html +/catalogue/page-3.html +/help/faq +/help/shipping-and-returns +/legal/terms +/legal/privacy + +# Relative paths of increasing depth. +detail.html +./detail.html +../catalogue/page-4.html +../../index.html +/catalogue/category/books/fiction/index.html +/catalogue/category/books/travel/mystery/historical/index.html +/a/b/c/d/e/f/g/h/i/j/k/index.html + +# One query parameter. +/catalogue/search?q=book +/catalogue/page-1.html?page=2 +/catalogue/detail?id=1042 + +# Several query parameters, in an order that canonicalization changes. +/catalogue/search?q=book&sort=price +/catalogue/search?sort=price&q=book +/catalogue/search?q=book&sort=price&page=3&per_page=20&in_stock=1 +/catalogue/search?zone=eu&q=book&min=10&max=90&sort=rating&page=2&view=grid&lang=en¤cy=EUR&ref=nav + +# Repeated keys, blank values and a bare key. +/catalogue/search?tag=fiction&tag=travel&tag=history +/catalogue/search?q=&sort= +/catalogue/search?featured + +# Characters that need percent-encoding. +/catalogue/search?q=cheap books +/catalogue/detail/a book about books.html +/catalogue/search?q=100%+cotton +/catalogue/search?price=%3E10&title=A%20%26%20B + +# Percent-escapes that are already valid, in both cases. +/catalogue/detail/%C3%A9dition-limit%C3%A9e.html +/catalogue/detail/%c3%a9dition-limit%c3%a9e.html +/catalogue/detail/%7Especial.html + +# Non-ASCII in the path and in the query. +/catalogue/detail/édition-limitée.html +/catalogue/search?q=édition +/catalogue/búsqueda?q=libro&categoría=ficción +/カタログ/詳細.html + +# Internationalized host names, encoded and decoded. +https://例え.テスト/catalogue/page-1.html +https://xn--r8jz45g.xn--zckzah/catalogue/page-2.html + +# Absolute URLs on the same host, on other hosts, and protocol-relative. +https://www.example.com/catalogue/page-5.html +https://www.example.com/catalogue/detail?id=1043 +http://www.example.com/catalogue/page-6.html +https://shop.example.com/catalogue/page-1.html +https://www.example.org/reviews/1042 +https://books.toscrape.com/catalogue/page-1.html +//cdn.example.com/catalogue/page-7.html +//www.example.com/catalogue/page-8.html + +# Ports, including the default one for the scheme. +https://www.example.com:443/catalogue/page-9.html +http://www.example.com:80/catalogue/page-10.html +https://staging.example.com:8443/catalogue/page-1.html + +# Host name case, which normalization lowercases. +https://WWW.EXAMPLE.COM/catalogue/Page-11.html +HTTPS://www.example.com/catalogue/page-12.html + +# Dot segments, empty segments and trailing slashes, which WHATWG +# normalization resolves and the standard library keeps. +/catalogue/../catalogue/page-13.html +/catalogue/./page-14.html +/catalogue//page-15.html +/catalogue/category/ +/catalogue/category + +# Fragments, which canonicalization drops and the deduplication key keeps. +/catalogue/page-16.html#reviews +/catalogue/page-16.html#description +/catalogue/page-17.html# +#top + +# Path parameters, where the semicolon is not the last segment. +/catalogue;sessionid=abc123/page-18.html +/catalogue/page-19.html;sessionid=abc123 + +# User information in the authority. +https://user:password@files.example.com/catalogue/page-1.html + +# A long URL, of the length that tracking parameters reach. +/catalogue/search?q=book&utm_source=newsletter&utm_medium=email&utm_campaign=spring-sale-2026&utm_term=fiction%20paperback&utm_content=hero-banner-variant-b&session=6f1c9a2e4b7d8f0a1c3e5d7b9f2a4c6e&ref=https%3A%2F%2Fwww.example.org%2Freviews%2F1042&page=2&sort=relevance + +# Extensions that the default deny_extensions rejects, and one compound +# extension, which only matches as a whole. +/media/cover-1042.jpg +/media/cover-1042.PNG +/media/catalogue.pdf +/static/style.css +/static/app.js +/downloads/catalogue.tar.gz +/downloads/catalogue.zip + +# Schemes that are not crawlable, which are rejected before any parsing. +mailto:orders@example.com +javascript:void(0) +tel:+441234567890 +data:text/plain,hello From b4279e243bbdf00054bb9c724d9fb4db2766068e Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:13:29 +0200 Subject: [PATCH 10/42] Document that process_value runs before allow and deny (#7940) --- scrapy/linkextractors/lxmlhtml.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 3fb741d7a..46ac39a28 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -282,6 +282,12 @@ class LxmlLinkExtractor: if m: return m.group(1) + ``process_value`` is called before the filtering parameters, such as + ``allow`` and ``deny``, which match the value that it returns. To drop + links based on their final URL, use the ``process_links`` parameter of + :class:`~scrapy.spiders.Rule`, which only receives links that those + parameters kept. + :type process_value: collections.abc.Callable :param strip: whether to strip whitespaces from extracted attributes. From 94dff69468a9e734e10e0feea2674ae1ebf46bf7 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:15:58 +0200 Subject: [PATCH 11/42] Add VCS CI job, fix support for upcoming parsel version (#7924) --- .github/workflows/tests-vcs-deps.yml | 53 ++++++++++++++++++++++++++++ scrapy/selector/unified.py | 13 +++++-- tox.ini | 45 +++++++++++++++++++++++ 3 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/tests-vcs-deps.yml diff --git a/.github/workflows/tests-vcs-deps.yml b/.github/workflows/tests-vcs-deps.yml new file mode 100644 index 000000000..bf867dba7 --- /dev/null +++ b/.github/workflows/tests-vcs-deps.yml @@ -0,0 +1,53 @@ +name: VCS dependencies + +permissions: + contents: read + +on: + schedule: + - cron: '0 4 * * *' + workflow_dispatch: + +concurrency: + group: ${{github.workflow}}-${{ github.ref }} + cancel-in-progress: true + +jobs: + tests: + name: tests + runs-on: ubuntu-latest + env: + PYTEST_ADDOPTS: -n auto --no-cov + TOXENV: vcs-deps + UV_PYTHON_PREFERENCE: only-system + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + + # Dependencies that ship wheels on PyPI are built from source here, so + # their build dependencies are needed: libxml2 and libxslt for lxml, + # libjpeg and zlib for Pillow, and autotools for the libuv bundled in + # uvloop. + - name: Install system libraries + run: | + sudo apt-get update + sudo apt-get install automake libjpeg-dev libtool libxml2-dev libxslt-dev zlib1g-dev + + - name: Set up uv + uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + with: + cache-dependency-glob: | + pyproject.toml + tox.ini + + - name: Install mitmproxy + run: uv tool install --python cpython mitmproxy + + - name: Run tests + run: uvx --with tox-uv tox diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index f6334c32c..fa91e2904 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -46,8 +46,10 @@ class Selector(_ParselSelector, object_ref): ``"json"``, ``"text"`` or ``None`` (default). It's passed to :class:`parsel.Selector` and its meaning is defined there. However, when ``type`` is ``None``, it is set to ``"xml"`` for an - :class:`~scrapy.http.XmlResponse` and to ``"html"`` otherwise before - passing it to :class:`parsel.Selector`. + :class:`~scrapy.http.XmlResponse` and to ``"html"`` for an + :class:`~scrapy.http.HtmlResponse` or for ``text`` before passing it to + :class:`parsel.Selector`, which for any other response is left to + determine the type from the response body. .. note:: JSON selector support requires ``parsel`` 1.8.0 or higher. With older versions setting ``type`` to ``"json"`` or ``"text"`` is not @@ -70,8 +72,13 @@ class Selector(_ParselSelector, object_ref): f"{self.__class__.__name__}.__init__() received both response and text" ) + # A response that is neither HTML nor XML, e.g. a JSON one, keeps type + # unset, so that parsel determines it from the body. if type is None: - type = "xml" if isinstance(response, XmlResponse) else "html" # noqa: A001 + if isinstance(response, XmlResponse): + type = "xml" # noqa: A001 + elif response is None or isinstance(response, HtmlResponse): + type = "html" # noqa: A001 if text is not None: response = _response_from_text(text, type) diff --git a/tox.ini b/tox.ini index c56ad011b..94c04f75f 100644 --- a/tox.ini +++ b/tox.ini @@ -201,6 +201,51 @@ setenv = {[min]setenv} commands = {[min]commands} +[testenv:vcs-deps] +basepython = python3 +deps = + {[testenv:extra-deps]deps} + uv +# Dependencies cap each other at their latest release, so their development +# branches usually cannot be resolved together: pyOpenSSL, for one, requires a +# cryptography older than the one cryptography itself is heading towards. +# --no-deps skips resolution entirely, replacing only these distributions and +# leaving the rest of the environment as the install above resolved it. +# +# Pillow and uvloop build from source, and need the libjpeg headers and +# autotools respectively. robotexclusionrulesparser has no public repository, +# so it stays at its latest release. +commands_pre = + uv pip install --python {envpython} --no-deps --reinstall \ + git+https://github.com/twisted/twisted \ + git+https://github.com/python-pillow/Pillow \ + git+https://github.com/MagicStack/uvloop \ + git+https://github.com/pyca/cryptography \ + git+https://github.com/scrapy/cssselect \ + git+https://github.com/tiran/defusedxml \ + git+https://github.com/scrapy/itemadapter \ + git+https://github.com/scrapy/itemloaders \ + git+https://github.com/lxml/lxml \ + git+https://github.com/pypa/packaging \ + git+https://github.com/scrapy/parsel \ + git+https://github.com/scrapy/protego \ + git+https://github.com/pyca/pyopenssl \ + git+https://github.com/scrapy/queuelib \ + git+https://github.com/pyca/service-identity \ + git+https://github.com/john-kurkowski/tldextract \ + git+https://github.com/scrapy/w3lib \ + git+https://github.com/zopefoundation/zope.interface \ + git+https://github.com/mcfletch/pydispatcher \ + git+https://github.com/boto/boto3 \ + git+https://github.com/bpython/bpython \ + git+https://github.com/google/brotli \ + git+https://github.com/python-hyper/brotlicffi \ + git+https://github.com/googleapis/python-storage \ + git+https://github.com/pydantic/httpx2\#subdirectory=src/httpx2 \ + git+https://github.com/ipython/ipython \ + git+https://github.com/prompt-toolkit/ptpython \ + git+https://github.com/indygreg/python-zstandard + [testenv:default-reactor] commands = {[testenv]commands} --reactor=default From 0e324f3d4a5576c51423c506ef554db5717ce603 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:17:44 +0200 Subject: [PATCH 12/42] Let spiders change allowed_domains at run time (#7912) --- docs/topics/spiders.rst | 7 ++++ scrapy/downloadermiddlewares/offsite.py | 11 ++++- tests/test_downloadermiddleware_offsite.py | 47 ++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 8fbf0c52d..f2cfeb712 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -59,9 +59,16 @@ scrapy.Spider :class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware` is enabled. + .. versionchanged:: VERSION + Changes to this attribute during a crawl are now taken into account. + Let's say your target url is ``https://www.example.com/1.html``, then add ``'example.com'`` to the list. + You may modify this attribute while the spider runs, e.g. to allow + domains that you only learn about from an earlier response. The change + affects requests scheduled after it. + .. autoattribute:: start_urls .. attribute:: custom_settings diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py index db85b62a1..28c0e09cb 100644 --- a/scrapy/downloadermiddlewares/offsite.py +++ b/scrapy/downloadermiddlewares/offsite.py @@ -22,10 +22,12 @@ logger = logging.getLogger(__name__) class OffsiteMiddleware: crawler: Crawler + host_regex: re.Pattern[str] def __init__(self, stats: StatsCollector): self.stats = stats self.domains_seen: set[str] = set() + self._allowed_domains: list[str] | None = None @classmethod def from_crawler(cls, crawler: Crawler) -> Self: @@ -37,7 +39,13 @@ class OffsiteMiddleware: return o def spider_opened(self, spider: Spider) -> None: - self.host_regex: re.Pattern[str] = self.get_host_regex(spider) + self._update_host_regex(spider) + + def _update_host_regex(self, spider: Spider) -> None: + allowed_domains = list(getattr(spider, "allowed_domains", None) or []) + if allowed_domains != self._allowed_domains: + self._allowed_domains = allowed_domains + self.host_regex = self.get_host_regex(spider) def request_scheduled(self, request: Request, spider: Spider) -> None: self.process_request(request) @@ -64,6 +72,7 @@ class OffsiteMiddleware: raise IgnoreRequest(f"Filtered offsite request to {domain!r}") def should_follow(self, request: Request, spider: Spider) -> bool: + self._update_host_regex(spider) regex = self.host_regex # hostname can be None for wrong urls (like javascript links) host = urlparse_cached(request).hostname or "" diff --git a/tests/test_downloadermiddleware_offsite.py b/tests/test_downloadermiddleware_offsite.py index cb17c2553..bab89814f 100644 --- a/tests/test_downloadermiddleware_offsite.py +++ b/tests/test_downloadermiddleware_offsite.py @@ -247,3 +247,50 @@ def test_ignore_request_reason(): IgnoreRequest, match=re.escape("Filtered offsite request to 'other.org'") ): mw.process_request(request) + + +class DomainSpider(Spider): + name = "a" + allowed_domains: list[str] + + +def test_dynamic_allowed_domains(): + crawler = get_crawler(DomainSpider) + spider = DomainSpider.from_crawler(crawler, allowed_domains=["a.example"]) + crawler.spider = spider + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + + with pytest.raises(IgnoreRequest): + mw.process_request(Request("https://b.example")) + + spider.allowed_domains.append("b.example") + assert mw.process_request(Request("https://b.example")) is None + + spider.allowed_domains.remove("a.example") + with pytest.raises(IgnoreRequest): + mw.process_request(Request("https://a.example")) + + +def test_dynamic_allowed_domains_caching(): + calls = 0 + + class TrackingMiddleware(OffsiteMiddleware): + def get_host_regex(self, spider: Spider) -> re.Pattern[str]: + nonlocal calls + calls += 1 + return super().get_host_regex(spider) + + crawler = get_crawler(DomainSpider) + spider = DomainSpider.from_crawler(crawler, allowed_domains=["a.example"]) + crawler.spider = spider + mw = TrackingMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + + for _ in range(3): + mw.process_request(Request("https://a.example")) + assert calls == 1 + + spider.allowed_domains.append("b.example") + assert mw.process_request(Request("https://b.example")) is None + assert calls == 2 From 0b2d220197fb7d9b274738cd812cb7d953f85c49 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:19:50 +0200 Subject: [PATCH 13/42] Improve docs for multi-spider runs (#7907) --- docs/topics/practices.rst | 12 ++++++++++++ docs/topics/settings.rst | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index dfa1e21f6..aeaf322c2 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -458,6 +458,18 @@ finishes before starting the next one: should not have a different value per spider, and :ref:`pre-crawler settings ` cannot be defined per spider. +Every other setting applies to each crawler separately. This includes +concurrency and politeness settings, such as :setting:`CONCURRENT_REQUESTS`, +:setting:`CONCURRENT_REQUESTS_PER_DOMAIN` and :setting:`DOWNLOAD_DELAY`, and +:ref:`AutoThrottle ` also throttles each crawler +separately. When crawling simultaneously, divide those values by the number of +crawlers to keep the combined load on your hardware and on target websites +unchanged. + +Because of this, running the same spider several times in the same process +multiplies those limits instead of increasing crawling capacity. To crawl +faster, raise :setting:`CONCURRENT_REQUESTS` on a single crawler. + .. seealso:: :ref:`run-from-script`. .. skip: end diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 793d65f35..dc09fdeeb 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -754,6 +754,11 @@ Default: ``60`` Timeout for processing of DNS queries in seconds. Float is supported. +The timeout starts when the query is queued into the Twisted reactor thread +pool, not when it is sent. If that thread pool is saturated, queries can time +out before being sent, in which case increasing +:setting:`REACTOR_THREADPOOL_MAXSIZE` helps more than increasing this setting. + .. note:: This setting is only used by :class:`~scrapy.resolver.CachingThreadedResolver`. It has no effect when From 9e84112221133f6a36dac1e0e6072a39ddfb2fd2 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:25:14 +0200 Subject: [PATCH 14/42] Cover update_vars() in the shell docs (#7889) --- docs/topics/shell.rst | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/topics/shell.rst b/docs/topics/shell.rst index 6f7e67cf9..42c5bd169 100644 --- a/docs/topics/shell.rst +++ b/docs/topics/shell.rst @@ -144,6 +144,32 @@ Those objects are: - ``settings`` - the current :ref:`Scrapy settings ` +.. _shell-update-vars: + +Adding your own objects +----------------------- + +To define additional objects, or to run code every time a response is fetched, +write a :ref:`custom project command ` in a module called +``shell``, which overrides the :command:`shell` command, and override its +``update_vars`` method. It is called on start and after every ``fetch``, and it +receives the mapping of variable names to objects: + +.. code-block:: python + + from scrapy.commands.shell import Command as ShellCommand + + + class Command(ShellCommand): + def update_vars(self, vars): + from myproject.utils import parse_product + + vars["parse_product"] = parse_product + if vars["response"] is not None: + vars["product"] = parse_product(vars["response"]) + +``response`` is ``None`` when the shell is started without a URL. + Example of shell session ======================== From f123c7a1cc9974c85450f45bff6d18ac49056ed5 Mon Sep 17 00:00:00 2001 From: Mridankan Mandal Date: Sun, 9 Aug 2026 15:25:58 +0530 Subject: [PATCH 15/42] Simplify cmdline settings test (#7853) Signed-off-by: Mridankan Mandal --- tests/test_cmdline/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/test_cmdline/__init__.py b/tests/test_cmdline/__init__.py index 98a85bc17..f6ebe5865 100644 --- a/tests/test_cmdline/__init__.py +++ b/tests/test_cmdline/__init__.py @@ -1,3 +1,4 @@ +import ast import json import os import pstats @@ -60,11 +61,8 @@ class TestCmdline: "-s", "EXTENSIONS=" + json.dumps(EXTENSIONS), ) - # XXX: There's gotta be a smarter way to do this... assert "..." not in settingsstr - for char in ("'", "<", ">"): - settingsstr = settingsstr.replace(char, '"') - settingsdict = json.loads(settingsstr) + settingsdict = ast.literal_eval(settingsstr) assert set(settingsdict.keys()) == set(EXTENSIONS.keys()) assert settingsdict[EXT_PATH] == 200 From cd7f422a1e477039ba40e96979b5f060030ac702 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 11:57:11 +0200 Subject: [PATCH 16/42] Use client.bucket() in GCSFeedStorage so object-level GCS permissions suffice (#7945) --- scrapy/extensions/feedexport.py | 2 +- tests/test_feedexport_storages.py | 4 ++-- tests/utils/cloud.py | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 118462bc9..fc2b2f43f 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -340,7 +340,7 @@ class GCSFeedStorage(BlockingFeedStorage): from google.cloud.storage import Client # noqa: PLC0415 client = Client(project=self.project_id) - bucket = client.get_bucket(self.bucket_name) + bucket = client.bucket(self.bucket_name) blob = bucket.blob(self.blob_name) blob.upload_from_file(file, predefined_acl=self.acl) finally: diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index 4d28872b7..6f9e33449 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -524,7 +524,7 @@ class TestGCSFeedStorage: f.seek.assert_called_once_with(0) m.assert_called_once_with(project=project_id) - client_mock.get_bucket.assert_called_once_with("mybucket") + client_mock.bucket.assert_called_once_with("mybucket") bucket_mock.blob.assert_called_once_with("export.csv") blob_mock.upload_from_file.assert_called_once_with(f, predefined_acl=acl) f.close.assert_called_once_with() @@ -548,7 +548,7 @@ class TestGCSFeedStorage: f.seek.assert_called_once_with(0) m.assert_called_once_with(project=project_id) - client_mock.get_bucket.assert_called_once_with("mybucket") + client_mock.bucket.assert_called_once_with("mybucket") bucket_mock.blob.assert_called_once_with("export.csv") blob_mock.upload_from_file.assert_called_once_with(f, predefined_acl=acl) f.close.assert_called_once_with() diff --git a/tests/utils/cloud.py b/tests/utils/cloud.py index 662e0b3ee..4e253fbdc 100644 --- a/tests/utils/cloud.py +++ b/tests/utils/cloud.py @@ -14,7 +14,6 @@ def mock_google_cloud_storage() -> tuple[Any, Any, Any]: bucket_mock = mock.create_autospec(Bucket) client_mock.bucket.return_value = bucket_mock - client_mock.get_bucket.return_value = bucket_mock blob_mock = mock.create_autospec(Blob) bucket_mock.blob.return_value = blob_mock From a18d58d7b50e22c8356aef3efaf50ae36e6b18a2 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 12:26:15 +0200 Subject: [PATCH 17/42] Document that process_spider_output receives a lazy result (#7939) --- docs/topics/spider-middleware.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index aa14f6801..78b211dac 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -122,6 +122,9 @@ one or more of these methods: This method is an :term:`asynchronous generator` called with the results from the spider after the spider has processed the response. + *result* is lazy: a generator callback runs as *result* is iterated, so + code that runs before that iteration runs before the callback body. + .. seealso:: :ref:`universal-spider-middleware`. :param response: the response which generated this output from the From 63d5ce6272a7a6596672d6187a0c56c819782deb Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 13:52:32 +0200 Subject: [PATCH 18/42] Fix cookie lookup for dotless hosts and IP addresses (#7900) --- scrapy/http/cookies.py | 4 ++-- tests/test_downloadermiddleware_cookies.py | 24 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/scrapy/http/cookies.py b/scrapy/http/cookies.py index 8edeae01c..555d930e6 100644 --- a/scrapy/http/cookies.py +++ b/scrapy/http/cookies.py @@ -54,9 +54,9 @@ class CookieJar: if not IPV4_RE.search(req_host): hosts = potential_domain_matches(req_host) if "." not in req_host: - hosts.append(req_host + ".local") + hosts += potential_domain_matches(req_host + ".local") else: - hosts = [req_host] + hosts = [req_host, "." + req_host] cookies = [] for host in hosts: diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 8d999d952..e4b66fe10 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -345,6 +345,30 @@ class TestCookiesMiddleware: assert "Cookie" in request.headers assert request.headers["Cookie"] == b"currencyCookie=USD" + @pytest.mark.parametrize( + ("url", "domain"), + [ + ("http://example-host/", "example-host.local"), + ("http://127.0.0.1/", "127.0.0.1"), + pytest.param( + "http://example-host/", + "example-host", + marks=pytest.mark.xfail( + reason=( + "http.cookiejar accepts a dotless domain for a dotless " + "host but never returns the resulting cookie" + ) + ), + ), + ], + ) + def test_explicit_local_domain(self, url: str, domain: str) -> None: + request = Request( + url, cookies=[{"name": "currencyCookie", "value": "USD", "domain": domain}] + ) + assert self.mw.process_request(request) is None + assert request.headers.get("Cookie") == b"currencyCookie=USD" + @pytest.mark.xfail(reason="Cookie header is not currently being processed") def test_keep_cookie_from_default_request_headers_middleware(self): DEFAULT_REQUEST_HEADERS = {"Cookie": "default=value; asdf=qwerty"} From 7c797968a31ecacc24e17b81e57aad44de803389 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 13:53:33 +0200 Subject: [PATCH 19/42] Docs: clarify the handling of exceptions raised in errbacks (#7898) --- docs/topics/request-response.rst | 4 ++++ docs/topics/spider-middleware.rst | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 75158440b..f810146e4 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -770,6 +770,10 @@ is raise while processing it. It receives a :exc:`~twisted.python.failure.Failure` as first parameter and can be used to track connection establishment timeouts, DNS errors etc. +If an errback raises an exception, Scrapy logs it and sends the +:signal:`spider_error` signal, unless the exception is the one that the errback +received, which Scrapy logs as a download error instead. + Here's an example spider logging all errors and catching some specific errors if needed: diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 78b211dac..db85906c1 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -145,8 +145,9 @@ one or more of these methods: .. method:: process_spider_exception(response, exception) - This method is called when a spider or :meth:`process_spider_output` - method (from a previous spider middleware) raises an exception. + This method is called when a spider callback or a + :meth:`process_spider_output` method (from a previous spider + middleware) raises an exception. :meth:`process_spider_exception` should return either ``None`` or an iterable of :class:`~scrapy.Request` or :ref:`item ` From 59ce27afddd4c30e72c4412c9b2b7a41183249a1 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 13:55:02 +0200 Subject: [PATCH 20/42] Send the bytes_received and headers_received signals over HTTP/2 (#7896) --- docs/topics/download-handlers.rst | 3 - scrapy/core/downloader/handlers/http2.py | 2 +- scrapy/core/http2/agent.py | 10 +-- scrapy/core/http2/protocol.py | 18 +++-- scrapy/core/http2/stream.py | 74 +++++++++++++++---- .../test_downloader_handler_twisted_http2.py | 12 --- tests/test_http2_client_protocol.py | 4 +- 7 files changed, 76 insertions(+), 47 deletions(-) diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index e0501c169..94e75ab6f 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -203,9 +203,6 @@ Other limitations: - IPv6 support requires setting :setting:`TWISTED_DNS_RESOLVER` to ``scrapy.resolver.CachingHostnameResolver``. -- No support for the :signal:`bytes_received` and :signal:`headers_received` - signals. - Known limitations of the HTTP/2 support: - No support for HTTP/2 Cleartext (h2c), since no major browser supports diff --git a/scrapy/core/downloader/handlers/http2.py b/scrapy/core/downloader/handlers/http2.py index f60c58d1b..9b3d4fbd4 100644 --- a/scrapy/core/downloader/handlers/http2.py +++ b/scrapy/core/downloader/handlers/http2.py @@ -40,7 +40,7 @@ class H2DownloadHandler(BaseDownloadHandler): from twisted.internet import reactor - self._pool = H2ConnectionPool(reactor, crawler.settings) + self._pool = H2ConnectionPool(reactor, crawler) self._context_factory = _load_context_factory_from_settings(crawler) self._bind_address = crawler.settings.get("DOWNLOAD_BIND_ADDRESS") diff --git a/scrapy/core/http2/agent.py b/scrapy/core/http2/agent.py index aa55e29a0..042557208 100644 --- a/scrapy/core/http2/agent.py +++ b/scrapy/core/http2/agent.py @@ -21,8 +21,8 @@ if TYPE_CHECKING: from twisted.internet.base import ReactorBase from twisted.internet.endpoints import HostnameEndpoint + from scrapy.crawler import Crawler from scrapy.http import Request, Response - from scrapy.settings import Settings from scrapy.spiders import Spider @@ -30,9 +30,9 @@ ConnectionKeyT = tuple[bytes, bytes, int] class H2ConnectionPool: - def __init__(self, reactor: ReactorBase, settings: Settings) -> None: + def __init__(self, reactor: ReactorBase, crawler: Crawler) -> None: self._reactor = reactor - self.settings = settings + self._crawler = crawler # Store a dictionary which is used to get the respective # H2ClientProtocolInstance using the key as Tuple(scheme, hostname, port) @@ -43,7 +43,7 @@ class H2ConnectionPool: ConnectionKeyT, deque[Deferred[H2ClientProtocol]] ] = {} - self._tls_verbose_logging: bool = settings.getbool( + self._tls_verbose_logging: bool = crawler.settings.getbool( "DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING" ) @@ -77,7 +77,7 @@ class H2ConnectionPool: factory = H2ClientFactory( uri, - self.settings, + self._crawler, conn_lost_deferred, tls_verbose_logging=self._tls_verbose_logging, ) diff --git a/scrapy/core/http2/protocol.py b/scrapy/core/http2/protocol.py index 7136e829e..2d59aba31 100644 --- a/scrapy/core/http2/protocol.py +++ b/scrapy/core/http2/protocol.py @@ -44,7 +44,7 @@ if TYPE_CHECKING: from twisted.python.failure import Failure from twisted.web.client import URI - from scrapy.settings import Settings + from scrapy.crawler import Crawler from scrapy.spiders import Spider @@ -90,7 +90,7 @@ class H2ClientProtocol(Protocol, TimeoutMixin): def __init__( self, uri: URI, - settings: Settings, + crawler: Crawler, conn_lost_deferred: Deferred[list[BaseException]], *, tls_verbose_logging: bool = False, @@ -100,11 +100,12 @@ class H2ClientProtocol(Protocol, TimeoutMixin): uri -- URI of the base url to which HTTP/2 Connection will be made. uri is used to verify that incoming client requests have correct base URL. - settings -- Scrapy project settings + crawler -- The crawler the requests belong to conn_lost_deferred -- Deferred that fires with the list of underlying exceptions to notify that connection was lost tls_verbose_logging -- Whether to log TLS details """ + self._crawler: Crawler = crawler self._conn_lost_deferred: Deferred[list[BaseException]] = conn_lost_deferred self._tls_verbose_logging: bool = tls_verbose_logging @@ -140,8 +141,8 @@ class H2ClientProtocol(Protocol, TimeoutMixin): # Both ip_address and uri are used by the Stream before # initiating the request to verify that the base address # Variables taken from Project Settings - "default_download_maxsize": settings.getint("DOWNLOAD_MAXSIZE"), - "default_download_warnsize": settings.getint("DOWNLOAD_WARNSIZE"), + "default_download_maxsize": crawler.settings.getint("DOWNLOAD_MAXSIZE"), + "default_download_warnsize": crawler.settings.getint("DOWNLOAD_WARNSIZE"), # Counter to keep track of opened streams. This counter # is used to make sure that not more than MAX_CONCURRENT_STREAMS # streams are opened which leads to ProtocolError @@ -208,6 +209,7 @@ class H2ClientProtocol(Protocol, TimeoutMixin): stream_id=next(self._stream_id_generator), request=request, protocol=self, + crawler=self._crawler, download_maxsize=getattr( spider, "download_maxsize", self.metadata["default_download_maxsize"] ), @@ -461,20 +463,20 @@ class H2ClientFactory(Factory): def __init__( self, uri: URI, - settings: Settings, + crawler: Crawler, conn_lost_deferred: Deferred[list[BaseException]], *, tls_verbose_logging: bool = False, ) -> None: self.uri = uri - self.settings = settings + self.crawler = crawler self.conn_lost_deferred = conn_lost_deferred self.tls_verbose_logging = tls_verbose_logging def buildProtocol(self, addr: IAddress) -> H2ClientProtocol: return H2ClientProtocol( self.uri, - self.settings, + self.crawler, self.conn_lost_deferred, tls_verbose_logging=self.tls_verbose_logging, ) diff --git a/scrapy/core/http2/stream.py b/scrapy/core/http2/stream.py index c6226bbca..4fc300d90 100644 --- a/scrapy/core/http2/stream.py +++ b/scrapy/core/http2/stream.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from contextlib import suppress from enum import Enum from io import BytesIO from typing import TYPE_CHECKING, Any @@ -12,9 +13,11 @@ from twisted.internet.error import ConnectionClosed from twisted.python.failure import Failure from twisted.web.client import ResponseFailed -from scrapy.exceptions import DownloadCancelledError +from scrapy import signals +from scrapy.exceptions import DownloadCancelledError, StopDownload from scrapy.http.headers import Headers from scrapy.utils._download_handlers import ( + check_stop_download, get_maxsize_msg, get_warnsize_msg, make_response, @@ -25,6 +28,7 @@ if TYPE_CHECKING: from collections.abc import Sequence from scrapy.core.http2.protocol import H2ClientProtocol + from scrapy.crawler import Crawler from scrapy.http import Request, Response @@ -82,6 +86,9 @@ class StreamCloseReason(Enum): # Actual response body size is more than allowed limit MAXSIZE_EXCEEDED_ACTUAL = 8 + # A signal handler raised StopDownload + STOP_DOWNLOAD = 9 + class Stream: """Represents a single HTTP/2 Stream. @@ -99,6 +106,7 @@ class Stream: stream_id: int, request: Request, protocol: H2ClientProtocol, + crawler: Crawler, download_maxsize: int = 0, download_warnsize: int = 0, ) -> None: @@ -107,10 +115,13 @@ class Stream: stream_id -- Unique identifier for the stream within a single HTTP/2 connection request -- The HTTP request associated to the stream protocol -- Parent H2ClientProtocol instance + crawler -- The crawler the request belongs to """ self.stream_id: int = stream_id self._request: Request = request self._protocol: H2ClientProtocol = protocol + self._crawler: Crawler = crawler + self._stop_download: StopDownload | None = None self._download_maxsize = self._request.meta.get( "download_maxsize", download_maxsize @@ -338,6 +349,13 @@ class Stream: self._response["body"].write(data) self._response["flow_controlled_size"] += flow_controlled_length + if stop_download := check_stop_download( + signals.bytes_received, self._crawler, self._request, data=data + ): + self._stop_download = stop_download + self.reset_stream(StreamCloseReason.STOP_DOWNLOAD) + return + # We check maxsize here in case the Content-Length header was not received if ( self._download_maxsize @@ -369,8 +387,20 @@ class Stream: else: self._response["headers"].appendlist(name, value) - # Check if we exceed the allowed max data size which can be received expected_size = int(self._response["headers"].get(b"Content-Length", -1)) + + if stop_download := check_stop_download( + signals.headers_received, + self._crawler, + self._request, + headers=self._response["headers"], + body_length=expected_size if expected_size >= 0 else None, + ): + self._stop_download = stop_download + self.reset_stream(StreamCloseReason.STOP_DOWNLOAD) + return + + # Check if we exceed the allowed max data size which can be received if self._download_maxsize and expected_size > self._download_maxsize: self.reset_stream(StreamCloseReason.MAXSIZE_EXCEEDED) return @@ -387,11 +417,18 @@ class Stream: if self.metadata["stream_closed_local"]: raise StreamClosedError(self.stream_id) - # Clear buffer earlier to avoid keeping data in memory for a long time - self._response["body"].truncate(0) + # The data received so far is the body of the response built for a + # stopped download, otherwise the buffer is cleared early to avoid + # keeping data in memory for a long time + if reason is not StreamCloseReason.STOP_DOWNLOAD: + self._response["body"].truncate(0) self.metadata["stream_closed_local"] = True - self._protocol.conn.reset_stream(self.stream_id, ErrorCodes.REFUSED_STREAM) + # The remote peer may have ended the stream already, e.g. because the + # whole response arrived within the data that triggered this reset, in + # which case there is nothing left to reset + with suppress(StreamClosedError): + self._protocol.conn.reset_stream(self.stream_id, ErrorCodes.REFUSED_STREAM) self.close(reason) def close( @@ -444,7 +481,7 @@ class Stream: logger.error(error_msg) self._deferred_response.errback(DownloadCancelledError(error_msg)) - elif reason is StreamCloseReason.ENDED: + elif reason in {StreamCloseReason.ENDED, StreamCloseReason.STOP_DOWNLOAD}: self._fire_response_deferred() # Stream was abruptly ended here @@ -495,13 +532,18 @@ class Stream: and fires the response deferred callback with the generated response instance""" - response = make_response( - url=self._request.url, - status=self._response["status"], - headers=self._response["headers"], - body=self._response["body"].getvalue(), - certificate=self._protocol.metadata["certificate"], - ip_address=self._protocol.metadata["ip_address"], - protocol="h2", - ) - self._deferred_response.callback(response) + try: + response = make_response( + url=self._request.url, + status=self._response["status"], + headers=self._response["headers"], + body=self._response["body"].getvalue(), + certificate=self._protocol.metadata["certificate"], + ip_address=self._protocol.metadata["ip_address"], + protocol="h2", + stop_download=self._stop_download, + ) + except StopDownload as exc: + self._deferred_response.errback(exc) + else: + self._deferred_response.callback(response) diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 449f2d635..9d4e161f3 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -186,18 +186,6 @@ class TestHttp2TLSVersion(H2DownloadHandlerMixin, TestHttpsTLSVersionBase): class TestHttp2WithCrawler(H2DownloadHandlerMixin, TestHttpWithCrawlerBase): is_secure = True - def test_bytes_received_stop_download_callback(self) -> None: # type: ignore[override] - pytest.skip("bytes_received support is not implemented") - - def test_bytes_received_stop_download_errback(self) -> None: # type: ignore[override] - pytest.skip("bytes_received support is not implemented") - - def test_headers_received_stop_download_callback(self) -> None: # type: ignore[override] - pytest.skip("headers_received support is not implemented") - - def test_headers_received_stop_download_errback(self) -> None: # type: ignore[override] - pytest.skip("headers_received support is not implemented") - @pytest.mark.skip(reason="Proxy support is not implemented yet") class TestHttp2Proxy(H2DownloadHandlerMixin, TestHttpProxyBase): diff --git a/tests/test_http2_client_protocol.py b/tests/test_http2_client_protocol.py index b8586d1ca..431b7a458 100644 --- a/tests/test_http2_client_protocol.py +++ b/tests/test_http2_client_protocol.py @@ -23,13 +23,13 @@ from twisted.web.static import File from scrapy.exceptions import DownloadCancelledError, DownloadTimeoutError from scrapy.http import JsonRequest, Request, Response -from scrapy.settings import Settings from scrapy.spiders import Spider from scrapy.utils.defer import ( deferred_f_from_coro_f, deferred_from_coro, maybe_deferred_to_future, ) +from scrapy.utils.test import get_crawler from tests.mockserver.http_resources import LeafResource, Status, put_child from tests.mockserver.utils import ssl_context_factory @@ -250,7 +250,7 @@ class TestHttps2ClientProtocol: acceptableProtocols=[b"h2"], ) uri = URI.fromBytes(bytes(self.get_url(server_port, "/"), "utf-8")) - h2_client_factory = H2ClientFactory(uri, Settings(), Deferred()) + h2_client_factory = H2ClientFactory(uri, get_crawler(), Deferred()) client_endpoint = SSL4ClientEndpoint( reactor, self.host, server_port, client_options ) From 050a8cf159a6b0c0c7d4ba08b98d957c0ab3416d Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 13:57:08 +0200 Subject: [PATCH 21/42] Improve the docs about delaying start request iteration (#7883) --- docs/topics/broad-crawls.rst | 5 +++-- docs/topics/signals.rst | 9 +++++++++ docs/topics/spiders.rst | 12 ++++++------ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/docs/topics/broad-crawls.rst b/docs/topics/broad-crawls.rst index cace1f883..d6b9fd6f9 100644 --- a/docs/topics/broad-crawls.rst +++ b/docs/topics/broad-crawls.rst @@ -182,8 +182,9 @@ Be mindful of memory leaks ========================== If your broad crawl shows a high memory usage, in addition to :ref:`crawling in -BFO order ` and :ref:`lowering concurrency -` you should :ref:`debug your memory leaks +BFO order `, :ref:`lowering concurrency +` and :ref:`delaying start request iteration +` you should :ref:`debug your memory leaks `. diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index ceea2f7c0..f060710a4 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -158,6 +158,15 @@ scheduler_empty See :ref:`start-requests-lazy` for an example. + .. warning:: Only wait for this signal from + :meth:`~scrapy.Spider.start`. While no request can be sent, e.g. while + the responses being parsed exceed + :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE`, the engine does not ask the + scheduler for requests, and hence this signal is not sent. So waiting + for it from a :ref:`callback ` can hang the crawl, + because the response being parsed is itself one of the responses that + may be blocking requests. + This signal does not support :ref:`asynchronous handlers `. diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index f2cfeb712..e68c208b7 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -396,8 +396,12 @@ Start requests Delaying start request iteration -------------------------------- -You can override the :meth:`~scrapy.Spider.start` method as follows to pause -its iteration whenever there are scheduled requests: +Scrapy iterates :meth:`~scrapy.Spider.start` as fast as it yields, so all start +requests reach the scheduler early in the crawl, however many they are. To +minimize the number of requests in the scheduler at any given time, and with it +resource usage (memory, or disk when using :setting:`JOBDIR`), override +:meth:`~scrapy.Spider.start` to pause its iteration whenever there are +scheduled requests: .. code-block:: python @@ -407,10 +411,6 @@ its iteration whenever there are scheduled requests: await self.crawler.signals.wait_for(signals.scheduler_empty) yield item_or_request -This can help minimize the number of requests in the scheduler at any given -time, to minimize resource usage (memory or disk, depending on -:setting:`JOBDIR`). - .. _builtin-spiders: Generic Spiders From 18ed0c0f7cea0fc23e1f837b72059889191e735f Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 13:58:09 +0200 Subject: [PATCH 22/42] Fall back to the response encoding in TextResponse.json() (#7897) --- docs/topics/request-response.rst | 3 --- scrapy/http/response/text.py | 16 ++++++++++++++-- tests/test_http_response_text.py | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index f810146e4..a177d1ad5 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -1432,9 +1432,6 @@ TextResponse objects .. automethod:: TextResponse.json() - Returns a Python object from deserialized JSON document. - The result is cached after the first call. - .. method:: TextResponse.urljoin(url) Constructs an absolute url by combining the Response's base url with diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index d01e23e47..64251780a 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -84,9 +84,21 @@ class TextResponse(Response): ) def json(self) -> Any: - """Deserialize a JSON document to a Python object.""" + """Deserialize a JSON document to a Python object. + + .. versionchanged:: VERSION + Bodies that cannot be decoded as UTF-8, UTF-16 or UTF-32, as the + JSON specification requires, are now decoded using + :attr:`TextResponse.encoding` instead of raising + :exc:`UnicodeDecodeError`. + + The result is cached after the first call. + """ if self._cached_decoded_json is _NONE: - self._cached_decoded_json = json.loads(self.body) + try: + self._cached_decoded_json = json.loads(self.body) + except UnicodeDecodeError: + self._cached_decoded_json = json.loads(self.text) return self._cached_decoded_json @property diff --git a/tests/test_http_response_text.py b/tests/test_http_response_text.py index efa63e049..f705dbcee 100644 --- a/tests/test_http_response_text.py +++ b/tests/test_http_response_text.py @@ -481,6 +481,22 @@ class TestTextResponse(TestResponseBase): ): text_response.json() + def test_json_response_non_utf8(self): + response = self.response_class( + "http://www.example.com", + body='{"message": "café"}'.encode("cp1252"), + headers={"Content-Type": "application/json"}, + ) + assert response.json() == {"message": "café"} + + def test_json_response_wrong_charset(self): + response = self.response_class( + "http://www.example.com", + body='{"message": "café"}'.encode(), + headers={"Content-Type": "application/json; charset=iso-8859-1"}, + ) + assert response.json() == {"message": "café"} + def test_cache_json_response(self): json_valid_bodies = [b"""{"ip": "109.187.217.200"}""", b"""null"""] for json_body in json_valid_bodies: From a6c017c2ca2b8a9bca8b46b692846f492fe77a00 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 13:59:22 +0200 Subject: [PATCH 23/42] Advise setting an identifying user agent (#7890) --- docs/intro/tutorial.rst | 5 ++++ docs/topics/practices.rst | 51 +++++++++++++++++++++++---------------- docs/topics/settings.rst | 5 ++++ 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index eaf492c95..efade47e6 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -72,6 +72,11 @@ This will create a ``tutorial`` directory with the following contents:: spiders/ # a directory where you'll later put your spiders __init__.py +Before crawling anything, open ``settings.py`` and uncomment the +:setting:`USER_AGENT` line to identify yourself, e.g. a project name plus a URL +or an email address. Website owners who take issue with your crawler can then +ask you to adjust it, rather than block it. + Our first Spider ================ diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index aeaf322c2..971bb9106 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -530,32 +530,41 @@ modules by separating them with commas. Avoiding getting banned ======================= -Some websites implement certain measures to prevent bots from crawling them, -with varying degrees of sophistication. Getting around those measures can be -difficult and tricky, and may sometimes require special infrastructure. Please -consider contacting `commercial support`_ if in doubt. +Websites tell regular visitors and crawlers apart by how their traffic looks: +the headers it carries, how fast it arrives, how many requests come from the +same place. Traffic that stands out can be blocked even when the crawling +itself would be welcome. -Here are some tips to keep in mind when dealing with these kinds of sites: +Where the website allows crawling, the most effective thing you can do is make +yourself known: set :setting:`USER_AGENT` to a value that identifies you and +lets its owners reach you, so that they can ask you to adjust your crawler +rather than block it. -* rotate your user agent from a pool of well-known ones from browsers (Google - around to get a list of them) -* disable cookies (see :setting:`COOKIES_ENABLED`) as some sites may use - cookies to spot bot behaviour -* use download delays (2 or higher). See :setting:`DOWNLOAD_DELAY` setting. -* if possible, use `Common Crawl`_ to fetch pages, instead of hitting the sites - directly -* use a pool of rotating IPs. For example, the free `Tor project`_ or paid +Where that is not enough, the following make your traffic resemble that of a +regular visitor: + +* rotate your user agent among those of common browsers, so that your requests + do not all look alike (search the web for an up-to-date list) +* disable cookies (see :setting:`COOKIES_ENABLED`), so that a session + identifier does not tie all your requests together +* space out your requests, 2 seconds apart or more, with the + :setting:`DOWNLOAD_DELAY` setting, to keep your pace closer to that of a + person browsing +* where possible, read pages from `Common Crawl`_, which sends no traffic to + the website at all +* spread your requests over a pool of IP addresses, so that none of them + accounts for your whole crawl. For example, the free `Tor project`_ or paid services like `ProxyMesh`_. -* for HTTPS websites, if blocking appears related to TLS behavior, consider - adjusting the :setting:`DOWNLOAD_TLS_MIN_VERSION` and - :setting:`DOWNLOAD_TLS_MAX_VERSION` settings, since some websites may respond - differently depending on the TLS method used by the client. -* use a ban avoidance service, such as `Zyte API`_, which provides a `Scrapy - plugin `__ and additional +* match the TLS behavior of a browser: some websites respond differently + depending on the TLS version of the client, which you can adjust with the + :setting:`DOWNLOAD_TLS_MIN_VERSION` and :setting:`DOWNLOAD_TLS_MAX_VERSION` + settings. +* let a service take care of all of the above, such as `Zyte API`_, which + provides a `Scrapy plugin + `__ and additional features, like `AI web scraping `__ -If you are still unable to prevent your bot getting banned, consider contacting -`commercial support`_. +If your crawler still gets blocked, consider contacting `commercial support`_. .. _static-analysis: diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index dc09fdeeb..ec222d999 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -2338,6 +2338,11 @@ also used by :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware if :setting:`ROBOTSTXT_USER_AGENT` setting is ``None`` and there is no overriding User-Agent header specified for the request. +Set it to a value that identifies you, including a URL or an email address +where website owners can reach you, e.g. ``"MyProject +(+https://example.com/bot)"``, so that they can ask you to adjust your crawler +rather than block it. + .. setting:: WARN_ON_GENERATOR_RETURN_VALUE WARN_ON_GENERATOR_RETURN_VALUE From 95fe7acc86f83643bfa98d5a0da97b8457a61774 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 18:14:20 +0200 Subject: [PATCH 24/42] Warn only once per request about the download warn size (#7963) --- scrapy/core/downloader/handlers/http11.py | 7 +++++-- tests/utils/bases/download_handlers_http.py | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 17dca5acb..3b5c08666 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -548,7 +548,8 @@ class _ScrapyAgent: txresponse._transport._producer.abortConnection() raise DownloadCancelledError(warning_msg) - if warnsize and expected_size > warnsize: + reached_warnsize = bool(warnsize and expected_size > warnsize) + if reached_warnsize: logger.warning( get_warnsize_msg(expected_size, warnsize, request, expected=True) ) @@ -561,6 +562,7 @@ class _ScrapyAgent: request=request, maxsize=maxsize, warnsize=warnsize, + reached_warnsize=reached_warnsize, fail_on_dataloss=fail_on_dataloss, crawler=self._crawler, tls_verbose_logging=self._tls_verbose_logging, @@ -625,6 +627,7 @@ class _ResponseReader(Protocol): fail_on_dataloss: bool, crawler: Crawler, *, + reached_warnsize: bool = False, tls_verbose_logging: bool = False, ): self._finished: Deferred[_ResultT] = finished @@ -634,7 +637,7 @@ class _ResponseReader(Protocol): self._maxsize: int = maxsize self._warnsize: int = warnsize self._fail_on_dataloss: bool = fail_on_dataloss - self._reached_warnsize: bool = False + self._reached_warnsize: bool = reached_warnsize self._bytes_received: int = 0 self._certificate: ssl.Certificate | None = None self._ip_address: ipaddress.IPv4Address | ipaddress.IPv6Address | None = None diff --git a/tests/utils/bases/download_handlers_http.py b/tests/utils/bases/download_handlers_http.py index e44f9bcb8..65244b938 100644 --- a/tests/utils/bases/download_handlers_http.py +++ b/tests/utils/bases/download_handlers_http.py @@ -628,6 +628,7 @@ class TestHttpBase(ABC): "Expected to receive 5 bytes which is larger than download warn size (4)" in caplog.text ) + assert caplog.text.count("download warn size (4)") == 1 @coroutine_test async def test_download_with_warnsize_no_content_length( From a0d1ad20013cd24da5aeba2c7742a2719d83f599 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 18:15:27 +0200 Subject: [PATCH 25/42] Document that follow_all requests share meta and cb_kwargs values (#7962) --- scrapy/http/response/__init__.py | 5 +++++ scrapy/http/response/text.py | 3 +++ scrapy/spidermiddlewares/metacopy.py | 3 +-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index f1db11488..f91cb2095 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -305,6 +305,11 @@ class Response(object_ref): :class:`~.TextResponse` provides a :meth:`~.TextResponse.follow_all` method which supports selectors in addition to absolute/relative URLs and Link objects. + + .. caution:: Every returned request gets its own *meta* and + *cb_kwargs* dictionaries, but the values within them are shared. + Mutating one of those values, e.g. appending to a list, affects + all the returned requests. """ if not hasattr(urls, "__iter__"): raise TypeError("'urls' argument must be an iterable") diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 64251780a..a8e452f7e 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -276,6 +276,9 @@ class TextResponse(Response): using the ``css`` or ``xpath`` parameters, this method will not produce requests for selectors from which links cannot be obtained (for instance, anchor tags without an ``href`` attribute) + + .. seealso:: :meth:`.Response.follow_all`, for a caution about mutable + *meta* and *cb_kwargs* values. """ arguments = [x for x in (urls, css, xpath) if x is not None] if len(arguments) != 1: diff --git a/scrapy/spidermiddlewares/metacopy.py b/scrapy/spidermiddlewares/metacopy.py index aa5a120c6..2bef974d6 100644 --- a/scrapy/spidermiddlewares/metacopy.py +++ b/scrapy/spidermiddlewares/metacopy.py @@ -15,8 +15,7 @@ logger = logging.getLogger(__name__) class MetaCopyDetectionMiddleware(BaseSpiderMiddleware): """Warn when a spider yields a request with internal meta keys that should - not be copied from response.meta, or when two requests share the same meta - dict object. + not be copied from response.meta. Each warning is emitted at most once per crawl. """ From c67bb8ac47c0af3ee78ef7f55642a85c701c2565 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 18:15:37 +0200 Subject: [PATCH 26/42] Document BROWSER=wslview for open_in_browser under WSL (#7965) --- scrapy/utils/response.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index 7747a7b9b..b3622c159 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -90,6 +90,10 @@ def open_in_browser( def parse_details(self, response): if "item name" not in response.text: open_in_browser(response) + + On the Windows Subsystem for Linux, set the ``BROWSER`` environment + variable to `wslview `_ to open the + response in a Windows browser, which cannot read Linux paths otherwise. """ # circular imports from scrapy.http import HtmlResponse, TextResponse # noqa: PLC0415 From 8e94df65262f9d4d2d016bd3bc173c90c766c390 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 18:18:33 +0200 Subject: [PATCH 27/42] Time out hanging tests in the VCS dependencies job (#7960) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Time out hanging tests in the VCS dependencies job * Do not test pydispatcher’s latest commit --- .github/workflows/tests-vcs-deps.yml | 5 ++++- tox.ini | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests-vcs-deps.yml b/.github/workflows/tests-vcs-deps.yml index bf867dba7..93f262b62 100644 --- a/.github/workflows/tests-vcs-deps.yml +++ b/.github/workflows/tests-vcs-deps.yml @@ -16,8 +16,11 @@ jobs: tests: name: tests runs-on: ubuntu-latest + timeout-minutes: 30 env: - PYTEST_ADDOPTS: -n auto --no-cov + # A development branch of a dependency can make a test hang forever, so + # tests get a time limit here that they do not need elsewhere. + PYTEST_ADDOPTS: -n auto --no-cov --timeout=120 TOXENV: vcs-deps UV_PYTHON_PREFERENCE: only-system steps: diff --git a/tox.ini b/tox.ini index 94c04f75f..ab53111b8 100644 --- a/tox.ini +++ b/tox.ini @@ -44,6 +44,7 @@ deps = pygments pytest pytest-cov >= 7.0.0 + pytest-timeout pytest-xdist sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 pytest-twisted >= 1.14.3 @@ -214,7 +215,8 @@ deps = # # Pillow and uvloop build from source, and need the libjpeg headers and # autotools respectively. robotexclusionrulesparser has no public repository, -# so it stays at its latest release. +# and PyDispatcher has seen no commit since 2023-10-23, so they stay at their +# latest release. commands_pre = uv pip install --python {envpython} --no-deps --reinstall \ git+https://github.com/twisted/twisted \ @@ -235,7 +237,6 @@ commands_pre = git+https://github.com/john-kurkowski/tldextract \ git+https://github.com/scrapy/w3lib \ git+https://github.com/zopefoundation/zope.interface \ - git+https://github.com/mcfletch/pydispatcher \ git+https://github.com/boto/boto3 \ git+https://github.com/bpython/bpython \ git+https://github.com/google/brotli \ From 5427080f4892fb8f33cc159fdcd6f8ce8d4c140e Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 18:37:24 +0200 Subject: [PATCH 28/42] Create a page on optimization (#7938) --- docs/conf.py | 5 + docs/index.rst | 6 +- docs/requirements.in | 1 + docs/requirements.txt | 3 + docs/topics/broad-crawls.rst | 195 ------------------- docs/topics/optimize.rst | 353 +++++++++++++++++++++++++++++++++++ docs/topics/settings.rst | 1 + 7 files changed, 366 insertions(+), 198 deletions(-) delete mode 100644 docs/topics/broad-crawls.rst create mode 100644 docs/topics/optimize.rst diff --git a/docs/conf.py b/docs/conf.py index 1b41adaad..ad55231bc 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -31,9 +31,14 @@ extensions = [ "sphinx_scrapy", "scrapyfixautodoc", # Must be after "sphinx.ext.autodoc" "sphinx.ext.coverage", + "sphinx_reredirects", "sphinx_rtd_dark_mode", ] +redirects = { + "topics/broad-crawls": "optimize.html#broad-crawls", +} + templates_path = ["_templates"] exclude_patterns = ["build", "Thumbs.db", ".DS_Store"] diff --git a/docs/index.rst b/docs/index.rst index 688cab81b..d45b2c208 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -152,7 +152,7 @@ Solving specific problems topics/contracts topics/practices topics/security - topics/broad-crawls + topics/optimize topics/developer-tools topics/dynamic-content topics/leaks @@ -180,8 +180,8 @@ Solving specific problems Understand the security implications of Scrapy defaults and how to harden them. -:doc:`topics/broad-crawls` - Tune Scrapy for crawling a lot domains in parallel. +:doc:`topics/optimize` + Find the bottleneck of your crawls and learn how to address it. :doc:`topics/developer-tools` Learn how to scrape with your browser's developer tools. diff --git a/docs/requirements.in b/docs/requirements.in index 3783dd1dc..77514642a 100644 --- a/docs/requirements.in +++ b/docs/requirements.in @@ -3,6 +3,7 @@ pydantic scrapy-spider-metadata sphinx sphinx-notfound-page +sphinx-reredirects sphinx-rtd-theme sphinx-rtd-dark-mode sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.10 diff --git a/docs/requirements.txt b/docs/requirements.txt index 0f5969401..b0a6d0b04 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -134,6 +134,7 @@ sphinx==9.1.0 # sphinx-llms-txt # sphinx-markdown-builder # sphinx-notfound-page + # sphinx-reredirects # sphinx-rtd-theme # sphinx-scrapy # sphinxcontrib-jquery @@ -147,6 +148,8 @@ sphinx-markdown-builder @ git+https://github.com/zytedata/sphinx-markdown-builde # via sphinx-scrapy sphinx-notfound-page==1.1.0 # via -r docs/requirements.in +sphinx-reredirects==1.1.0 + # via -r docs/requirements.in sphinx-rtd-dark-mode==1.3.0 # via -r docs/requirements.in sphinx-rtd-theme==3.1.0 diff --git a/docs/topics/broad-crawls.rst b/docs/topics/broad-crawls.rst deleted file mode 100644 index d6b9fd6f9..000000000 --- a/docs/topics/broad-crawls.rst +++ /dev/null @@ -1,195 +0,0 @@ -.. _topics-broad-crawls: - -============ -Broad Crawls -============ - -Scrapy defaults are optimized for crawling specific sites. These sites are -often handled by a single Scrapy spider, although this is not necessary or -required (for example, there are generic spiders that handle any given site -thrown at them). - -In addition to this "focused crawl", there is another common type of crawling -which covers a large (potentially unlimited) number of domains, and is only -limited by time or other arbitrary constraint, rather than stopping when the -domain was crawled to completion or when there are no more requests to perform. -These are called "broad crawls" and is the typical crawlers employed by search -engines. - -These are some common properties often found in broad crawls: - -* they crawl many domains (often, unbounded) instead of a specific set of sites - -* they don't necessarily crawl domains to completion, because it would be - impractical (or impossible) to do so, and instead limit the crawl by time or - number of pages crawled - -* they are simpler in logic (as opposed to very complex spiders with many - extraction rules) because data is often post-processed in a separate stage - -* they crawl many domains concurrently, which allows them to achieve faster - crawl speeds by not being limited by any particular site constraint (each site - is crawled slowly to respect politeness, but many sites are crawled in - parallel) - -As said above, Scrapy default settings are optimized for focused crawls, not -broad crawls. However, due to its asynchronous architecture, Scrapy is very -well suited for performing fast broad crawls. This page summarizes some things -you need to keep in mind when using Scrapy for doing broad crawls, along with -concrete suggestions of Scrapy settings to tune in order to achieve an -efficient broad crawl. - -.. _broad-crawls-scheduler-priority-queue: - -.. _broad-crawls-concurrency: - -Increase concurrency -==================== - -Concurrency is the number of requests that are processed in parallel. There is -a global limit (:setting:`CONCURRENT_REQUESTS`) and an additional limit that -can be set per domain (:setting:`CONCURRENT_REQUESTS_PER_DOMAIN`). - -The default global concurrency limit in Scrapy is not suitable for crawling -many different domains in parallel, so you will want to increase it. How much -to increase it will depend on how much CPU and memory your crawler will have -available. - -A good starting point is ``100``: - -.. code-block:: python - - CONCURRENT_REQUESTS = 100 - -But the best way to find out is by doing some trials and identifying at what -concurrency your Scrapy process gets CPU bounded. For optimum performance, you -should pick a concurrency where CPU usage is at 80-90%. - -Increasing concurrency also increases memory usage. If memory usage is a -concern, you might need to lower your global concurrency limit accordingly. - - -Increase Twisted IO thread pool maximum size -============================================ - -Currently Scrapy does DNS resolution in a blocking way with usage of thread -pool. With higher concurrency levels the crawling could be slow or even fail -hitting DNS resolver timeouts. Possible solution to increase the number of -threads handling DNS queries. The DNS queue will be processed faster speeding -up establishing of connection and crawling overall. - -To increase maximum thread pool size use: - -.. code-block:: python - - REACTOR_THREADPOOL_MAXSIZE = 20 - -Setup your own DNS -================== - -If you have multiple crawling processes and single central DNS, it can act -like DoS attack on the DNS server resulting to slow down of entire network or -even blocking your machines. To avoid this setup your own DNS server with -local cache and upstream to some large DNS like OpenDNS or Verizon. - -Reduce log level -================ - -When doing broad crawls you are often only interested in the crawl rates you -get and any errors found. These stats are reported by Scrapy when using the -``INFO`` log level. In order to save CPU (and log storage requirements) you -should not use ``DEBUG`` log level when performing large broad crawls in -production. Using ``DEBUG`` level when developing your (broad) crawler may be -fine though. - -To set the log level use: - -.. code-block:: python - - LOG_LEVEL = "INFO" - -Disable cookies -=============== - -Disable cookies unless you *really* need. Cookies are often not needed when -doing broad crawls (search engine crawlers ignore them), and they improve -performance by saving some CPU cycles and reducing the memory footprint of your -Scrapy crawler. - -To disable cookies use: - -.. code-block:: python - - COOKIES_ENABLED = False - -Disable retries -=============== - -Retrying failed HTTP requests can slow down the crawls substantially, especially -when sites causes are very slow (or fail) to respond, thus causing a timeout -error which gets retried many times, unnecessarily, preventing crawler capacity -to be reused for other domains. - -To disable retries use: - -.. code-block:: python - - RETRY_ENABLED = False - -Reduce download timeout -======================= - -Unless you are crawling from a very slow connection (which shouldn't be the -case for broad crawls) reduce the download timeout so that stuck requests are -discarded quickly and free up capacity to process the next ones. - -To reduce the download timeout use: - -.. code-block:: python - - DOWNLOAD_TIMEOUT = 15 - -Disable redirects -================= - -Consider disabling redirects, unless you are interested in following them. When -doing broad crawls it's common to save redirects and resolve them when -revisiting the site at a later crawl. This also help to keep the number of -request constant per crawl batch, otherwise redirect loops may cause the -crawler to dedicate too many resources on any specific domain. - -To disable redirects use: - -.. code-block:: python - - REDIRECT_ENABLED = False - -.. _broad-crawls-bfo: - -Crawl in BFO order -================== - -:ref:`Scrapy crawls in DFO order by default `. - -In broad crawls, however, page crawling tends to be faster than page -processing. As a result, unprocessed early requests stay in memory until the -final depth is reached, which can significantly increase memory usage. - -:ref:`Crawl in BFO order ` instead to save memory. - - -Be mindful of memory leaks -========================== - -If your broad crawl shows a high memory usage, in addition to :ref:`crawling in -BFO order `, :ref:`lowering concurrency -` and :ref:`delaying start request iteration -` you should :ref:`debug your memory leaks -`. - - -Install a specific Twisted reactor -================================== - -If the crawl is exceeding the system's capabilities, you might want to try -installing a specific Twisted reactor, via the :setting:`TWISTED_REACTOR` setting. diff --git a/docs/topics/optimize.rst b/docs/topics/optimize.rst new file mode 100644 index 000000000..cea49fca3 --- /dev/null +++ b/docs/topics/optimize.rst @@ -0,0 +1,353 @@ +.. _optimize: + +============ +Optimization +============ + +A crawl goes as fast as its slowest part allows. :ref:`Find out which part that +is ` before changing any setting. + +:ref:`Broad crawls ` have their own set of recommended +adjustments. + +.. _optimize-bottleneck: + +Finding the bottleneck +====================== + +The bottleneck depends on the spider: on the same machine, one crawl can be +limited by its own parsing code and another by the target website. So measure +the crawl that you want to optimize. + +:class:`~scrapy.extensions.logstats.LogStats` reports crawl speed every +:setting:`LOGSTATS_INTERVAL` seconds: + +.. code-block:: text + + [scrapy.extensions.logstats] INFO: Crawled 1200 pages (at 60 pages/min), scraped 1150 items (at 58 items/min) + +A rate that stays flat as you raise :setting:`CONCURRENT_REQUESTS` means +something else is the limit. + + +Reading the engine status +------------------------- + +The :ref:`telnet console ` reports, through ``est()``, +what every part of the engine is doing at a given moment: + +.. code-block:: text + + len(engine.downloader.active) : 16 + len(engine._slot.scheduler.mqs) : 92 + len(engine.scraper.slot.active) : 0 + engine.scraper.slot.active_size : 0 + engine.scraper.slot.needs_backout() : False + +Take a few readings at different points of the crawl: + +- ``len(engine.downloader.active)`` stays at :setting:`CONCURRENT_REQUESTS`: + the downloader is the limit. You are waiting on the network or on the + target website. See :ref:`optimize-concurrency`. + +- ``len(engine.downloader.active)`` stays below + :setting:`CONCURRENT_REQUESTS` while the scheduler queues (``mqs``, + ``dqs``) hold requests: something throttles those requests before they + reach the downloader, usually :setting:`CONCURRENT_REQUESTS_PER_DOMAIN`, + :setting:`DOWNLOAD_DELAY` or :ref:`AutoThrottle `. + +- Both the downloader and the scheduler queues stay near empty: your spider + is not producing requests fast enough. A crawl that walks pagination one + page at a time cannot use more concurrency than it creates. See + :ref:`optimize-requests`. + +- ``needs_backout()`` is ``True``, or ``active_size`` approaches + :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE`: responses arrive faster than your + callbacks and :ref:`item pipelines ` handle them. The + bottleneck is your own code. + +- ``len(engine._slot.scheduler.mqs)`` grows without settling: the crawl + discovers requests faster than it downloads them. This is what makes long + crawls run out of memory. + + +Reading resource usage +---------------------- + +CPU + Scrapy runs in a single process, and everything except DNS resolution and + code you explicitly move to a thread runs in a single thread. One CPU core + is the ceiling; a process sitting at 100% of a core is CPU-bound no matter + how many cores the machine has. + + Use a sampling profiler, such as py-spy_, to find out which code is + spending that CPU. :ref:`Selectors ` and item pipelines + are the usual answer. + + .. _py-spy: https://github.com/benfred/py-spy + +Memory + The :ref:`memory usage extension ` records + :stat:`memusage/startup` and :stat:`memusage/max`. A :stat:`memusage/max` + far above :stat:`memusage/startup` is expected; what matters is whether it + keeps growing for as long as the crawl runs. + + Growth that tracks ``len(engine._slot.scheduler.mqs)`` is a scheduling + problem, covered in :ref:`optimize-memory`. Growth that does not is a + :ref:`memory leak `. + +Network + Compare :stat:`downloader/response_bytes` over the crawl time against your + available bandwidth. Saturated bandwidth caps concurrency regardless of any + setting. + + DNS resolution is separate: it runs on a thread pool of + :setting:`REACTOR_THREADPOOL_MAXSIZE` threads, and results are cached + (:setting:`DNSCACHE_ENABLED`, :setting:`DNSCACHE_SIZE`). It only becomes a + limit of its own when there are many different domains to resolve, as in + :ref:`broad crawls `, where it shows up as slow starts and + DNS timeouts. + +Disk + :ref:`Feed exports ` write to disk on most crawls, + although item data is usually small enough for that not to matter. The ones + to suspect are + :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware` and + the :ref:`media pipelines `, which write whole + responses, and :setting:`JOBDIR`, which writes every scheduled request. + + +.. _optimize-concurrency: + +Sending more requests at a time +=============================== + +:setting:`CONCURRENT_REQUESTS` caps how many requests are being downloaded at +any given moment, :setting:`CONCURRENT_REQUESTS_PER_DOMAIN` caps how many of +those may target the same domain, and :setting:`DOWNLOAD_DELAY` sets a minimum +wait between two consecutive requests to the same domain. A project generated by +:command:`startproject` gets one request per second per domain out of these. + +Raise them to crawl a single website faster, and see +:ref:`broad-crawls-concurrency` to spread requests across many websites +instead. + +The limit that matters, though, is the one the target website tolerates. +Exceeding it gets you throttled, served errors or banned, all of which make the +crawl slower than a lower concurrency would have been. To find that limit: + +- Read the :ref:`robots.txt ` file of the website. Scrapy + does not act on its ``Crawl-delay`` and ``Request-rate`` directives, so when + they are present, translate them into :setting:`DOWNLOAD_DELAY` and + concurrency settings yourself. + +- Check the traffic that the website already gets, using a service like + `SimilarWeb`_ or `Cloudflare Radar`_. A rate that is a rounding error next + to what the website serves anyway is unlikely to be a problem for it. + + .. _SimilarWeb: https://www.similarweb.com/ + .. _Cloudflare Radar: https://radar.cloudflare.com/ + +- Look for a documented way in. An API, a bulk export or a search endpoint is + both faster for you and cheaper for the website than crawling its pages, and + the terms of service may state a rate. + +- Crawl when the website is idle, in its own timezone, so that the capacity + you take is capacity nobody else wanted. + +- Raise concurrency gradually and watch the website respond. + :stat:`downloader/response_status_count/{status_code}` counts for 429, 503 + or the ban page of the website, growing :stat:`retry/count`, or a + :ref:`download latency ` that climbs as you push harder, + all mean you have gone past the limit. + + +.. _optimize-requests: + +Producing requests faster +========================= + +A spider that discovers its requests one response at a time keeps the +downloader idle no matter how high you set :setting:`CONCURRENT_REQUESTS`. To +put more requests in the scheduler earlier: + +- Request every page at once when you can work out how many there are, e.g. + from a page count or from a result count and a page size in the first + response, instead of following a link to the next page on every response. + +- Get URLs from a source that lists many of them at once, such as a sitemap + or a search or export endpoint of the target website. For a crawl that + needs nothing else, :class:`~scrapy.spiders.SitemapSpider` reads sitemaps + for you. + +- Raise the :attr:`~scrapy.Request.priority` of pagination requests, so that + they are downloaded before the requests that they compete with, and + discover the rest of the crawl sooner. + +Each of these trades memory for speed: a request produced before the downloader +can take it waits in the scheduler, or on disk if you set :setting:`JOBDIR`. +Pushed far enough, they turn memory or disk into your new bottleneck, which is +why :ref:`optimize-memory` recommends the reverse of the last point. + + +.. _optimize-resources: + +Lowering resource usage +======================= + +.. _optimize-memory: + +Lowering memory usage +--------------------- + +- Lower :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE`. + +- Lower :setting:`DOWNLOAD_MAXSIZE`, which allows a single response to take up + to 1 GiB of memory by default, multiplied by your concurrency. Set + :setting:`DOWNLOAD_WARNSIZE` first to find out whether the website actually + serves responses that big. + +- Lower the number of :ref:`scheduled requests ` held in + memory: + + - Increase the :attr:`~scrapy.Request.priority` of requests whose + :attr:`~scrapy.Request.callback` cannot yield additional requests. + + For example, the following spider uses a higher priority (1) for book + requests than for pagination requests: + + .. code-block:: python + + from scrapy import Spider + + + class BooksToScrapeComSpider(Spider): + name = "books_toscrape_com" + start_urls = [ + "http://books.toscrape.com/catalogue/category/books/mystery_3/index.html" + ] + + def parse(self, response): + next_page_links = response.css(".next a") + yield from response.follow_all(next_page_links) + book_links = response.css("article a") + yield from response.follow_all(book_links, callback=self.parse_book, priority=1) + + def parse_book(self, response): + yield { + "name": response.css("h1::text").get(), + "price": response.css(".price_color::text").re_first("£(.*)"), + "url": response.url, + } + + .. note:: If the number of request-yielding, low-priority requests + scheduled at any given time is lower than concurrency settings + (:setting:`CONCURRENT_REQUESTS_PER_DOMAIN` or + :setting:`CONCURRENT_REQUESTS`), as in the example above, this can + slow down your crawl by turning those requests into a bottleneck. + + - If you have many :ref:`start requests `, consider + :ref:`delaying their iteration `. + + - Set :setting:`JOBDIR` to offload all scheduled requests to disk. + +- Be on the lookout for :ref:`memory leaks `. + + +Lowering network usage +---------------------- + +- Install brotli_ and zstandard_ to support brotli-compressed_ and + zstd-compressed_ responses. + + .. _brotli-compressed: https://www.ietf.org/rfc/rfc7932.txt + .. _brotli: https://pypi.org/project/Brotli/ + .. _zstd-compressed: https://www.ietf.org/rfc/rfc8478.txt + .. _zstandard: https://pypi.org/project/zstandard/ + +- Enable :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware` + while developing your spider, so that re-runs do not download the same + responses again. + + +Lowering CPU usage +------------------ + +- Set :setting:`LOG_LEVEL` to ``"INFO"`` or higher. + +- Restrict what you parse. A :ref:`selector ` over a + smaller part of the response, or a single query whose result you reuse, + beats repeated queries over the whole document. + + +Other tips +---------- + +- Try :ref:`using the asyncio reactor ` with uvloop_ as + :ref:`custom event loop `, i.e. setting + :setting:`ASYNCIO_EVENT_LOOP` to ``"uvloop.Loop"``. + + .. _uvloop: https://github.com/MagicStack/uvloop + + Alternatively, try :ref:`switching to a non-asyncio reactor + `. + +- Disable unused :ref:`components `. + + For example, set :setting:`COOKIES_ENABLED` to ``False`` unless you need + cookies. + +- Split the crawl across separate processes to use more than one CPU core. + See :ref:`distributed-crawls`. + + +.. _broad-crawls: +.. _topics-broad-crawls: + +Speeding up broad crawls +======================== + +While Scrapy is well suited for **broad crawls**, i.e. crawls that target many +websites, the default :ref:`settings ` are optimized for +crawls targeting a single website. + +For broad crawls, consider these adjustments: + +- .. _broad-crawls-concurrency: + + Increase the global concurrency: + + - Set :setting:`CONCURRENT_REQUESTS` as close to + :setting:`CONCURRENT_REQUESTS_PER_DOMAIN` × [number of target domains] + (e.g. 8 × 10 domains = 80 concurrent requests) as your CPU and memory + allow. + + - Increase :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE` when increasing + :setting:`CONCURRENT_REQUESTS` stops making a difference. + +- .. _broad-crawls-bfo: + + If memory is a bottleneck, see if :ref:`crawling in BFO order ` lowers + memory usage. + +- Improve DNS resolution speed: + + - Set up your own DNS server, with a local cache and upstream to a `large + DNS server`_, to avoid slowing down your network. + + .. _large DNS server: https://en.wikipedia.org/wiki/Public_recursive_name_server#Notable_public_DNS_service_operators + + - Increase :setting:`REACTOR_THREADPOOL_MAXSIZE` to the minimum value + that avoids DNS resolution timeouts and makes a noticeable positive + impact in crawl speed. + +- Lower the negative impact of some responses: + + - Set :setting:`RETRY_ENABLED` to ``False`` or, if you need retries, + consider lowering :setting:`RETRY_TIMES`. + + - Lower :setting:`DOWNLOAD_TIMEOUT` to a more reasonable value, to + discard stuck requests more quickly. + + - Set :setting:`REDIRECT_ENABLED` to ``False`` unless you want to follow + redirects. diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index ec222d999..0ea60e9cf 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1913,6 +1913,7 @@ Type of in-memory queue used by the scheduler. Other available type is: .. setting:: SCHEDULER_PRIORITY_QUEUE +.. _broad-crawls-scheduler-priority-queue: SCHEDULER_PRIORITY_QUEUE ------------------------ From 609f64c55d8603710218113ad42b1bbf940b3978 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 19:24:14 +0200 Subject: [PATCH 29/42] Make brotli a hard dependency (#7929) --- .github/workflows/tests-ubuntu.yml | 1 + docs/intro/install.rst | 2 - docs/topics/downloader-middleware.rst | 9 ++- pyproject.toml | 6 +- .../downloadermiddlewares/httpcompression.py | 24 +------- scrapy/utils/_compression.py | 9 ++- tests/test_command_shell.py | 6 +- ...st_downloadermiddleware_httpcompression.py | 55 ------------------- tox.ini | 8 +-- 9 files changed, 18 insertions(+), 102 deletions(-) diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index cd726a2fe..b58b2af7b 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -86,6 +86,7 @@ jobs: - python-version: pypy3.11-7.3.20 env: TOXENV: pypy3-extra-deps + coverage: true - python-version: "3.14" env: TOXENV: botocore diff --git a/docs/intro/install.rst b/docs/intro/install.rst index cba8c15a1..866c5abe2 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -111,8 +111,6 @@ The following extras are available: - Provides * - ``bpython`` - :ref:`bpython shell ` - * - ``brotli`` - - :ref:`Brotli response decompression ` * - ``gcs`` - :ref:`Google Cloud Storage ` for :ref:`feed exports ` and diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index fcfe7fd29..66a0164d9 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -741,14 +741,13 @@ HttpCompressionMiddleware .. class:: HttpCompressionMiddleware - This middleware allows compressed (gzip, deflate) traffic to be + This middleware allows compressed (gzip, deflate, `brotli`_) traffic to be sent/received from web sites. - This middleware also supports decoding `brotli-compressed`_ responses with - the :ref:`brotli ` extra, and `zstd-compressed`_ - responses with the :ref:`zstd ` extra. + This middleware also supports decoding `zstd-compressed`_ responses with + the :ref:`zstd ` extra. -.. _brotli-compressed: https://www.ietf.org/rfc/rfc7932.txt +.. _brotli: https://www.ietf.org/rfc/rfc7932.txt .. _zstd-compressed: https://www.ietf.org/rfc/rfc8478.txt diff --git a/pyproject.toml b/pyproject.toml index 0dcbade90..4edfb7ec8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,6 +26,8 @@ dependencies = [ # Platform-specific dependencies 'PyDispatcher>=2.0.5; platform_python_implementation == "CPython"', 'PyPyDispatcher>=2.1.0; platform_python_implementation == "PyPy"', + 'brotli>=1.2.0; implementation_name != "pypy"', + 'brotlicffi>=1.2.0.0; implementation_name == "pypy"', ] classifiers = [ "Development Status :: 5 - Production/Stable", @@ -62,10 +64,6 @@ Tracker = "https://github.com/scrapy/scrapy/issues" [project.optional-dependencies] bpython = ["bpython>=0.7.1"] -brotli = [ - "brotli>=1.2.0; implementation_name != 'pypy'", - "brotlicffi>=1.2.0.0; implementation_name == 'pypy'", -] gcs = ["google-cloud-storage>=1.29.0"] httpx = ["httpx2[http2,socks]>=2.0.0"] images = ["Pillow>=8.3.2"] diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index b99c323c5..0045ddcaa 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -30,27 +30,7 @@ if TYPE_CHECKING: logger = getLogger(__name__) -ACCEPTED_ENCODINGS: list[bytes] = [b"gzip", b"deflate"] - -try: - try: - import brotli - except ImportError: - import brotlicffi as brotli -except ImportError: - pass -else: - try: - brotli.Decompressor.can_accept_more_data # noqa: B018 - except AttributeError: # pragma: no cover - warnings.warn( - "You have brotli installed. But 'br' encoding support now requires " - "brotli's or brotlicffi's version >= 1.2.0. Please upgrade " - "brotli/brotlicffi to make Scrapy decode 'br' encoded responses.", - stacklevel=2, - ) - else: - ACCEPTED_ENCODINGS.append(b"br") +ACCEPTED_ENCODINGS: list[bytes] = [b"gzip", b"deflate", b"br"] if find_spec("zstandard") is not None: ACCEPTED_ENCODINGS.append(b"zstd") @@ -205,8 +185,6 @@ class HttpCompressionMiddleware: f"{self.__class__.__name__} cannot decode the response for {response.url} " f"from unsupported encoding(s) '{encodings_str}'." ) - if b"br" in encodings: - msg += " You need to install brotli or brotlicffi >= 1.2.0 to decode 'br'." if b"zstd" in encodings: msg += " You need to install zstandard to decode 'zstd'." logger.warning(msg) diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 4767c29f2..ac98a61c8 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -2,11 +2,10 @@ import contextlib import zlib from io import BytesIO -with contextlib.suppress(ImportError): - try: - import brotli - except ImportError: - import brotlicffi as brotli +try: + import brotli +except ImportError: + import brotlicffi as brotli with contextlib.suppress(ImportError): import zstandard diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 29667a1ae..6bc1ebbbb 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -201,7 +201,7 @@ class TestInteractiveShell: env = os.environ.copy() env["SCRAPY_PYTHON_SHELL"] = "python" logfile = BytesIO() - p = PopenSpawn(args, env=env, timeout=5) + p = PopenSpawn(args, env=env, timeout=60) p.logfile_read = logfile p.expect_exact("Available Scrapy objects") p.sendline(f"fetch('{mockserver.url('/')}')") @@ -235,7 +235,7 @@ class TestInteractiveShell: def _run_interactive_shell(self, env: dict[str, str]) -> str: args = (sys.executable, "-m", "scrapy.cmdline", "shell") logfile = BytesIO() - p = PopenSpawn(args, env=env, timeout=5) + p = PopenSpawn(args, env=env, timeout=60) p.logfile_read = logfile p.expect_exact("Available Scrapy objects") p.sendeof() @@ -256,7 +256,7 @@ class TestInteractiveShell: self._isolate_config(env, config_home) args = (sys.executable, "-m", "scrapy.cmdline", "shell") logfile = BytesIO() - p = PopenSpawn(args, env=env, timeout=10) + p = PopenSpawn(args, env=env, timeout=60) p.logfile_read = logfile p.expect_exact("Available Scrapy objects") # The standard Python shell never imports IPython, whereas the IPython diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index fa0707491..a43bb51ba 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -52,20 +52,6 @@ FORMAT = { } -def _skip_if_no_br() -> None: - try: - try: - import brotli # noqa: PLC0415 - - brotli.Decompressor.can_accept_more_data - except (ImportError, AttributeError): - import brotlicffi # noqa: PLC0415 - - brotlicffi.Decompressor.can_accept_more_data - except (ImportError, AttributeError): - pytest.skip("no brotli support") - - def _skip_if_no_zstd() -> None: pytest.importorskip("zstandard") @@ -161,8 +147,6 @@ class TestHttpCompression: self.assertStatsEqual("httpcompression/response_bytes", 74837) def test_process_response_br(self): - _skip_if_no_br() - response = self._getresponse("br") assert response.request request = response.request @@ -174,32 +158,6 @@ class TestHttpCompression: self.assertStatsEqual("httpcompression/response_count", 1) self.assertStatsEqual("httpcompression/response_bytes", 74837) - def test_process_response_br_unsupported(self, caplog: pytest.LogCaptureFixture): - if find_spec("brotli") is not None or find_spec("brotlicffi") is not None: - pytest.skip("Requires not having brotli support") - response = self._getresponse("br") - assert response.request - request = response.request - assert response.headers["Content-Encoding"] == b"br" - caplog.clear() - with caplog.at_level( - WARNING, logger="scrapy.downloadermiddlewares.httpcompression" - ): - newresponse = self.mw.process_response(request, response) - assert caplog.record_tuples == [ - ( - "scrapy.downloadermiddlewares.httpcompression", - WARNING, - ( - "HttpCompressionMiddleware cannot decode the response for " - "http://scrapytest.org/ from unsupported encoding(s) 'br'. " - "You need to install brotli or brotlicffi >= 1.2.0 to decode 'br'." - ), - ), - ] - assert newresponse is not response - assert newresponse.headers.getlist("Content-Encoding") == [b"br"] - def test_process_response_zstd(self): _skip_if_no_zstd() @@ -550,8 +508,6 @@ class TestHttpCompression: assert cause.decompressed_size < 1_100_000 def test_compression_bomb_setting_br(self): - _skip_if_no_br() - self._test_compression_bomb_setting("br") def test_compression_bomb_setting_deflate(self): @@ -609,8 +565,6 @@ class TestHttpCompression: @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_compression_bomb_spider_attr_br(self): - _skip_if_no_br() - self._test_compression_bomb_spider_attr("br") @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") @@ -643,8 +597,6 @@ class TestHttpCompression: assert cause.decompressed_size < 1_100_000 def test_compression_bomb_request_meta_br(self): - _skip_if_no_br() - self._test_compression_bomb_request_meta("br") def test_compression_bomb_request_meta_deflate(self): @@ -689,8 +641,6 @@ class TestHttpCompression: def test_download_warnsize_setting_br( self, caplog: pytest.LogCaptureFixture ) -> None: - _skip_if_no_br() - self._test_download_warnsize_setting(caplog, "br") def test_download_warnsize_setting_deflate( @@ -744,8 +694,6 @@ class TestHttpCompression: def test_download_warnsize_spider_attr_br( self, caplog: pytest.LogCaptureFixture ) -> None: - _skip_if_no_br() - self._test_download_warnsize_spider_attr(caplog, "br") @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") @@ -799,8 +747,6 @@ class TestHttpCompression: def test_download_warnsize_request_meta_br( self, caplog: pytest.LogCaptureFixture ) -> None: - _skip_if_no_br() - self._test_download_warnsize_request_meta(caplog, "br") def test_download_warnsize_request_meta_deflate( @@ -834,7 +780,6 @@ class TestHttpCompression: return new_response def test_process_truncated_response_br(self): - _skip_if_no_br() resp = self._get_truncated_response("br") assert resp.body.startswith(b"= 1.2.0; implementation_name != "pypy" # optional for HTTP compress downloader middleware tests - brotlicffi >= 1.2.0.0; implementation_name == "pypy" # optional for HTTP compress downloader middleware tests google-cloud-storage httpx2[http2,socks] ipython @@ -189,8 +189,6 @@ deps = Twisted[http2]==21.7.0 boto3==1.20.0 bpython==0.7.1 - brotli==1.2.0; implementation_name != "pypy" - brotlicffi==1.2.0.0; implementation_name == "pypy" google-cloud-storage==1.29.0 httpx2[http2,socks]==2.0.0 ipython==8.15.0 @@ -291,7 +289,6 @@ commands = basepython = pypy3 deps = {[testenv:extra-deps]deps} -commands = {[testenv:pypy3]commands} [testenv:min-pypy3] basepython = pypy3.11 @@ -301,6 +298,7 @@ deps = pytest==8.4.0 Protego==0.1.15 Twisted==21.7.0 + brotlicffi==1.2.0.0 cryptography==44.0.2 cssselect==0.9.1 itemadapter==0.1.0 From 7d9516f33292c8f81a41c9b113f3a135a66c8384 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 19:30:30 +0200 Subject: [PATCH 30/42] Canonicalize each extracted link once (#7961) --- scrapy/linkextractors/lxmlhtml.py | 2 +- tests/test_linkextractors.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 46ac39a28..75f9753e7 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -332,7 +332,7 @@ class LxmlLinkExtractor: unique=unique, process=process_value, strip=strip, - canonicalized=not canonicalize, + canonicalized=True, ) self.allow_res: list[re.Pattern[str]] = self._compile_regexes(allow) self.deny_res: list[re.Pattern[str]] = self._compile_regexes(deny) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 95a6aee54..7b73a133f 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -9,6 +9,7 @@ from w3lib import __version__ as w3lib_version from scrapy.http import HtmlResponse, XmlResponse from scrapy.link import Link +from scrapy.linkextractors import lxmlhtml from scrapy.linkextractors.lxmlhtml import LxmlLinkExtractor, LxmlParserLinkExtractor from tests import get_testdata @@ -798,6 +799,25 @@ class Base: class TestLxmlLinkExtractor(Base.TestLinkExtractorBase): extractor_cls = LxmlLinkExtractor + def test_canonicalize_once_per_link(self, monkeypatch): + canonicalize_url = lxmlhtml.canonicalize_url + calls = [] + + def counting_canonicalize_url(url, *args, **kwargs): + calls.append(url) + return canonicalize_url(url, *args, **kwargs) + + monkeypatch.setattr(lxmlhtml, "canonicalize_url", counting_canonicalize_url) + response = HtmlResponse( + "https://example.com", + body=b"".join(b'x' % i for i in range(10)), + ) + lx = self.extractor_cls(canonicalize=True) + assert lx.extract_links(response) == [ + Link(url="https://example.com/p?a=1&b=2", text="x") + ] + assert len(calls) == 10 + def test_link_restrict_text(self): html = b""" Pic of a cat From 786ab1049431bd890bbf7b0b35551037b385d04a Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 19:31:53 +0200 Subject: [PATCH 31/42] Document how to write a custom item exporter (#7931) --- docs/topics/exporters.rst | 66 +++++++++++++++++++++++++++++++++++++++ scrapy/exporters.py | 27 +++++++++------- tests/test_exporters.py | 12 +++---- 3 files changed, 88 insertions(+), 17 deletions(-) diff --git a/docs/topics/exporters.rst b/docs/topics/exporters.rst index c43b7e20f..56b995e18 100644 --- a/docs/topics/exporters.rst +++ b/docs/topics/exporters.rst @@ -136,6 +136,70 @@ Example: return f"$ {str(value)}" return super().serialize_field(field, name, value) +.. _custom-exporters: + +Writing your own item exporter +============================== + +To write an item exporter, subclass :class:`BaseItemExporter` and implement +:meth:`~BaseItemExporter.export_item`, where +:meth:`~BaseItemExporter.get_serialized_fields` gives you the ``(name, value)`` +pairs to export. + +To make your exporter available to the :ref:`feed exports +`, list it in the :setting:`FEED_EXPORTERS` setting. Feed +exports :ref:`build ` it with the output file as the first +positional argument, and with the ``fields``, ``encoding`` and ``indent`` +:ref:`feed options ` and every key of ``item_export_kwargs`` as +keyword arguments, so your ``__init__`` method must forward unknown keyword +arguments to :class:`BaseItemExporter`. + +The file object belongs to whoever opened it, i.e. to the feed storage in the +case of feed exports, which also closes it. If you need a text file, for +example to use :func:`csv.writer` or another Python API that does not accept a +binary file, wrap it with :class:`io.TextIOWrapper` and call +:meth:`~io.TextIOBase.detach` on the wrapper in +:meth:`~BaseItemExporter.finish_exporting`; otherwise the wrapper closes the +underlying file when it is garbage-collected. + +For example, the following item exporter writes items as blocks of +``name: value`` lines: + +.. code-block:: python + + from io import TextIOWrapper + + from scrapy.exporters import BaseItemExporter + + + class TextItemExporter(BaseItemExporter): + def __init__(self, file, item_separator="\n", **kwargs): + super().__init__(**kwargs) + self.item_separator = item_separator + self.stream = TextIOWrapper( + file, encoding=self.encoding or "utf-8", write_through=True + ) + + def export_item(self, item): + for name, value in self.get_serialized_fields(item): + print(f"{name}: {value}", file=self.stream) + self.stream.write(self.item_separator) + + def finish_exporting(self): + self.stream.detach() + +To use it as the ``txt`` feed format: + +.. code-block:: python + + FEED_EXPORTERS = {"txt": "myproject.exporters.TextItemExporter"} + FEEDS = { + "items.txt": { + "format": "txt", + "item_export_kwargs": {"item_separator": "---\n"}, + }, + } + .. _topics-exporters-reference: Built-in Item Exporters reference @@ -168,6 +232,8 @@ BaseItemExporter Exports the given item. This method must be implemented in subclasses. + .. automethod:: BaseItemExporter.get_serialized_fields + .. method:: serialize_field(field, name, value) Return the serialized value for the given field. You can override this diff --git a/scrapy/exporters.py b/scrapy/exporters.py index ea600d1a8..7f8aaf059 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -85,11 +85,16 @@ class BaseItemExporter(ABC): declared = (name for name in adapter.field_names() if name in populated) return dict.fromkeys([*declared, *adapter.keys()]) - def _get_serialized_fields( + def get_serialized_fields( self, item: Any, default_value: Any = None, include_empty: bool | None = None ) -> Iterable[tuple[str, Any]]: - """Return the fields to export as an iterable of tuples - (name, serialized_value) + """Return the fields of *item* to export, as an iterable of + ``(name, serialized_value)`` tuples, taking :attr:`fields_to_export` + into account and applying :meth:`serialize_field` to every value. + + Fields missing from *item* are exported with *default_value*. + + *include_empty* overrides :attr:`export_empty_fields`. """ item = ItemAdapter(item) @@ -136,7 +141,7 @@ class JsonLinesItemExporter(BaseItemExporter): self.encoder: JSONEncoder = ScrapyJSONEncoder(**self._kwargs) def export_item(self, item: Any) -> None: - itemdict = dict(self._get_serialized_fields(item)) + itemdict = dict(self.get_serialized_fields(item)) data = self.encoder.encode(itemdict) + "\n" self.file.write(to_bytes(data, self.encoding)) @@ -176,7 +181,7 @@ class JsonItemExporter(BaseItemExporter): self.file.write(b"]") def export_item(self, item: Any) -> None: - itemdict = dict(self._get_serialized_fields(item)) + itemdict = dict(self.get_serialized_fields(item)) data = to_bytes(self.encoder.encode(itemdict), self.encoding) self._add_comma_after_first() self.file.write(data) @@ -216,7 +221,7 @@ class XmlItemExporter(BaseItemExporter): self._beautify_indent(depth=1) self.xg.startElement(self.item_element, AttributesImpl({})) self._beautify_newline() - for name, value in self._get_serialized_fields(item, default_value=""): + for name, value in self.get_serialized_fields(item, default_value=""): self._export_xml_field(name, value, depth=2) self._beautify_indent(depth=1) self.xg.endElement(self.item_element) @@ -310,7 +315,7 @@ class CsvItemExporter(BaseItemExporter): f"See: https://docs.scrapy.org/en/latest/topics/feed-exports.html#feed-export-fields", ) self._data_loss_warned = True - fields = self._get_serialized_fields(item, default_value="", include_empty=True) + fields = self.get_serialized_fields(item, default_value="", include_empty=True) values = list(self._build_row(x for _, x in fields)) self.csv_writer.writerow(values) @@ -347,7 +352,7 @@ class PickleItemExporter(BaseItemExporter): self.protocol: int = protocol def export_item(self, item: Any) -> None: - d = dict(self._get_serialized_fields(item)) + d = dict(self.get_serialized_fields(item)) pickle.dump(d, self.file, self.protocol) @@ -365,7 +370,7 @@ class MarshalItemExporter(BaseItemExporter): self.file: BytesIO = file def export_item(self, item: Any) -> None: - marshal.dump(dict(self._get_serialized_fields(item)), self.file) + marshal.dump(dict(self.get_serialized_fields(item)), self.file) class PprintItemExporter(BaseItemExporter): @@ -374,7 +379,7 @@ class PprintItemExporter(BaseItemExporter): self.file: BytesIO = file def export_item(self, item: Any) -> None: - itemdict = dict(self._get_serialized_fields(item)) + itemdict = dict(self.get_serialized_fields(item)) self.file.write(to_bytes(pprint.pformat(itemdict) + "\n")) @@ -417,5 +422,5 @@ class PythonItemExporter(BaseItemExporter): yield key, self._serialize_value(value) def export_item(self, item: Any) -> dict[str | bytes, Any]: # type: ignore[override] - result: dict[str | bytes, Any] = dict(self._get_serialized_fields(item)) + result: dict[str | bytes, Any] = dict(self.get_serialized_fields(item)) return result diff --git a/tests/test_exporters.py b/tests/test_exporters.py index b857728ba..99afb8beb 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -108,26 +108,26 @@ class TestBaseItemExporter(ABC): def test_fields_to_export(self): ie = self._get_exporter(fields_to_export=["name"]) - assert list(ie._get_serialized_fields(self.i)) == [("name", "John\xa3")] + assert list(ie.get_serialized_fields(self.i)) == [("name", "John\xa3")] ie = self._get_exporter(fields_to_export=["name"], encoding="latin-1") - _, name = next(iter(ie._get_serialized_fields(self.i))) + _, name = next(iter(ie.get_serialized_fields(self.i))) assert isinstance(name, str) assert name == "John\xa3" ie = self._get_exporter(fields_to_export={"name": "名稱"}) - assert list(ie._get_serialized_fields(self.i)) == [("名稱", "John\xa3")] + assert list(ie.get_serialized_fields(self.i)) == [("名稱", "John\xa3")] def test_field_order(self): item = self.item_class(age="22", name="John\xa3") ie = self._get_exporter() - assert [name for name, _ in ie._get_serialized_fields(item)] == ["name", "age"] + assert [name for name, _ in ie.get_serialized_fields(item)] == ["name", "age"] def test_field_order_dict_item(self): ie = self._get_exporter() - assert [name for name, _ in ie._get_serialized_fields({"age": "22"})] == ["age"] + assert [name for name, _ in ie.get_serialized_fields({"age": "22"})] == ["age"] assert [ - name for name, _ in ie._get_serialized_fields({"age": "22", "name": "John"}) + name for name, _ in ie.get_serialized_fields({"age": "22", "name": "John"}) ] == ["age", "name"] def test_field_custom_serializer(self): From 482a02d30c298e67fd610e5f27e56cfe11e9b4c5 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 19:41:32 +0200 Subject: [PATCH 32/42] Add a cookies documentation page (#7947) --- docs/faq.rst | 2 +- docs/index.rst | 4 + docs/topics/cookies.rst | 138 ++++++++++++++++++++++++++ docs/topics/downloader-middleware.rst | 98 +----------------- docs/topics/request-response.rst | 60 +---------- docs/topics/settings.rst | 5 +- 6 files changed, 150 insertions(+), 157 deletions(-) create mode 100644 docs/topics/cookies.rst diff --git a/docs/faq.rst b/docs/faq.rst index 80658a5bf..ef3d07a2b 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -292,7 +292,7 @@ Does Scrapy manage cookies automatically? Yes, Scrapy receives and keeps track of cookies sent by servers, and sends them back on subsequent requests, like any regular web browser does. -For more info see :ref:`topics-request-response` and :ref:`cookies-mw`. +For more info see :ref:`cookies`. How can I see the cookies being sent and received from Scrapy? -------------------------------------------------------------- diff --git a/docs/index.rst b/docs/index.rst index d45b2c208..de06e3488 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -78,6 +78,7 @@ Basic concepts topics/item-pipeline topics/feed-exports topics/request-response + topics/cookies topics/link-extractors topics/settings topics/exceptions @@ -109,6 +110,9 @@ Basic concepts :doc:`topics/request-response` Understand the classes used to represent HTTP requests and responses. +:doc:`topics/cookies` + Send and receive cookies. + :doc:`topics/link-extractors` Convenient classes to extract links to follow from pages. diff --git a/docs/topics/cookies.rst b/docs/topics/cookies.rst new file mode 100644 index 000000000..ab5f3dca0 --- /dev/null +++ b/docs/topics/cookies.rst @@ -0,0 +1,138 @@ +.. _cookies: +.. _cookies-mw: + +======= +Cookies +======= + +Scrapy keeps track of the cookies that websites set and sends them back on +later requests to those websites, just like a web browser does. That is the job +of :class:`~scrapy.downloadermiddlewares.cookies.CookiesMiddleware`, which is +enabled by default. + + +Setting cookies on a request +============================ + +.. invisible-code-block: python + + from scrapy import Request + +Use the ``cookies`` parameter of :class:`~scrapy.Request` to send cookies of +your own, either as a dict: + +.. code-block:: python + + request = Request( + url="https://example.com", + cookies={"currency": "USD", "country": "UY"}, + ) + +Or as a list of dicts, which also lets you set cookie attributes: + +.. code-block:: python + + request = Request( + url="https://example.com", + cookies=[ + { + "name": "currency", + "value": "USD", + "domain": "example.com", + "path": "/currency", + "secure": True, + }, + ], + ) + +Setting attributes is only useful if the cookies are stored for later requests, +i.e. if :reqmeta:`dont_merge_cookies` is not enabled. + +.. caution:: Cookies set through the ``Cookie`` header are not handled by + :class:`~scrapy.downloadermiddlewares.cookies.CookiesMiddleware`, which + drops that header. + +.. caution:: When a cookie name or value is a byte sequence that is not UTF-8 + encoded, the cookie is dropped and a warning is logged. See + :ref:`topics-logging-advanced-customization` to customize the logging + behavior. + + +.. reqmeta:: cookiejar + +Multiple cookie sessions per spider +=================================== + +By default all requests share a single cookie jar (session). To use different +ones, pass an identifier in the :reqmeta:`cookiejar` request meta key: + +.. skip: next +.. code-block:: python + + for i, url in enumerate(urls): + yield Request(url, meta={"cookiejar": i}, callback=self.parse_page) + +The :reqmeta:`cookiejar` meta key is not "sticky", so you need to keep passing +it along on subsequent requests: + +.. code-block:: python + + def parse_page(self, response): + return Request( + "https://example.com/otherpage", + meta={"cookiejar": response.meta["cookiejar"]}, + callback=self.parse_other_page, + ) + + +.. reqmeta:: dont_merge_cookies + +Skipping the cookie jar for a request +===================================== + +Set the :reqmeta:`dont_merge_cookies` request meta key to ``True`` to keep a +request from touching the cookie jar in either direction: no stored cookie is +sent with the request, and no cookie received in the response is stored. The +cookies of the request itself are ignored as well. + + +.. setting:: COOKIES_ENABLED + +COOKIES_ENABLED +=============== + +Default: ``True`` + +Whether to enable :class:`~scrapy.downloadermiddlewares.cookies.CookiesMiddleware`. +If disabled, no cookies are sent to web servers. + + +.. setting:: COOKIES_DEBUG + +COOKIES_DEBUG +============= + +Default: ``False`` + +If enabled, Scrapy logs all cookies sent in requests (i.e. the ``Cookie`` +header) and all cookies received in responses (i.e. the ``Set-Cookie`` +header):: + + 2011-04-06 14:35:10-0300 [scrapy.core.engine] INFO: Spider opened + 2011-04-06 14:35:10-0300 [scrapy.downloadermiddlewares.cookies] DEBUG: Sending cookies to: + Cookie: clientlanguage_nl=en_EN + 2011-04-06 14:35:14-0300 [scrapy.downloadermiddlewares.cookies] DEBUG: Received cookies from: <200 http://www.diningcity.com/netherlands/index.html> + Set-Cookie: JSESSIONID=B~FA4DC0C496C8762AE4F1A620EAB34F38; Path=/ + Set-Cookie: ip_isocode=US + Set-Cookie: clientlanguage_nl=en_EN; Expires=Thu, 07-Apr-2011 21:21:34 GMT; Path=/ + 2011-04-06 14:49:50-0300 [scrapy.core.engine] DEBUG: Crawled (200) (referer: None) + [...] + + +CookiesMiddleware +================= + +.. module:: scrapy.downloadermiddlewares.cookies + :synopsis: Cookies Downloader Middleware + +.. autoclass:: CookiesMiddleware diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 66a0164d9..2d6ba0cf3 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -169,106 +169,10 @@ middleware, see the :ref:`downloader middleware usage guide For a list of the components enabled by default (and their orders) see the :setting:`DOWNLOADER_MIDDLEWARES_BASE` setting. -.. _cookies-mw: - CookiesMiddleware ----------------- -.. module:: scrapy.downloadermiddlewares.cookies - :synopsis: Cookies Downloader Middleware - -.. class:: CookiesMiddleware - - This middleware enables working with sites that require cookies, such as - those that use sessions. It keeps track of cookies sent by web servers, and - sends them back on subsequent requests (from that spider), just like web - browsers do. - - .. caution:: When non-UTF8 encoded byte sequences are passed to a - :class:`~scrapy.Request`, the ``CookiesMiddleware`` will log - a warning. Refer to :ref:`topics-logging-advanced-customization` - to customize the logging behaviour. - - .. caution:: Cookies set via the ``Cookie`` header are not considered by the - :ref:`cookies-mw`. If you need to set cookies for a request, use the - :class:`Request.cookies ` parameter. This is a known - current limitation that is being worked on. - -The following settings can be used to configure the cookie middleware: - -* :setting:`COOKIES_ENABLED` -* :setting:`COOKIES_DEBUG` - -.. reqmeta:: cookiejar - -Multiple cookie sessions per spider -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -There is support for keeping multiple cookie sessions per spider by using the -:reqmeta:`cookiejar` Request meta key. By default it uses a single cookie jar -(session), but you can pass an identifier to use different ones. - -For example: - -.. skip: next -.. code-block:: python - - for i, url in enumerate(urls): - yield scrapy.Request(url, meta={"cookiejar": i}, callback=self.parse_page) - -Keep in mind that the :reqmeta:`cookiejar` meta key is not "sticky". You need to keep -passing it along on subsequent requests. For example: - -.. code-block:: python - - def parse_page(self, response): - # do some processing - return scrapy.Request( - "http://www.example.com/otherpage", - meta={"cookiejar": response.meta["cookiejar"]}, - callback=self.parse_other_page, - ) - -.. setting:: COOKIES_ENABLED - -COOKIES_ENABLED -~~~~~~~~~~~~~~~ - -Default: ``True`` - -Whether to enable the cookies middleware. If disabled, no cookies will be sent -to web servers. - -Notice that despite the value of :setting:`COOKIES_ENABLED` setting if -``Request.``:reqmeta:`meta['dont_merge_cookies'] ` -evaluates to ``True`` the request cookies will **not** be sent to the -web server and received cookies in :class:`~scrapy.http.Response` will -**not** be merged with the existing cookies. - -For more detailed information see the ``cookies`` parameter in -:class:`~scrapy.Request`. - -.. setting:: COOKIES_DEBUG - -COOKIES_DEBUG -~~~~~~~~~~~~~ - -Default: ``False`` - -If enabled, Scrapy will log all cookies sent in requests (i.e. ``Cookie`` -header) and all cookies received in responses (i.e. ``Set-Cookie`` header). - -Here's an example of a log with :setting:`COOKIES_DEBUG` enabled:: - - 2011-04-06 14:35:10-0300 [scrapy.core.engine] INFO: Spider opened - 2011-04-06 14:35:10-0300 [scrapy.downloadermiddlewares.cookies] DEBUG: Sending cookies to: - Cookie: clientlanguage_nl=en_EN - 2011-04-06 14:35:14-0300 [scrapy.downloadermiddlewares.cookies] DEBUG: Received cookies from: <200 http://www.diningcity.com/netherlands/index.html> - Set-Cookie: JSESSIONID=B~FA4DC0C496C8762AE4F1A620EAB34F38; Path=/ - Set-Cookie: ip_isocode=US - Set-Cookie: clientlanguage_nl=en_EN; Expires=Thu, 07-Apr-2011 21:21:34 GMT; Path=/ - 2011-04-06 14:49:50-0300 [scrapy.core.engine] DEBUG: Crawled (200) (referer: None) - [...] +See :ref:`cookies`. DefaultHeadersMiddleware diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index a177d1ad5..1d97e39b6 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -53,65 +53,13 @@ Request objects ``None`` is passed as value, the HTTP header will not be sent at all. .. caution:: Cookies set via the ``Cookie`` header are not considered by the - :ref:`cookies-mw`. If you need to set cookies for a request, use the - ``cookies`` argument. This is a known current limitation that is being - worked on. + :ref:`cookie middleware `. If you need to set cookies for a + request, use the ``cookies`` argument. :type headers: dict - :param cookies: the request cookies. These can be sent in two forms. - - .. invisible-code-block: python - - from scrapy import Request - - 1. Using a dict: - - .. code-block:: python - - request_with_cookies = Request( - url="http://www.example.com", - cookies={"currency": "USD", "country": "UY"}, - ) - - 2. Using a list of dicts: - - .. code-block:: python - - request_with_cookies = Request( - url="https://www.example.com", - cookies=[ - { - "name": "currency", - "value": "USD", - "domain": "example.com", - "path": "/currency", - "secure": True, - }, - ], - ) - - The latter form allows for customizing the ``domain`` and ``path`` - attributes of the cookie. This is only useful if the cookies are saved - for later requests. - - .. reqmeta:: dont_merge_cookies - - When some site returns cookies (in a response) those are stored in the - cookies for that domain and will be sent again in future requests. - That's the typical behaviour of any regular web browser. - - Note that setting the :reqmeta:`dont_merge_cookies` key to ``True`` in - :attr:`request.meta ` causes custom cookies to be - ignored. - - For more info see :ref:`cookies-mw`. - - .. caution:: Cookies set via the ``Cookie`` header are not considered by the - :ref:`cookies-mw`. If you need to set cookies for a request, use the - :class:`scrapy.Request.cookies ` parameter. This is a known - current limitation that is being worked on. - + :param cookies: the request cookies, as a dict of cookie names and values + or as a list of dicts with a cookie each. See :ref:`cookies`. :type cookies: dict or list :param encoding: the encoding of this request (defaults to ``'utf-8'``). diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 0ea60e9cf..de665f42e 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -654,9 +654,8 @@ The default headers used for Scrapy HTTP Requests. They're populated in the :class:`~scrapy.downloadermiddlewares.defaultheaders.DefaultHeadersMiddleware`. .. caution:: Cookies set via the ``Cookie`` header are not considered by the - :ref:`cookies-mw`. If you need to set cookies for a request, use the - :class:`Request.cookies ` parameter. This is a known - current limitation that is being worked on. + :ref:`cookie middleware `. If you need to set cookies for a + request, use the :class:`Request.cookies ` parameter. .. caution:: A ``Referer`` header defined here only reaches requests for which :class:`~scrapy.spidermiddlewares.referer.RefererMiddleware` does not set From a5614544a2ae3e2efe9d96ff32e476b57d9bec8f Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 19:51:20 +0200 Subject: [PATCH 33/42] Type tests involving items and exports (#7867) --- pyproject.toml | 12 --- scrapy/extensions/feedexport.py | 2 +- scrapy/pipelines/images.py | 4 +- tests/mockserver/ftp.py | 8 +- tests/test_exporters.py | 74 +++++++------ tests/test_feedexport.py | 36 ++++--- tests/test_feedexport_postprocess.py | 18 +++- tests/test_feedexport_storages.py | 50 ++++----- tests/test_feedexport_uri_params.py | 33 ++++-- tests/test_item.py | 44 ++++---- tests/test_loader.py | 6 +- tests/test_pipeline_crawl.py | 10 +- tests/test_pipeline_files.py | 37 ++++--- tests/test_pipeline_images.py | 52 +++++---- tests/test_pipeline_media.py | 156 +++++++++++++++++++-------- tests/test_pipelines.py | 12 ++- 16 files changed, 343 insertions(+), 211 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4edfb7ec8..5ec1d05a4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -123,23 +123,11 @@ module = [ "tests.test_downloaderslotssettings", "tests.test_dupefilters", "tests.test_engine_loop", - "tests.test_exporters", "tests.test_extension_statsmailer", "tests.test_extension_throttle", - "tests.test_feedexport", - "tests.test_feedexport_postprocess", - "tests.test_feedexport_storages", - "tests.test_feedexport_uri_params", - "tests.test_item", "tests.test_linkextractors", - "tests.test_loader", "tests.test_logformatter", "tests.test_mail", - "tests.test_pipeline_crawl", - "tests.test_pipeline_files", - "tests.test_pipeline_images", - "tests.test_pipeline_media", - "tests.test_pipelines", "tests.test_pqueues", "tests.test_scheduler_base", "tests.test_settings", diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index fc2b2f43f..e4294ac90 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -149,7 +149,7 @@ class BlockingFeedStorage(ABC): return NamedTemporaryFile(prefix="feed-", dir=path) - def store(self, file: IO[bytes]) -> Deferred[None] | None: + def store(self, file: IO[bytes]) -> Deferred[None]: return deferred_from_coro(run_in_thread(self._store_in_thread, file)) @abstractmethod diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 79b6c4f27..5e7a4b409 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -29,7 +29,7 @@ from scrapy.utils.defer import ensure_awaitable from scrapy.utils.python import to_bytes if TYPE_CHECKING: - from collections.abc import Iterable + from collections.abc import Iterator from os import PathLike from PIL import Image @@ -180,7 +180,7 @@ class ImagesPipeline(FilesPipeline): info: MediaPipeline.SpiderInfo, *, item: Any = None, - ) -> Iterable[tuple[str, Image.Image, BytesIO]]: + ) -> Iterator[tuple[str, Image.Image, BytesIO]]: path = self.file_path(request, response=response, info=info, item=item) orig_image = self._Image.open(BytesIO(response.body)) transposed_image = self._ImageOps.exif_transpose(orig_image) diff --git a/tests/mockserver/ftp.py b/tests/mockserver/ftp.py index 1edd64dda..72760b5ac 100644 --- a/tests/mockserver/ftp.py +++ b/tests/mockserver/ftp.py @@ -27,11 +27,12 @@ class MockFTPServer: (anonymous) and a temporary root path that you can read from the :attr:`path` attribute.""" + proc: Popen[str] + port: int + path: Path + def __init__(self) -> None: - self.proc: Popen[str] | None = None self.host: str = "127.0.0.1" - self.port: int | None = None - self.path: Path | None = None def __enter__(self) -> Self: self.path = Path(mkdtemp()) @@ -63,7 +64,6 @@ class MockFTPServer: traceback: TracebackType | None, ) -> None: rmtree(str(self.path)) - assert self.proc is not None self.proc.kill() self.proc.communicate() diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 99afb8beb..956697a04 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -4,6 +4,7 @@ import marshal import pickle import re from abc import ABC, abstractmethod +from collections.abc import Mapping from datetime import datetime from io import BytesIO from typing import Any @@ -63,18 +64,18 @@ class TestBaseItemExporter(ABC): self.ie = self._get_exporter() @abstractmethod - def _get_exporter(self, **kwargs) -> BaseItemExporter: + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: raise NotImplementedError - def _check_output(self): # noqa: B027 + def _check_output(self) -> None: # noqa: B027 pass - def _assert_expected_item(self, exported_dict): + def _assert_expected_item(self, exported_dict: dict[str, Any]) -> None: for k, v in exported_dict.items(): exported_dict[k] = to_unicode(v) assert self.i == self.item_class(**exported_dict) - def _get_nonstring_types_item(self): + def _get_nonstring_types_item(self) -> dict[str, Any]: return { "boolean": False, "number": 22, @@ -82,7 +83,7 @@ class TestBaseItemExporter(ABC): "float": 3.14, } - def assertItemExportWorks(self, item): + def assertItemExportWorks(self, item: Any) -> None: self.ie.start_exporting() self.ie.export_item(item) self.ie.finish_exporting() @@ -92,7 +93,7 @@ class TestBaseItemExporter(ABC): del self.ie self._check_output() - def test_export_item(self): + def test_export_item(self) -> None: self.assertItemExportWorks(self.i) def test_export_dict_item(self): @@ -142,7 +143,7 @@ class TestBaseItemExporter(ABC): class TestPythonItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PythonItemExporter(**kwargs) def test_invalid_option(self): @@ -173,6 +174,7 @@ class TestPythonItemExporter(TestBaseItemExporter): "age": [{"age": [{"age": "22", "name": "Joseph"}], "name": "Maria"}], "name": "Jesus", } + assert exported is not None assert isinstance(exported["age"][0], dict) assert isinstance(exported["age"][0]["age"][0], dict) @@ -186,6 +188,7 @@ class TestPythonItemExporter(TestBaseItemExporter): "age": [{"age": [{"age": "22", "name": "Joseph"}], "name": "Maria"}], "name": "Jesus", } + assert exported is not None assert isinstance(exported["age"][0], dict) assert isinstance(exported["age"][0]["age"][0], dict) @@ -202,10 +205,10 @@ class TestPythonItemExporterDataclass(TestPythonItemExporter): class TestPprintItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PprintItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self._assert_expected_item(eval(self.output.getvalue())) @@ -215,10 +218,10 @@ class TestPprintItemExporterDataclass(TestPprintItemExporter): class TestPickleItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PickleItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self._assert_expected_item(pickle.loads(self.output.getvalue())) def test_export_multiple_items(self): @@ -252,10 +255,10 @@ class TestPickleItemExporterDataclass(TestPickleItemExporter): class TestMarshalItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return MarshalItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self.output.seek(0) self._assert_expected_item(marshal.load(self.output)) @@ -279,7 +282,7 @@ class TestMarshalItemExporterDataclass(TestMarshalItemExporter): class TestCsvItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: # We need a fresh instance for each exporter, because # CsvItemExporter.stream.__del__() closes the underlying file # (CsvItemExporter.finish_exporting() calls detach() but not all tests @@ -287,8 +290,10 @@ class TestCsvItemExporter(TestBaseItemExporter): self.output = BytesIO() return CsvItemExporter(self.output, **kwargs) - def assertCsvEqual(self, first, second, msg=None): - def split_csv(csv): + def assertCsvEqual( + self, first: bytes | str, second: bytes | str, msg: str | None = None + ) -> None: + def split_csv(csv: bytes | str) -> list[list[str]]: return [ sorted(re.split(r"(,|\s+)", line)) for line in to_unicode(csv).splitlines(True) @@ -296,13 +301,15 @@ class TestCsvItemExporter(TestBaseItemExporter): assert split_csv(first) == split_csv(second), msg - def _check_output(self): + def _check_output(self) -> None: self.output.seek(0) self.assertCsvEqual( to_unicode(self.output.read()), "age,name\r\n22,John\xa3\r\n" ) - def assertExportResult(self, item, expected, **kwargs): + def assertExportResult( + self, item: Any, expected: bytes | str = b"", **kwargs: Any + ) -> None: fp = BytesIO() ie = CsvItemExporter(fp, **kwargs) ie.start_exporting() @@ -383,7 +390,6 @@ class TestCsvItemExporter(TestBaseItemExporter): with pytest.raises(UnicodeEncodeError): self.assertExportResult( item={"text": "W\u0275\u200brd"}, - expected=None, encoding="windows-1251", ) @@ -417,7 +423,7 @@ class TestCsvItemExporterDataclass(TestCsvItemExporter): class TestXmlItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: # We need a fresh instance for each exporter, because # XmlItemExporter.stream.__del__() closes the underlying file # (XmlItemExporter.finish_exporting() calls detach() but not all tests @@ -425,20 +431,22 @@ class TestXmlItemExporter(TestBaseItemExporter): self.output = BytesIO() return XmlItemExporter(self.output, **kwargs) - def assertXmlEquivalent(self, first, second, msg=None): - def xmltuple(elem): + def assertXmlEquivalent( + self, first: bytes, second: bytes, msg: str | None = None + ) -> None: + def xmltuple(elem: Any) -> list[Any]: children = list(elem.iterchildren()) if children: return [(child.tag, sorted(xmltuple(child))) for child in children] return [(elem.tag, [(elem.text, ())])] - def xmlsplit(xmlcontent): + def xmlsplit(xmlcontent: bytes) -> list[Any]: doc = lxml.etree.fromstring(xmlcontent) return xmltuple(doc) assert xmlsplit(first) == xmlsplit(second), msg - def assertExportResult(self, item, expected_value): + def assertExportResult(self, item: Any, expected_value: bytes) -> None: fp = BytesIO() ie = XmlItemExporter(fp) ie.start_exporting() @@ -447,7 +455,7 @@ class TestXmlItemExporter(TestBaseItemExporter): del ie # See the first “del self.ie” in this file for context. self.assertXmlEquivalent(fp.getvalue(), expected_value) - def _check_output(self): + def _check_output(self) -> None: expected_value = ( b'\n' b"22John\xc2\xa3" @@ -538,10 +546,10 @@ class TestJsonLinesItemExporter(TestBaseItemExporter): "age": {"name": "Maria", "age": {"name": "Joseph", "age": "22"}}, } - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return JsonLinesItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: exported = json.loads(to_unicode(self.output.getvalue().strip())) assert exported == ItemAdapter(self.i).asdict() @@ -582,14 +590,14 @@ class TestJsonLinesItemExporterDataclass(TestJsonLinesItemExporter): class TestJsonItemExporter(TestJsonLinesItemExporter): _expected_nested = [TestJsonLinesItemExporter._expected_nested] - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return JsonItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: exported = json.loads(to_unicode(self.output.getvalue().strip())) assert exported == [ItemAdapter(self.i).asdict()] - def assertTwoItemsExported(self, item): + def assertTwoItemsExported(self, item: Any) -> None: self.ie.start_exporting() self.ie.export_item(item) self.ie.export_item(item) @@ -658,7 +666,7 @@ class TestJsonItemExporter(TestJsonLinesItemExporter): class TestJsonItemExporterToBytes(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: kwargs["encoding"] = "latin" return JsonItemExporter(self.output, **kwargs) @@ -690,7 +698,9 @@ class TestCustomExporterItem: def test_exporter_custom_serializer(self): class CustomItemExporter(BaseItemExporter): - def serialize_field(self, field, name, value): + def serialize_field( + self, field: Mapping[str, Any] | Field, name: str, value: Any + ) -> Any: if name == "age": return str(int(value) + 1) return super().serialize_field(field, name, value) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 40a763efd..d0f6a297c 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -100,6 +100,8 @@ class InstrumentedFeedSlot(FeedSlot): """Instrumented FeedSlot subclass for keeping track of calls to start_exporting and finish_exporting.""" + update_listener: Callable[[str], None] + def start_exporting(self): self.update_listener("start") super().start_exporting() @@ -109,7 +111,7 @@ class InstrumentedFeedSlot(FeedSlot): super().finish_exporting() @classmethod - def subscribe__listener(cls, listener): + def subscribe__listener(cls, listener: IsExportingListener) -> None: cls.update_listener = listener.update @@ -119,7 +121,7 @@ class IsExportingListener: finish_exporting and when a call to finish_exporting has been made before a call to start_exporting.""" - def __init__(self): + def __init__(self) -> None: self.start_without_finish = False self.finish_without_start = False @@ -307,6 +309,7 @@ class TestFeedExport(TestFeedExportBase): } crawler = get_crawler(ItemSpider, settings) yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/success_count/FileFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/success_count/FileFeedStorage") == 1 @@ -330,6 +333,7 @@ class TestFeedExport(TestFeedExportBase): side_effect=store, ): yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/failed_count/FileFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/failed_count/FileFeedStorage") == 1 @@ -347,6 +351,7 @@ class TestFeedExport(TestFeedExportBase): } crawler = get_crawler(ItemSpider, settings) yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/success_count/FileFeedStorage" in crawler.stats.get_stats() assert "feedexport/success_count/StdoutFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/success_count/FileFeedStorage") == 1 @@ -487,7 +492,7 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_start_finish_exporting_no_items(self): - items = [] + items: list[Any] = [] settings = { "FEEDS": { self._random_temp_filename(): {"format": "json"}, @@ -526,7 +531,7 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_start_finish_exporting_no_items_exception(self): - items = [] + items: list[Any] = [] settings = { "FEEDS": { self._random_temp_filename(): {"format": "json"}, @@ -611,7 +616,7 @@ class TestFeedExport(TestFeedExportBase): items = [{"foo": "bar"}] header = ["foo"] rows = [{"foo": "bar"}] - settings = {"FEED_EXPORT_FIELDS": []} + settings: dict[str, Any] = {"FEED_EXPORT_FIELDS": []} await self.assertExportedCsv(items, header, rows) await self.assertExportedJsonLines(items, rows, settings) @@ -727,14 +732,14 @@ class TestFeedExport(TestFeedExportBase): def accepts(self, item): return isinstance(item, MyItem) - class CustomFilter2(scrapy.extensions.feedexport.ItemFilter): + class CustomFilter2(ItemFilter): def accepts(self, item): return "foo" in item.fields - class CustomFilter3(scrapy.extensions.feedexport.ItemFilter): + class CustomFilter3(ItemFilter): def accepts(self, item): return ( - isinstance(item, tuple(self.item_classes)) and item["foo"] == "bar1" + isinstance(item, tuple(self.item_classes)) and item["foo"] == "bar1" # type: ignore[index] ) formats = { @@ -834,7 +839,7 @@ class TestFeedExport(TestFeedExportBase): } for fmt, expected in formats.items(): - settings = { + settings: dict[str, Any] = { "FEEDS": { self._random_temp_filename(): {"format": fmt}, }, @@ -911,7 +916,7 @@ class TestFeedExport(TestFeedExportBase): {"key": "value"}, ] - test_cases = [ + test_cases: list[dict[str, Any]] = [ # JSON { "format": "json", @@ -1132,7 +1137,7 @@ class TestFeedExport(TestFeedExportBase): expected_with_title_csv = b"foo,bar\r\nFOO,BAR\r\n" expected_without_title_csv = b"FOO,BAR\r\n" - test_cases = [ + test_cases: list[dict[str, Any]] = [ # with title { "options": { @@ -1166,6 +1171,9 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_storage_file_no_postprocessing(self): class Storage: + open_file: IO[bytes] + store_file: IO[bytes] + def __init__(self, uri, *, feed_options=None): pass @@ -1187,6 +1195,10 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_storage_file_postprocessing(self): class Storage: + open_file: IO[bytes] + store_file: IO[bytes] + file_was_closed: bool + def __init__(self, uri, *, feed_options=None): pass @@ -1299,7 +1311,7 @@ class TestItemFilter: class TestFeedExportInit: def test_unsupported_storage(self): - settings = { + settings: dict[str, Any] = { "FEEDS": { "unsupported://uri": {}, }, diff --git a/tests/test_feedexport_postprocess.py b/tests/test_feedexport_postprocess.py index 36d8586ce..69696e65e 100644 --- a/tests/test_feedexport_postprocess.py +++ b/tests/test_feedexport_postprocess.py @@ -74,7 +74,13 @@ class TestFeedPostProcessedExports(TestFeedExportBase): return content - def get_gzip_compressed(self, data, compresslevel=9, mtime=0, filename=""): + def get_gzip_compressed( + self, + data: bytes, + compresslevel: int = 9, + mtime: int = 0, + filename: str = "", + ) -> bytes: data_stream = BytesIO() gzipf = gzip.GzipFile( fileobj=data_stream, @@ -539,11 +545,13 @@ class TestFeedPostProcessedExports(TestFeedExportBase): data = await self.exported_data(self.items, settings) - for filename, result in data.items(): + for filename, data_bytes in data.items(): + expected: Any + result: Any if "pickle" in filename: - expected, result = self.items[0], pickle.loads(result) + expected, result = self.items[0], pickle.loads(data_bytes) elif "marshal" in filename: - expected, result = self.items[0], marshal.loads(result) + expected, result = self.items[0], marshal.loads(data_bytes) else: - expected = filename_to_expected[filename] + expected, result = filename_to_expected[filename], data_bytes assert result == expected diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index 6f9e33449..d7fb5c9c2 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -95,27 +95,34 @@ class TestFileFeedStorage: assert storage.path == path +def get_test_spider(settings: dict[str, Any] | None = None) -> scrapy.Spider: + class TestSpider(scrapy.Spider): + name = "test_spider" + + crawler = get_crawler(settings_dict=settings) + return TestSpider.from_crawler(crawler) + + class TestFTPFeedStorage: - def get_test_spider(self, settings=None): - class TestSpider(scrapy.Spider): - name = "test_spider" - - crawler = get_crawler(settings_dict=settings) - return TestSpider.from_crawler(crawler) - - async def _store(self, uri, content, feed_options=None, settings=None): + async def _store( + self, + uri: str, + content: bytes, + feed_options: dict[str, Any] | None = None, + settings: dict[str, Any] | None = None, + ) -> None: crawler = get_crawler(settings_dict=settings or {}) storage = FTPFeedStorage.from_crawler( crawler, uri, feed_options=feed_options, ) - spider = self.get_test_spider() + spider = get_test_spider() file = storage.open(spider) file.write(content) await maybe_deferred_to_future(storage.store(file)) - def _assert_stored(self, path: Path, content): + def _assert_stored(self, path: Path, content: bytes) -> None: assert path.exists() try: assert path.read_bytes() == content @@ -165,7 +172,7 @@ class TestFTPFeedStorage: def test_uri_auth_quote(self): # RFC3986: 3.2.1. User Information pw_quoted = quote(string.punctuation, safe="") - st = FTPFeedStorage(f"ftp://foo:{pw_quoted}@example.com/some_path", {}) + st = FTPFeedStorage(f"ftp://foo:{pw_quoted}@example.com/some_path") assert st.password == string.punctuation def test_uri_without_hostname(self): @@ -181,24 +188,17 @@ class MyBlockingFeedStorage(BlockingFeedStorage): class TestBlockingFeedStorage: - def get_test_spider(self, settings=None): - class TestSpider(scrapy.Spider): - name = "test_spider" - - crawler = get_crawler(settings_dict=settings) - return TestSpider.from_crawler(crawler) - def test_default_temp_dir(self): b = MyBlockingFeedStorage() - storage_file = b.open(self.get_test_spider()) + storage_file = b.open(get_test_spider()) storage_dir = Path(storage_file.name).parent assert str(storage_dir) == tempfile.gettempdir() def test_temp_file(self, tmp_path): b = MyBlockingFeedStorage() - spider = self.get_test_spider({"FEED_TEMPDIR": str(tmp_path)}) + spider = get_test_spider({"FEED_TEMPDIR": str(tmp_path)}) storage_file = b.open(spider) storage_dir = Path(storage_file.name).parent assert storage_dir == tmp_path @@ -207,7 +207,7 @@ class TestBlockingFeedStorage: b = MyBlockingFeedStorage() invalid_path = tmp_path / "invalid_path" - spider = self.get_test_spider({"FEED_TEMPDIR": str(invalid_path)}) + spider = get_test_spider({"FEED_TEMPDIR": str(invalid_path)}) with pytest.raises(OSError, match="Not a Directory:"): b.open(spider=spider) @@ -311,7 +311,7 @@ class TestS3FeedStorage: assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" assert storage.region_name == region_name - assert storage.s3_client._client_config.region_name == region_name + assert storage.s3_client._client_config.region_name == region_name # type: ignore[attr-defined] def test_from_crawler_without_acl(self): settings = { @@ -353,7 +353,7 @@ class TestS3FeedStorage: ) assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" - assert storage.s3_client._client_config.region_name == "us-east-1" + assert storage.s3_client._client_config.region_name == "us-east-1" # type: ignore[attr-defined] def test_from_crawler_with_acl(self): settings = { @@ -394,7 +394,7 @@ class TestS3FeedStorage: assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" assert storage.region_name == region_name - assert storage.s3_client._client_config.region_name == region_name + assert storage.s3_client._client_config.region_name == region_name # type: ignore[attr-defined] def test_init_without_max_pool_connections(self) -> None: storage = S3FeedStorage("s3://mybucket/export.csv", "access_key", "secret_key") @@ -497,7 +497,7 @@ class TestGCSFeedStorage: def test_parse_empty_acl(self): pytest.importorskip("google.cloud.storage") - settings = {"GCS_PROJECT_ID": "123", "FEED_STORAGE_GCS_ACL": ""} + settings: dict[str, Any] = {"GCS_PROJECT_ID": "123", "FEED_STORAGE_GCS_ACL": ""} crawler = get_crawler(settings_dict=settings) storage = GCSFeedStorage.from_crawler(crawler, "gs://mybucket/export.csv") assert storage.acl is None diff --git a/tests/test_feedexport_uri_params.py b/tests/test_feedexport_uri_params.py index 150d8449f..21c291732 100644 --- a/tests/test_feedexport_uri_params.py +++ b/tests/test_feedexport_uri_params.py @@ -2,6 +2,7 @@ from __future__ import annotations import warnings from abc import ABC, abstractmethod +from typing import TYPE_CHECKING, Any import pytest @@ -10,16 +11,27 @@ from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.extensions.feedexport import FeedExporter from scrapy.utils.test import get_crawler +if TYPE_CHECKING: + from collections.abc import Callable + + from scrapy.crawler import Crawler + class TestURIParams(ABC): spider_name = "uri_params_spider" deprecated_options = False @abstractmethod - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: raise NotImplementedError - def _crawler_feed_exporter(self, settings): + def _crawler_feed_exporter( + self, settings: dict[str, Any] + ) -> tuple[Crawler, FeedExporter]: if self.deprecated_options: with pytest.warns( ScrapyDeprecationWarning, @@ -29,6 +41,7 @@ class TestURIParams(ABC): else: crawler = get_crawler(settings_dict=settings) feed_exporter = crawler.get_extension(FeedExporter) + assert feed_exporter is not None return crawler, feed_exporter def test_default(self): @@ -116,8 +129,12 @@ class TestURIParams(ABC): class TestURIParamsSetting(TestURIParams): deprecated_options = True - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): - extra_settings = {} + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: + extra_settings: dict[str, Any] = {} if uri_params: extra_settings["FEED_URI_PARAMS"] = uri_params return { @@ -129,8 +146,12 @@ class TestURIParamsSetting(TestURIParams): class TestURIParamsFeedOption(TestURIParams): deprecated_options = False - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): - options = { + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: + options: dict[str, Any] = { "format": "jl", } if uri_params: diff --git a/tests/test_item.py b/tests/test_item.py index 7b4c2e918..45732f157 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -1,4 +1,5 @@ from abc import ABCMeta +from typing import Any from unittest import mock import pytest @@ -7,9 +8,6 @@ from scrapy.item import Field, Item, ItemMeta class TestItem: - def assertSortedEqual(self, first, second, msg=None): - assert sorted(first) == sorted(second), msg - def test_simple(self): class TestItem(Item): name = Field() @@ -98,16 +96,16 @@ class TestItem: i = TestItem() with pytest.raises(AttributeError): - i.name = "john" + i.name = "john" # type: ignore[assignment] def test_custom_methods(self): class TestItem(Item): name = Field() - def get_name(self): + def get_name(self) -> Any: return self["name"] - def change_name(self, name): + def change_name(self, name: str) -> None: self["name"] = name i = TestItem() @@ -121,40 +119,40 @@ class TestItem: def test_metaclass(self): class TestItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] i = TestItem() i["name"] = "John" - assert list(i.keys()) == ["name"] - assert list(i.values()) == ["John"] + assert list(i.keys()) == ["name"] # type: ignore[operator] + assert list(i.values()) == ["John"] # type: ignore[operator] i["keys"] = "Keys" i["values"] = "Values" - self.assertSortedEqual(list(i.keys()), ["keys", "values", "name"]) - self.assertSortedEqual(list(i.values()), ["Keys", "Values", "John"]) + assert sorted(i.keys()) == ["keys", "name", "values"] # type: ignore[operator] + assert sorted(i.values()) == ["John", "Keys", "Values"] # type: ignore[operator] def test_metaclass_with_fields_attribute(self): class TestItem(Item): fields = {"new": Field(default="X")} item = TestItem(new="New") - self.assertSortedEqual(list(item.keys()), ["new"]) - self.assertSortedEqual(list(item.values()), ["New"]) + assert list(item.keys()) == ["new"] + assert list(item.values()) == ["New"] def test_fields_order(self): class TestItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] assert list(TestItem.fields) == ["name", "keys", "values"] def test_fields_order_inheritance(self): class ParentItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] class TestItem(ParentItem): extra = Field() @@ -169,16 +167,16 @@ class TestItem: def test_metaclass_inheritance(self): class ParentItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] class TestItem(ParentItem): keys = Field() i = TestItem() i["keys"] = 3 - assert list(i.keys()) == ["keys"] - assert list(i.values()) == [3] + assert list(i.keys()) == ["keys"] # type: ignore[operator] + assert list(i.values()) == [3] # type: ignore[operator] def test_metaclass_multiple_inheritance_simple(self): class A(Item): @@ -314,7 +312,7 @@ class TestItemMeta: def f(self): # For rationale of this see: # https://github.com/python/cpython/blob/ee1a81b77444c6715cbe610e951c655b6adab88b/Lib/test/test_super.py#L222 - return __class__ + return __class__ # type: ignore[name-defined] MyItem() diff --git a/tests/test_loader.py b/tests/test_loader.py index c094d25d8..969bf5b98 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -79,7 +79,7 @@ class TestBasicItemLoader: class InitializationTestMixin: - item_class: type | None = None + item_class: type def test_keep_single_value(self): """Loaded item should contain values from the initial item""" @@ -311,7 +311,7 @@ class TestSelectortemLoader: def test_init_method_with_base_response(self): """Selector should be None after initialization""" response = Response("https://scrapy.org") - l = ProcessorItemLoader(response=response) + l = ProcessorItemLoader(response=response) # type: ignore[arg-type] assert l.selector is None def test_init_method_with_response(self): @@ -461,6 +461,7 @@ class TestSubselectorLoader: l = NestedItemLoader(response=self.response) nl = l.nested_xpath("//header") + assert nl.selector is not None nl.add_xpath("name", "div/text()") nl.add_css("name_div", "#id") nl.add_value("name_value", nl.selector.xpath('div[@id = "id"]/text()').getall()) @@ -476,6 +477,7 @@ class TestSubselectorLoader: def test_nested_css(self): l = NestedItemLoader(response=self.response) nl = l.nested_css("header") + assert nl.selector is not None nl.add_xpath("name", "div/text()") nl.add_css("name_div", "#id") nl.add_value("name_value", nl.selector.xpath('div[@id = "id"]/text()').getall()) diff --git a/tests/test_pipeline_crawl.py b/tests/test_pipeline_crawl.py index 0681371ef..8b522255a 100644 --- a/tests/test_pipeline_crawl.py +++ b/tests/test_pipeline_crawl.py @@ -23,7 +23,10 @@ if TYPE_CHECKING: class MediaDownloadSpider(SimpleSpider): name = "mediadownload" - def _process_url(self, url): + media_key: str + media_urls_key: str + + def _process_url(self, url: str) -> str: return url def parse(self, response): @@ -44,14 +47,15 @@ class MediaDownloadSpider(SimpleSpider): class BrokenLinksMediaDownloadSpider(MediaDownloadSpider): name = "brokenmedia" - def _process_url(self, url): + def _process_url(self, url: str) -> str: return url + ".foo" class RedirectedMediaDownloadSpider(MediaDownloadSpider): name = "redirectedmedia" - def _process_url(self, url): + def _process_url(self, url: str) -> str: + assert self.mockserver return add_or_replace_parameter( self.mockserver.url("/redirect-to"), "goto", url ) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4e7fb118b..3a8a3d0f7 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -22,6 +22,7 @@ from itemadapter import ItemAdapter from twisted.internet.defer import Deferred from twisted.python.failure import Failure +from scrapy.crawler import Crawler from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request, Response from scrapy.item import Field, Item @@ -75,7 +76,7 @@ class DeferredFSFilesStore(FSFilesStore): """A simple store with persist_file() returning a deferred.""" def persist_file(self, path, buf, info, meta=None, headers=None): - deferred = Deferred() + deferred: Deferred[None] = Deferred() # short-hand super() doesn't work in nested functions parent_persist_file = super().persist_file @@ -152,7 +153,7 @@ class TestFilesPipeline: file_path( Request("http://www.dorma.co.uk/images/product_details/2532"), response=Response("http://www.dorma.co.uk/images/product_details/2532"), - info=object(), + info=object(), # type: ignore[arg-type] ) == "full/244e0dd7d96a3b7b01f54eded250c9e272577aa1" ) @@ -383,7 +384,7 @@ class TestFilesPipeline: """ class CustomFilesPipeline(FilesPipeline): - def file_path(self, request, response=None, info=None, item=None): + def file_path(self, request, response=None, info=None, item=None) -> str: return f"full/{item.get('path')}" file_path = CustomFilesPipeline.from_crawler( @@ -476,7 +477,7 @@ class TestFilesPipeline: item["file_urls"] = bad_type with pytest.raises(TypeError, match="file_urls must be a list of URLs"): - list(pipeline.get_media_requests(item, None)) + list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] class TestFilesPipelineFieldsMixin(ABC): @@ -491,10 +492,10 @@ class TestFilesPipelineFieldsMixin(ABC): pipeline = FilesPipeline.from_crawler( get_crawler(None, {"FILES_STORE": tmp_path}) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] assert requests[0].url == url results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + item = pipeline.item_completed(results, item, None) # type: ignore[arg-type] files = ItemAdapter(item).get("files") assert files == [results[0][1]] assert isinstance(item, self.item_class) @@ -512,10 +513,10 @@ class TestFilesPipelineFieldsMixin(ABC): }, ) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] assert requests[0].url == url results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + item = pipeline.item_completed(results, item, None) # type: ignore[arg-type] custom_files = ItemAdapter(item).get("custom_files") assert custom_files == [results[0][1]] assert isinstance(item, self.item_class) @@ -581,8 +582,10 @@ class TestFilesPipelineCustomSettings: ("FILES_RESULT_FIELD", "FILES_RESULT_FIELD", "files_result_field"), } - def _generate_fake_settings(self, tmp_path, prefix=None): - def random_string(): + def _generate_fake_settings( + self, tmp_path: Path, prefix: str | None = None + ) -> dict[str, Any]: + def random_string() -> str: return "".join([chr(random.randint(97, 123)) for _ in range(10)]) settings = { @@ -599,7 +602,7 @@ class TestFilesPipelineCustomSettings: for k, v in settings.items() } - def _generate_fake_pipeline(self): + def _generate_fake_pipeline(self) -> type[FilesPipeline]: class UserDefinedFilePipeline(FilesPipeline): EXPIRES = 1001 FILES_URLS_FIELD = "alfa" @@ -739,14 +742,14 @@ class TestFilesPipelineCustomSettings: def test_file_pipeline_using_pathlike_objects(self, tmp_path): class CustomFilesPipelineWithPathLikeDir(FilesPipeline): - def file_path(self, request, response=None, info=None, *, item=None): - return Path("subdir") / Path(request.url).name + def file_path(self, request, response=None, info=None, *, item=None) -> str: + return str(Path("subdir") / Path(request.url).name) pipeline = CustomFilesPipelineWithPathLikeDir.from_crawler( get_crawler(None, {"FILES_STORE": tmp_path}) ) request = Request("http://example.com/image01.jpg") - assert pipeline.file_path(request) == Path("subdir/image01.jpg") + assert pipeline.file_path(request) == str(Path("subdir/image01.jpg")) class TestFSFilesStore: @@ -1092,7 +1095,7 @@ class TestFTPFileStore: store.port, store.username, store.password, - store.USE_ACTIVE_MODE, + bool(store.USE_ACTIVE_MODE), ) assert data == content @@ -1160,7 +1163,7 @@ class TestBuildFromCrawler: _from_crawler_called = False @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> "Pipeline": settings = crawler.settings store_uri = settings["FILES_STORE"] o = cls(store_uri, crawler=crawler) @@ -1179,7 +1182,7 @@ def test_files_pipeline_raises_notconfigured_when_files_store_invalid(store): settings = Settings() settings.clear() settings.set("FILES_STORE", store, priority="cmdline") - crawler = get_crawler(settings_dict=settings) + crawler = get_crawler(settings_dict=dict(settings)) with pytest.raises(NotConfigured): FilesPipeline.from_crawler(crawler) diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 19e61579f..43316f125 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -91,7 +91,7 @@ class TestImagesPipeline: file_path( Request("http://www.dorma.co.uk/images/product_details/2532"), response=Response("http://www.dorma.co.uk/images/product_details/2532"), - info=object(), + info=DUMMY_SPIDER_INFO, ) == "full/244e0dd7d96a3b7b01f54eded250c9e272577aa1.jpg" ) @@ -120,7 +120,7 @@ class TestImagesPipeline: Request("file:///tmp/some.name/foo"), name, response=Response("file:///tmp/some.name/foo"), - info=object(), + info=DUMMY_SPIDER_INFO, ) == "thumbs/50/850233df65a5b83361798f532f1fc549cd13cbe9.jpg" ) @@ -133,7 +133,7 @@ class TestImagesPipeline: class CustomImagesPipeline(ImagesPipeline): def thumb_path( self, request, thumb_id, response=None, info=None, item=None - ): + ) -> str: return f"thumb/{thumb_id}/{item.get('path')}" thumb_path = CustomImagesPipeline.from_crawler( @@ -159,11 +159,23 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp1, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp1, request=req, info=DUMMY_SPIDER_INFO + ) + ) with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp2, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp2, request=req, info=DUMMY_SPIDER_INFO + ) + ) with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp3, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp3, request=req, info=DUMMY_SPIDER_INFO + ) + ) def test_get_images(self): self.pipeline.min_width = 0 @@ -176,7 +188,7 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") get_images_gen = self.pipeline.get_images( - response=resp, request=req, info=object() + response=resp, request=req, info=DUMMY_SPIDER_INFO ) path, new_im, new_buf = next(get_images_gen) @@ -201,7 +213,7 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") get_images_gen = self.pipeline.get_images( - response=resp, request=req, info=object() + response=resp, request=req, info=DUMMY_SPIDER_INFO ) path, new_im, _ = next(get_images_gen) @@ -230,7 +242,7 @@ class TestImagesPipeline: def test_convert_image(self): SIZE = (100, 100) # straight forward case: RGB and JPEG - COLOUR = (0, 127, 255) + COLOUR: tuple[int, ...] = (0, 127, 255) im, buf = _create_image("JPEG", "RGB", SIZE, COLOUR) converted, converted_buf = self.pipeline.convert_image(im, response_body=buf) assert converted.mode == "RGB" @@ -296,7 +308,7 @@ class TestImagesPipeline: item["image_urls"] = bad_type with pytest.raises(TypeError, match="image_urls must be a list of URLs"): - list(pipeline.get_media_requests(item, None)) + list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) class TestImagesPipelineFieldsMixin(ABC): @@ -311,10 +323,10 @@ class TestImagesPipelineFieldsMixin(ABC): pipeline = ImagesPipeline.from_crawler( get_crawler(None, {"IMAGES_STORE": "s3://example/images/"}) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) assert requests[0].url == url - results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + results: Any = [(True, {"url": url})] + item = pipeline.item_completed(results, item, DUMMY_SPIDER_INFO) images = ItemAdapter(item).get("images") assert images == [results[0][1]] assert isinstance(item, self.item_class) @@ -332,10 +344,10 @@ class TestImagesPipelineFieldsMixin(ABC): }, ) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) assert requests[0].url == url - results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + results: Any = [(True, {"url": url})] + item = pipeline.item_completed(results, item, DUMMY_SPIDER_INFO) custom_images = ItemAdapter(item).get("custom_images") assert custom_images == [results[0][1]] assert isinstance(item, self.item_class) @@ -410,13 +422,15 @@ class TestImagesPipelineCustomSettings: "IMAGES_RESULT_FIELD": "images", } - def _generate_fake_settings(self, tmp_path, prefix=None): + def _generate_fake_settings( + self, tmp_path: Path, prefix: str | None = None + ) -> dict[str, Any]: """ :param prefix: string for setting keys :return: dictionary of image pipeline settings """ - def random_string(): + def random_string() -> str: return "".join([chr(random.randint(97, 123)) for _ in range(10)]) settings = { @@ -439,7 +453,7 @@ class TestImagesPipelineCustomSettings: for k, v in settings.items() } - def _generate_fake_pipeline_subclass(self): + def _generate_fake_pipeline_subclass(self) -> type[ImagesPipeline]: """ :return: ImagePipeline class will all uppercase attributes set. """ diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index ba1c18006..50787d7b7 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from typing import TYPE_CHECKING, Any, cast from unittest.mock import MagicMock import pytest @@ -10,7 +11,12 @@ from scrapy import signals from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.pipelines.files import FileException -from scrapy.pipelines.media import MediaPipeline, _MediaRequestFiltered +from scrapy.pipelines.media import ( + FileInfo, + FileInfoOrError, + MediaPipeline, + _MediaRequestFiltered, +) from scrapy.utils.defer import _defer_sleep_async from scrapy.utils.log import failure_to_exc_info from scrapy.utils.signal import disconnect_all @@ -19,21 +25,48 @@ from scrapy.utils.test import get_crawler from tests.utils.decorators import coroutine_test from tests.utils.media_pipelines import mocked_download_func +if TYPE_CHECKING: + from collections.abc import Awaitable + + from twisted.internet.defer import Deferred + + from scrapy.crawler import Crawler + class UserDefinedPipeline(MediaPipeline): - def media_to_download(self, request, info, *, item=None): - pass + def media_to_download( + self, request: Request, info: MediaPipeline.SpiderInfo, *, item: Any = None + ) -> Deferred[FileInfo | None] | None: + return None - def get_media_requests(self, item, info): - pass + def get_media_requests( + self, item: Any, info: MediaPipeline.SpiderInfo + ) -> list[Request]: + return [] - def media_downloaded(self, response, request, info, *, item=None): - return {} + def media_downloaded( + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: + return cast("FileInfo", {}) - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: failure.raiseException() - def file_path(self, request, response=None, info=None, *, item=None): + def file_path( + self, + request: Request, + response: Response | None = None, + info: MediaPipeline.SpiderInfo | None = None, + *, + item: Any = None, + ) -> str: return "" @@ -48,8 +81,14 @@ class TestBaseMediaPipeline: self.pipe = self.pipeline_class.from_crawler(crawler) self.pipe.open_spider() self.info = self.pipe.spiderinfo + assert crawler.request_fingerprinter is not None self.fingerprint = crawler.request_fingerprinter.fingerprint + @property + def mocked_pipe(self) -> MockedMediaPipeline: + assert isinstance(self.pipe, MockedMediaPipeline) + return self.pipe + def teardown_method(self): for name, signal in vars(signals).items(): if not name.startswith("_"): @@ -121,11 +160,13 @@ class TestBaseMediaPipeline: # When calling the method that caches the Request's result ... self.pipe._cache_result_and_execute_waiters(failure, fp, info) # ... it should store the Twisted Failure ... - assert info.downloaded[fp] == failure + downloaded = info.downloaded[fp] + assert downloaded == failure # ... encapsulating the original FileException ... - assert info.downloaded[fp].value == file_exc + assert isinstance(downloaded, Failure) + assert downloaded.value == file_exc # ... but it should not store the StopIteration exception on its context - context = getattr(info.downloaded[fp].value, "__context__", None) + context = getattr(downloaded.value, "__context__", None) assert context is None def test_default_item_completed(self, caplog: pytest.LogCaptureFixture) -> None: @@ -134,7 +175,7 @@ class TestBaseMediaPipeline: # Check that failures are logged by default fail = Failure(Exception()) - results = [(True, 1), (False, fail)] + results: Any = [(True, 1), (False, fail)] caplog.clear() new_item = self.pipe.item_completed(results, item, self.info) @@ -158,7 +199,7 @@ class TestBaseMediaPipeline: by item_completed(), as they are not download errors.""" item = {"name": "name"} fail = Failure(_MediaRequestFiltered("Filtered offsite request")) - results = [(True, 1), (False, fail)] + results: Any = [(True, 1), (False, fail)] with caplog.at_level(logging.DEBUG): new_item = self.pipe.item_completed(results, item, self.info) @@ -174,29 +215,44 @@ class TestBaseMediaPipeline: class MockedMediaPipeline(UserDefinedPipeline): - def __init__(self, *args, crawler=None, **kwargs): + def __init__(self, *args: Any, crawler: Crawler, **kwargs: Any): super().__init__(*args, crawler=crawler, **kwargs) - self._mockcalled = [] + self._mockcalled: list[str] = [] - def media_to_download(self, request, info, *, item=None): + def media_to_download( + self, request: Request, info: MediaPipeline.SpiderInfo, *, item: Any = None + ) -> Deferred[FileInfo | None] | None: self._mockcalled.append("media_to_download") if "result" in request.meta: return request.meta.get("result") return super().media_to_download(request, info) - def get_media_requests(self, item, info): + def get_media_requests( + self, item: Any, info: MediaPipeline.SpiderInfo + ) -> list[Request]: self._mockcalled.append("get_media_requests") - return item.get("requests") + return item.get("requests") # type: ignore[no-any-return] - def media_downloaded(self, response, request, info, *, item=None): + def media_downloaded( + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: self._mockcalled.append("media_downloaded") return super().media_downloaded(response, request, info) - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: self._mockcalled.append("media_failed") return super().media_failed(failure, request, info) - def item_completed(self, results, item, info): + def item_completed( + self, results: list[FileInfoOrError], item: Any, info: MediaPipeline.SpiderInfo + ) -> Any: self._mockcalled.append("item_completed") item = super().item_completed(results, item, info) item["results"] = results @@ -204,7 +260,14 @@ class MockedMediaPipeline(UserDefinedPipeline): class AsyncMediaDownloadedPipeline(MockedMediaPipeline): - async def media_downloaded(self, response, request, info, *, item=None): + async def media_downloaded( # type: ignore[override] + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: return super().media_downloaded(response, request, info) @@ -212,7 +275,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): pipeline_class = MockedMediaPipeline def _errback(self, result): - self.pipe._mockcalled.append("request_errback") + self.mocked_pipe._mockcalled.append("request_errback") return result @coroutine_test @@ -226,7 +289,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): item = {"requests": req} new_item = await self.pipe.process_item(item) assert new_item["results"] == [(True, {})] - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_downloaded", @@ -248,7 +311,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): assert new_item["results"][0][0] is False assert isinstance(new_item["results"][0][1], Failure) assert new_item["results"][0][1].value == exc - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", @@ -270,7 +333,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): assert new_item["results"][1][0] is False assert isinstance(new_item["results"][1][1], Failure) assert new_item["results"][1][1].value == exc - m = self.pipe._mockcalled + m = self.mocked_pipe._mockcalled # only once assert m[0] == "get_media_requests" # first hook called assert m.count("get_media_requests") == 1 @@ -294,7 +357,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): # returns iterable of Requests req1 = Request("http://url1") req2 = Request("http://url2") - item = {"requests": iter([req1, req2])} + item = {"requests": iter([req1, req2])} # type: ignore[dict-item] new_item = await self.pipe.process_item(item) assert new_item is item assert self.fingerprint(req1) in self.info.downloaded @@ -304,7 +367,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): async def test_results_are_cached_across_multiple_items(self): rsp1 = Response("http://url1") req1 = Request("http://url1", meta={"response": rsp1}) - item = {"requests": req1} + item: dict[str, Any] = {"requests": req1} new_item = await self.pipe.process_item(item) assert new_item is item assert new_item["results"] == [(True, {})] @@ -335,7 +398,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): new_item = await self.pipe.process_item({"requests": req2}) assert new_item["results"][0][0] is False assert new_item["results"][0][1].value is exc - assert self.pipe._mockcalled.count("media_to_download") == 1 + assert self.mocked_pipe._mockcalled.count("media_to_download") == 1 @coroutine_test async def test_cached_failure_calls_errback(self): @@ -347,13 +410,13 @@ class TestMediaPipeline(TestBaseMediaPipeline): ) def errback(failure): - self.pipe._mockcalled.append("request_errback") + self.mocked_pipe._mockcalled.append("request_errback") return {"recovered": failure.value} req = Request("http://url1", errback=errback) new_item = await self.pipe.process_item({"requests": req}) assert new_item["results"] == [(True, {"recovered": exc})] - assert self.pipe._mockcalled.count("request_errback") == 1 + assert self.mocked_pipe._mockcalled.count("request_errback") == 1 @coroutine_test async def test_results_are_cached_for_requests_of_single_item(self): @@ -362,14 +425,14 @@ class TestMediaPipeline(TestBaseMediaPipeline): req2 = Request( req1.url, meta={"response": Response("http://donot.download.me")} ) - item = {"requests": [req1, req2]} + item: dict[str, Any] = {"requests": [req1, req2]} new_item = await self.pipe.process_item(item) assert new_item is item assert new_item["results"] == [(True, {}), (True, {})] @coroutine_test async def test_wait_if_request_is_downloading(self): - def _check_downloading(response): + def _check_downloading(response: Response) -> Response: fp = self.fingerprint(req1) assert fp in self.info.downloading assert fp in self.info.waiting @@ -398,7 +461,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): item = {"requests": req} new_item = await self.pipe.process_item(item) assert new_item["results"] == [(True, "ITSME")] - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "item_completed", @@ -422,7 +485,9 @@ class TestAsyncMediaDownloaded(TestMediaPipeline): class TestMediaPipelineAllowRedirectSettings: - def _assert_request_no3xx(self, pipeline_class, settings): + def _assert_request_no3xx( + self, pipeline_class: type[MediaPipeline], settings: dict[str, Any] + ) -> None: pipe = pipeline_class(crawler=get_crawler(None, settings)) request = Request("http://url") pipe._modify_media_request(request) @@ -477,7 +542,7 @@ class TestBuildFromCrawler: self._init_called = True @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> Pipeline: settings = crawler.settings store_uri = settings["FILES_STORE"] o = cls(store_uri, settings=settings, crawler=crawler) @@ -493,9 +558,10 @@ class TestBuildFromCrawler: def test_has_from_crawler(self): class Pipeline(UserDefinedPipeline): _from_crawler_called = False + store_uri: str @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> Pipeline: settings = crawler.settings o = super().from_crawler(crawler) o._from_crawler_called = True @@ -509,7 +575,9 @@ class TestBuildFromCrawler: class MediaFailedNonePipeline(MockedMediaPipeline): - def media_failed(self, failure, request, info): + def media_failed( # type: ignore[override] + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> None: self._mockcalled.append("media_failed") @@ -524,7 +592,7 @@ class TestMediaFailedNone(TestBaseMediaPipeline): req = Request("http://url1", meta={"response": Exception("foo")}) new_item = await self.pipe.process_item({"requests": req}) assert new_item["results"] == [(True, None)] - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", @@ -533,7 +601,9 @@ class TestMediaFailedNone(TestBaseMediaPipeline): class MediaFailedFailurePipeline(MockedMediaPipeline): - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: self._mockcalled.append("media_failed") return failure # deprecated @@ -544,7 +614,7 @@ class TestMediaFailedFailure(TestBaseMediaPipeline): pipeline_class = MediaFailedFailurePipeline def _errback(self, result): - self.pipe._mockcalled.append("request_errback") + self.mocked_pipe._mockcalled.append("request_errback") return result @coroutine_test @@ -565,7 +635,7 @@ class TestMediaFailedFailure(TestBaseMediaPipeline): assert new_item["results"][0][0] is False assert isinstance(new_item["results"][0][1], Failure) assert new_item["results"][0][1].value == exc - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index e65b5bc32..4b34946ae 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -47,7 +47,7 @@ class DeferredPipeline: return succeed(None) def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() d.addCallback(self.cb) d.callback(item) return d @@ -55,7 +55,7 @@ class DeferredPipeline: class AsyncDefPipeline: async def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() call_later(0, d.callback, None) await maybe_deferred_to_future(d) item["pipeline_passed"] = True @@ -64,7 +64,7 @@ class AsyncDefPipeline: class AsyncDefAsyncioPipeline: async def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() loop = asyncio.get_event_loop() loop.call_later(0, d.callback, None) await deferred_to_future(d) @@ -75,12 +75,12 @@ class AsyncDefAsyncioPipeline: class AsyncDefNotAsyncioPipeline: async def process_item(self, item): - d1 = Deferred() + d1: Deferred[Any] = Deferred() from twisted.internet import reactor reactor.callLater(0, d1.callback, None) await d1 - d2 = Deferred() + d2: Deferred[Any] = Deferred() reactor.callLater(0, d2.callback, None) await maybe_deferred_to_future(d2) item["pipeline_passed"] = True @@ -120,6 +120,8 @@ class OpenSpiderExceptionAsyncPipeline: class ItemSpider(Spider): name = "itemspider" + mockserver: MockServer + async def start(self): yield Request(self.mockserver.url("/status?n=200")) From bee31890a23f403ca31b084dc3ec73c2c99eec43 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 19:56:03 +0200 Subject: [PATCH 34/42] Report exceptions from Spider.start() (#7884) --- docs/topics/signals.rst | 13 ++++-- docs/topics/spiders.rst | 32 +++++++++++++++ docs/topics/stats.rst | 17 +++++--- scrapy/core/engine.py | 23 ++++++++++- scrapy/exceptions.py | 8 +++- tests/test_crawler_subprocess.py | 2 +- tests/test_engine_loop.py | 69 ++++++++++++++++++++++++++++++++ 7 files changed, 152 insertions(+), 12 deletions(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index f060710a4..acbf7ddef 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -338,15 +338,22 @@ spider_error .. signal:: spider_error .. function:: spider_error(failure, response, spider) - Sent when a spider callback generates an error (i.e. raises an exception). + Sent when a spider callback or the :meth:`~scrapy.Spider.start` method of a + spider generates an error (i.e. raises an exception). + + .. versionchanged:: VERSION + Exceptions from :meth:`~scrapy.Spider.start` are also reported, see + :ref:`start-error`. This signal does not support :ref:`asynchronous handlers `. :param failure: the exception raised :type failure: twisted.python.failure.Failure - :param response: the response being processed when the exception was raised - :type response: :class:`~scrapy.http.Response` object + :param response: the response being processed when the exception was + raised, or ``None`` if the exception came from + :meth:`~scrapy.Spider.start`. + :type response: :class:`~scrapy.http.Response` | ``None`` :param spider: the spider which raised the exception :type spider: :class:`~scrapy.Spider` object diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index e68c208b7..95c80d5dc 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -411,6 +411,38 @@ scheduled requests: await self.crawler.signals.wait_for(signals.scheduler_empty) yield item_or_request +.. _start-error: + +Handling start errors +--------------------- + +An exception raised by :meth:`~scrapy.Spider.start` ends its iteration, so any +remaining start items and requests are never sent. Scrapy logs the exception, +sends the :signal:`spider_error` signal, and, once the already scheduled +requests are done, closes the spider with the ``start_error`` +:stat:`finish_reason`. + +.. versionchanged:: VERSION + The close reason used to be ``finished``, and neither the + :signal:`spider_error` signal nor the :stat:`spider_exceptions/count` stat + reported the exception. + +To keep the iteration going, catch the exception yourself: + +.. code-block:: python + + async def start(self): + for url in self.start_urls: + try: + request = Request(url) + except ValueError: + self.logger.exception(f"Skipping start URL {url}") + else: + yield request + +To stop the crawl instead, and choose your own :stat:`finish_reason`, raise +:exc:`~scrapy.exceptions.CloseSpider`. + .. _builtin-spiders: Generic Spiders diff --git a/docs/topics/stats.rst b/docs/topics/stats.rst index b558c1cf2..6fb7783d1 100644 --- a/docs/topics/stats.rst +++ b/docs/topics/stats.rst @@ -301,6 +301,10 @@ one per actual value of the placeholder. - ``shutdown``: the crawl was interrupted, e.g. by a system signal such as ``SIGINT`` (:kbd:`Ctrl-C`). + - ``start_error``: :meth:`~scrapy.Spider.start` raised an exception, so + some :ref:`start requests ` may never have been sent, + see :ref:`start-error`. + Third-party components and your own code may use any other reason, e.g. by raising :exc:`~scrapy.exceptions.CloseSpider` with it. @@ -721,18 +725,21 @@ one per actual value of the placeholder. .. stat:: spider_exceptions/count ``spider_exceptions/count`` - Number of unhandled exceptions raised by spider callbacks. + Number of unhandled exceptions raised by spider callbacks or by + :meth:`~scrapy.Spider.start`. - Set by the :ref:`scraper `. + Set by the :ref:`engine ` and the :ref:`scraper + `. .. stat:: spider_exceptions/{exception} ``spider_exceptions/{exception}`` - Number of unhandled exceptions raised by spider callbacks, per exception, - where ``{exception}`` is the class name of the exception, e.g. + Same as :stat:`spider_exceptions/count`, per exception, where + ``{exception}`` is the class name of the exception, e.g. ``spider_exceptions/ValueError``. - Set by the :ref:`scraper `. + Set by the :ref:`engine ` and the :ref:`scraper + `. .. stat:: start_time diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index efac5f71c..b7a1e0fcd 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -125,6 +125,9 @@ class ExecutionEngine: ] = spider_closed_callback self.start_time: float | None = None self._start: AsyncIterator[Any] | None = None + # Whether Spider.start() raised, i.e. some start items or requests may + # never have reached the engine. + self._start_error: bool = False self._closewait: Deferred[None] | None = None self._start_request_processing_awaitable: ( asyncio.Future[None] | Deferred[None] | None @@ -277,13 +280,30 @@ class ExecutionEngine: item_or_request = await anext(self._start) except StopAsyncIteration: self._start = None + except CloseSpider as exception: + self._start = None + _schedule_coro( + self.close_spider_async(reason=exception.reason or "cancelled") + ) except Exception as exception: self._start = None + self._start_error = True exception_traceback = format_exc() logger.error( f"Error while reading start items and requests: {exception}.\n{exception_traceback}", exc_info=True, ) + self.signals.send_catch_log( + signal=signals.spider_error, + failure=Failure(), + response=None, + spider=self.spider, + ) + assert self.crawler.stats + self.crawler.stats.inc_value("spider_exceptions/count") + self.crawler.stats.inc_value( + f"spider_exceptions/{type(exception).__name__}" + ) else: if not self.spider: return # spider already closed @@ -579,7 +599,8 @@ class ExecutionEngine: if DontCloseSpider in detected_ex: return if self.spider_is_idle(): - ex = detected_ex.get(CloseSpider, CloseSpider(reason="finished")) + default_reason = "start_error" if self._start_error else "finished" + ex = detected_ex.get(CloseSpider, CloseSpider(reason=default_reason)) assert isinstance(ex, CloseSpider) # typing _schedule_coro(self.close_spider_async(reason=ex.reason)) diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index ccaccddf2..fa5bc1c1e 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -56,8 +56,12 @@ class DontCloseSpider(Exception): class CloseSpider(Exception): - """Raised from a :ref:`spider callback ` to request the - spider to be closed/stopped. + """Raised from a :ref:`spider callback ` or from + :meth:`~scrapy.Spider.start` to request the spider to be closed/stopped. + + .. versionchanged:: VERSION + Raising it from :meth:`~scrapy.Spider.start` closes the spider, instead + of being reported as a start error. *reason* is a string with the reason for closing. diff --git a/tests/test_crawler_subprocess.py b/tests/test_crawler_subprocess.py index 240482586..defee1041 100644 --- a/tests/test_crawler_subprocess.py +++ b/tests/test_crawler_subprocess.py @@ -400,7 +400,7 @@ class TestAsyncCrawlerProcessSubprocess(TestCrawlerProcessSubprocessBase): def test_reactorless_import_hook(self) -> None: log = self.run_script("reactorless_import_hook.py") assert "Not using a Twisted reactor" in log - assert "Spider closed (finished)" in log + assert "Spider closed (start_error)" in log assert "ImportError: Import of twisted.internet.reactor is forbidden" in log def test_reactorless_import_hook_uninstall(self) -> None: diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index 14ec3d184..1ecf8b8de 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING, Any from scrapy import Request, Spider, signals from scrapy.core.scheduler import BaseScheduler +from scrapy.exceptions import CloseSpider from scrapy.utils.asyncio import call_later, sleep from scrapy.utils.test import get_crawler from tests.mockserver.http import MockServer @@ -140,6 +141,74 @@ class TestMain: assert crawler.stats.get_value("finish_reason") == "shutdown" assert not actual_urls + @coroutine_test + async def test_start_error(self, caplog: pytest.LogCaptureFixture) -> None: + class TestSpider(Spider): + name = "test" + + async def start(self): + yield Request("data:,a") + raise ValueError + + def parse(self, response): + pass + + actual_urls = [] + errors = [] + + def track_url(request, spider): + actual_urls.append(request.url) + + def track_error(failure, response, spider): + errors.append((failure, response)) + + settings = {"SCHEDULER": MemoryScheduler} + crawler = get_crawler(TestSpider, settings_dict=settings) + crawler.signals.connect(track_url, signals.request_reached_downloader) + crawler.signals.connect(track_error, signals.spider_error) + + caplog.clear() + with caplog.at_level(ERROR): + await crawler.crawl_async() + + # The requests yielded before the exception are still crawled. + assert actual_urls == ["data:,a"] + assert len(caplog.records) == 1 + assert len(errors) == 1 + failure, response = errors[0] + assert isinstance(failure.value, ValueError) + assert response is None + assert crawler.stats + assert crawler.stats.get_value("finish_reason") == "start_error" + assert crawler.stats.get_value("spider_exceptions/count") == 1 + assert crawler.stats.get_value("spider_exceptions/ValueError") == 1 + + @coroutine_test + async def test_close_spider_from_start( + self, caplog: pytest.LogCaptureFixture + ) -> None: + class TestSpider(Spider): + name = "test" + + async def start(self): + yield Request("data:,a") + raise CloseSpider("my_reason") + + def parse(self, response): + pass + + settings = {"SCHEDULER": MemoryScheduler} + crawler = get_crawler(TestSpider, settings_dict=settings) + + caplog.clear() + with caplog.at_level(ERROR): + await crawler.crawl_async() + + assert not caplog.records + assert crawler.stats + assert crawler.stats.get_value("finish_reason") == "my_reason" + assert crawler.stats.get_value("spider_exceptions/count") is None + class TestRequestSendOrder: seconds = 0.1 # increase if flaky From aadfd153275258262ec94e531f81757a3520bd39 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 19:58:35 +0200 Subject: [PATCH 35/42] Do not mangle log messages when a log formatter returns empty or tuple args (#7936) --- scrapy/utils/log.py | 14 +++++++++----- tests/test_utils_log.py | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 7645b235e..ee357c031 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -245,7 +245,7 @@ class LogCounterHandler(logging.Handler): def logformatter_adapter( logkws: LogFormatterResult, -) -> tuple[int, str, dict[str, Any] | tuple[Any, ...]]: +) -> tuple[Any, ...]: """ Helper that takes the dictionary output from the methods in LogFormatter and adapts it into a tuple of positional arguments for logger.log calls. @@ -253,10 +253,14 @@ def logformatter_adapter( level = logkws.get("level", logging.INFO) message = logkws.get("msg") or "" - # NOTE: This also handles 'args' being an empty dict, that case doesn't - # play well in logger.log calls - args = cast("dict[str, Any]", logkws) if not logkws.get("args") else logkws["args"] - + args = logkws.get("args") + # logging interpolates the message whenever it receives any positional + # argument, so empty args are left out. Tuple args become one positional + # argument each, while a dict is a single positional argument. + if not args: + return (level, message) + if isinstance(args, tuple): + return (level, message, *args) return (level, message, args) diff --git a/tests/test_utils_log.py b/tests/test_utils_log.py index 7f5301387..42b2b95fd 100644 --- a/tests/test_utils_log.py +++ b/tests/test_utils_log.py @@ -5,7 +5,7 @@ import logging import re import sys from io import StringIO -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast import pytest from twisted.python.failure import Failure @@ -16,6 +16,7 @@ from scrapy.utils.log import ( StreamLogger, TopLevelFormatter, failure_to_exc_info, + logformatter_adapter, ) from scrapy.utils.test import get_crawler from tests.spiders import LogSpider @@ -24,6 +25,7 @@ if TYPE_CHECKING: from collections.abc import Generator, Mapping, MutableMapping from scrapy.crawler import Crawler + from scrapy.logformatter import LogFormatterResult class TestFailureToExcInfo: @@ -311,3 +313,36 @@ class TestLoggingWithExtra: assert log_contents["message"] == log_message assert self.regex_pattern.match(log_contents["spider"]) assert log_contents["important_info"] == extra["important_info"] + + +class TestLogformatterAdapter: + @staticmethod + def _log(caplog: pytest.LogCaptureFixture, logkws: LogFormatterResult) -> str: + with caplog.at_level(logging.INFO): + logging.getLogger(__name__).log(*logformatter_adapter(logkws)) + return caplog.records[-1].getMessage() + + @pytest.mark.parametrize("args", [None, {}, ()]) + def test_empty_args( + self, + caplog: pytest.LogCaptureFixture, + args: dict[str, Any] | tuple[Any, ...] | None, + ) -> None: + logkws = cast( + "LogFormatterResult", + {"level": logging.INFO, "msg": "90% done", "args": args}, + ) + assert self._log(caplog, logkws) == "90% done" + + @pytest.mark.parametrize( + ("msg", "args"), + [("%(pct)d%% done", {"pct": 90}), ("%d%% done", (90,))], + ) + def test_args( + self, + caplog: pytest.LogCaptureFixture, + msg: str, + args: dict[str, Any] | tuple[Any, ...], + ) -> None: + logkws: LogFormatterResult = {"level": logging.INFO, "msg": msg, "args": args} + assert self._log(caplog, logkws) == "90% done" From fc082aa914cfb2a610d672fb39d5beaba47b0a7d Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 20:01:31 +0200 Subject: [PATCH 36/42] Clarify the docs about reactor settings (#7880) --- docs/topics/settings.rst | 46 ++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index de665f42e..de74ad5cf 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -69,9 +69,10 @@ Example:: precedence and override the project ones. .. note:: :ref:`Pre-crawler settings ` cannot be defined - per spider, and :ref:`reactor settings ` should not have - a different value per spider when :ref:`running multiple spiders in the - same process `. + per spider, and :ref:`reactor settings ` and + :ref:`logging settings ` are subject to restrictions when + :ref:`running multiple spiders in the same process + `. One way to do so is by setting their :attr:`~scrapy.Spider.custom_settings` attribute: @@ -329,32 +330,41 @@ Reactor settings **Reactor settings** are settings tied to the :doc:`Twisted reactor `. -These settings can be defined from a spider. However, because only 1 reactor -can be used per process, these settings cannot use a different value per spider -when :ref:`running multiple spiders in the same process -`. +Because only 1 reactor can be used per process, these settings cannot use a +different value per spider when :ref:`running multiple spiders in the same +process `. -In general, if different spiders define different values, the first defined -value is used. However, if two spiders request a different reactor, an -exception is raised. - -These settings are: +These settings are used upon installing the reactor: - :setting:`ASYNCIO_EVENT_LOOP` (not possible to set per-spider when using :class:`~scrapy.crawler.AsyncCrawlerProcess`, see below) +- :setting:`TWISTED_REACTOR` (ignored when using + :class:`~scrapy.crawler.AsyncCrawlerProcess`, see below) + +They can be :ref:`set from a spider `, but only the values +from the first spider that runs are used, since that is when the reactor is +installed. If a later spider asks for a different reactor or a different event +loop, an exception is raised. With +:class:`~scrapy.crawler.CrawlerRunner` and +:class:`~scrapy.crawler.AsyncCrawlerRunner` the reactor must be installed +beforehand, so these settings are only used to check that the installed reactor +and event loop match them. + +These settings are applied when starting the reactor: + - :setting:`TWISTED_DNS_RESOLVER` and settings used by the corresponding component, e.g. :setting:`DNSCACHE_ENABLED`, :setting:`DNSCACHE_SIZE` and :setting:`DNS_TIMEOUT` for the default one. - :setting:`REACTOR_THREADPOOL_MAXSIZE` -- :setting:`TWISTED_REACTOR` (ignored when using - :class:`~scrapy.crawler.AsyncCrawlerProcess`, see below) - -:setting:`ASYNCIO_EVENT_LOOP` and :setting:`TWISTED_REACTOR` are used upon -installing the reactor. The rest of the settings are applied when starting -the reactor. +They are read from the settings of the +:class:`~scrapy.crawler.CrawlerProcess` or +:class:`~scrapy.crawler.AsyncCrawlerProcess` object, so setting them from a +spider or an :ref:`add-on ` has no effect. They are ignored +altogether when using :class:`~scrapy.crawler.CrawlerRunner` or +:class:`~scrapy.crawler.AsyncCrawlerRunner`, which do not start the reactor. There is an additional restriction for :setting:`TWISTED_REACTOR` and :setting:`ASYNCIO_EVENT_LOOP` when using From ba29ce8d3bca5d525d39cd5f05c97e8416f43857 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 20:02:44 +0200 Subject: [PATCH 37/42] Add a docs section on using download_async() from a downloader middleware (#7872) --- docs/topics/downloader-middleware.rst | 55 +++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 2d6ba0cf3..965108214 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -156,6 +156,61 @@ defines one or more of these methods: :param exception: the raised exception :type exception: an ``Exception`` object +.. _mw-download: + +Downloading a request from a downloader middleware +================================================== + +A downloader middleware can download a request of its own while it processes +another one, e.g. to fetch something that the request it is processing needs. +The built-in :ref:`robots.txt middleware ` does that: it +holds each request while it downloads the ``robots.txt`` file of its website. + +Use :meth:`crawler.engine.download_async() +` for that: + +.. code-block:: python + + from scrapy import Request + from scrapy.http.request import NO_CALLBACK + + + class TokenMiddleware: + def __init__(self, crawler): + self.crawler = crawler + self.token = None + + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + + async def process_request(self, request): + if request.meta.get("dont_obey_robotstxt"): + return + if self.token is None: + response = await self.crawler.engine.download_async( + Request( + "https://example.com/token", + callback=NO_CALLBACK, + meta={"dont_obey_robotstxt": True}, + ) + ) + self.token = response.text + request.headers["Authorization"] = self.token + +Requests that you download this way go through the downloader middleware chain +as well, including your own middleware and the :ref:`robots.txt middleware +`, which holds a request until the ``robots.txt`` file of +its website arrives. Be careful not to introduce deadlocks: a request that you +download must not end up waiting for the request that is waiting for it. Hence +:reqmeta:`dont_obey_robotstxt` above, which makes both middlewares let the token +request through. + +While the first token response is in transit, ``process_request`` runs for other +requests as well, and the middleware above downloads a token for each of them. +Cache the task that downloads the token, and not only its result, to download +the token only once. + .. _topics-downloader-middleware-ref: Built-in downloader middleware reference From d41bfaec05d90fbca2f2c7f1652741fcf7a06a05 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 20:05:43 +0200 Subject: [PATCH 38/42] Document how to disallow subdomains of allowed domains (#7903) --- docs/topics/downloader-middleware.rst | 35 +--------------- scrapy/downloadermiddlewares/offsite.py | 48 +++++++++++++++++++++- tests/test_downloadermiddleware_offsite.py | 16 ++++++++ 3 files changed, 65 insertions(+), 34 deletions(-) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 965108214..dec9904d2 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -799,40 +799,9 @@ OffsiteMiddleware .. module:: scrapy.downloadermiddlewares.offsite :synopsis: Offsite Middleware -.. class:: OffsiteMiddleware +.. autoclass:: OffsiteMiddleware - .. versionadded:: 2.11.2 - - Filters out Requests for URLs outside the domains covered by the spider. - - This middleware filters out every request whose host names aren't in the - spider's :attr:`~scrapy.Spider.allowed_domains` attribute. - All subdomains of any domain in the list are also allowed. - E.g. the rule ``www.example.org`` will also allow ``bob.www.example.org`` - but not ``www2.example.com`` nor ``example.com``. - - When your spider returns a request for a domain not belonging to those - covered by the spider, this middleware will log a debug message similar to - this one:: - - DEBUG: Filtered offsite request to 'offsite.example': - - To avoid filling the log with too much noise, it will only print one of - these messages for each new domain filtered. So, for example, if another - request for ``offsite.example`` is filtered, no log message will be - printed. But if a request for ``other.example`` is filtered, a message - will be printed (but only for the first request filtered). - - If the spider doesn't define an - :attr:`~scrapy.Spider.allowed_domains` attribute, or the - attribute is empty, the offsite middleware will allow all requests. - - .. reqmeta:: allow_offsite - - If the request has the :attr:`~scrapy.Request.dont_filter` attribute set to - ``True`` or :attr:`Request.meta ` has ``allow_offsite`` - set to ``True``, then the OffsiteMiddleware will allow the request even if - its domain is not listed in allowed domains. + .. automethod:: should_follow RedirectMiddleware ------------------ diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py index 28c0e09cb..0d5c9e4c9 100644 --- a/scrapy/downloadermiddlewares/offsite.py +++ b/scrapy/downloadermiddlewares/offsite.py @@ -21,6 +21,36 @@ logger = logging.getLogger(__name__) class OffsiteMiddleware: + """Filter out requests for URLs outside the domains covered by the spider. + + .. versionadded:: 2.11.2 + + A request is allowed if its host name is in the + :attr:`~scrapy.Spider.allowed_domains` attribute of the spider, or is a + subdomain of one of those domains. E.g. ``www.example.org`` also allows + ``bob.www.example.org``, but neither ``www2.example.org`` nor + ``example.org``. See :meth:`should_follow` to use a different policy. + + If the spider does not define :attr:`~scrapy.Spider.allowed_domains`, or + the attribute is empty, every request is allowed. + + Filtered requests are logged as follows:: + + DEBUG: Filtered offsite request to 'offsite.example': + + Only the first request filtered for a given domain is logged, to keep the + log readable. + + .. reqmeta:: allow_offsite + + allow_offsite + ------------- + + Requests with the ``allow_offsite`` :attr:`~scrapy.Request.meta` key set to + ``True``, or with :attr:`~scrapy.Request.dont_filter` set to ``True``, are + allowed regardless of their host name. + """ + crawler: Crawler host_regex: re.Pattern[str] @@ -72,6 +102,23 @@ class OffsiteMiddleware: raise IgnoreRequest(f"Filtered offsite request to {domain!r}") def should_follow(self, request: Request, spider: Spider) -> bool: + """Return ``True`` if *request* is on site, ``False`` if it must be + filtered out. + + Override this method to implement a different offsite policy. For + example, to allow the domains in + :attr:`~scrapy.Spider.allowed_domains` but none of their subdomains: + + .. code-block:: python + + from scrapy.downloadermiddlewares.offsite import OffsiteMiddleware + from scrapy.utils.httpobj import urlparse_cached + + + class RootOnlyOffsiteMiddleware(OffsiteMiddleware): + def should_follow(self, request, spider): + return urlparse_cached(request).hostname in spider.allowed_domains + """ self._update_host_regex(spider) regex = self.host_regex # hostname can be None for wrong urls (like javascript links) @@ -79,7 +126,6 @@ class OffsiteMiddleware: return bool(regex.search(host)) def get_host_regex(self, spider: Spider) -> re.Pattern[str]: - """Override this method to implement a different offsite policy""" allowed_domains = getattr(spider, "allowed_domains", None) if not allowed_domains: return re.compile("") # allow all by default diff --git a/tests/test_downloadermiddleware_offsite.py b/tests/test_downloadermiddleware_offsite.py index bab89814f..1f91dcd4b 100644 --- a/tests/test_downloadermiddleware_offsite.py +++ b/tests/test_downloadermiddleware_offsite.py @@ -6,6 +6,7 @@ import pytest from scrapy import Request, Spider from scrapy.downloadermiddlewares.offsite import OffsiteMiddleware from scrapy.exceptions import IgnoreRequest +from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.test import get_crawler UNSET = object() @@ -237,6 +238,21 @@ def test_repeated_offsite_domain(): assert crawler.stats.get_value("offsite/filtered") == 2 +def test_should_follow_override(): + class RootOnlyOffsiteMiddleware(OffsiteMiddleware): + def should_follow(self, request: Request, spider: Spider) -> bool: + allowed_domains: list[str] = getattr(spider, "allowed_domains", []) + return urlparse_cached(request).hostname in allowed_domains + + crawler = get_crawler(Spider) + crawler.spider = crawler._create_spider(name="a", allowed_domains=["example.com"]) + mw = RootOnlyOffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(crawler.spider) + assert mw.process_request(Request("https://example.com/1")) is None + with pytest.raises(IgnoreRequest): + mw.process_request(Request("https://www.example.com/1")) + + def test_ignore_request_reason(): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=["example.com"]) From 5270f3cf99d10877f9b4c044e8a21de883c94ea5 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 20:07:10 +0200 Subject: [PATCH 39/42] Add a depth_reset request metadata key (#7913) --- docs/topics/spider-middleware.rst | 20 +------------------- scrapy/spidermiddlewares/depth.py | 26 +++++++++++++++++++++++++- tests/test_spidermiddleware_depth.py | 13 +++++++++++++ 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index db85906c1..1c9ee0c77 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -228,25 +228,7 @@ DepthMiddleware .. module:: scrapy.spidermiddlewares.depth :synopsis: Depth Spider Middleware -.. class:: DepthMiddleware - - DepthMiddleware is used for tracking the depth of each Request inside the - site being scraped. It works by setting ``request.meta['depth'] = 0`` whenever - there is no value previously set (usually just the first Request) and - incrementing it by 1 otherwise. - - It can be used to limit the maximum depth to scrape, control Request - priority based on their depth, and things like that. - - The :class:`DepthMiddleware` can be configured through the following - settings (see the settings documentation for more info): - - * :setting:`DEPTH_LIMIT` - The maximum depth that will be allowed to - crawl for any site. If zero, no limit will be imposed. - * :setting:`DEPTH_STATS_VERBOSE` - Whether to collect the number of - requests for each depth. - * :setting:`DEPTH_PRIORITY` - Whether to prioritize the requests based on - their depth. +.. autoclass:: DepthMiddleware HttpErrorMiddleware ------------------- diff --git a/scrapy/spidermiddlewares/depth.py b/scrapy/spidermiddlewares/depth.py index 0131b62e7..1781543e3 100644 --- a/scrapy/spidermiddlewares/depth.py +++ b/scrapy/spidermiddlewares/depth.py @@ -28,6 +28,27 @@ logger = logging.getLogger(__name__) class DepthMiddleware(BaseSpiderMiddleware): + """Track the depth of each request within the site being scraped, setting + ``request.meta["depth"]`` to 0 when there is no value previously set + (usually just the first request) and incrementing it by 1 otherwise. + + It can be used to limit the maximum depth to scrape, control request + priority based on their depth, and things like that, through the + :setting:`DEPTH_LIMIT`, :setting:`DEPTH_STATS_VERBOSE` and + :setting:`DEPTH_PRIORITY` settings. + + .. reqmeta:: depth_reset + + depth_reset + ----------- + + .. versionadded:: VERSION + + :attr:`~scrapy.Request.meta` key that, set to ``True``, gives a request + depth 0 instead of the depth of its source response plus 1, e.g. to keep + :setting:`DEPTH_LIMIT` from applying across a domain change. + """ + crawler: Crawler def __init__( # pylint: disable=super-init-not-called @@ -87,10 +108,13 @@ class DepthMiddleware(BaseSpiderMiddleware): def get_processed_request( self, request: Request, response: Response | None ) -> Request | None: + # Consumed here so that it cannot reach response.meta and, from there, + # spread to further requests through a meta copy. + depth_reset = request.meta.pop("depth_reset", False) if response is None: # start requests return request - depth = response.meta["depth"] + 1 + depth = 0 if depth_reset else response.meta["depth"] + 1 request.meta["depth"] = depth if self.prio: request.priority -= depth * self.prio diff --git a/tests/test_spidermiddleware_depth.py b/tests/test_spidermiddleware_depth.py index 2aa76195e..bdacd7287 100644 --- a/tests/test_spidermiddleware_depth.py +++ b/tests/test_spidermiddleware_depth.py @@ -60,6 +60,19 @@ def test_process_spider_output(mw: DepthMiddleware, stats: StatsCollector) -> No assert rdm == 1 +def test_depth_reset(mw: DepthMiddleware, stats: StatsCollector) -> None: + resp = Response("https://example.com") + resp.request = Request("https://example.com", meta={"depth": 5}) + result = [Request("https://example.com", meta={"depth_reset": True})] + + out = list(mw.process_spider_output(resp, result)) + + assert out == result + assert out[0].meta["depth"] == 0 + assert "depth_reset" not in out[0].meta + assert stats.get_value("request_depth_count/0") == 1 + + def test_process_spider_output_no_response( mw: DepthMiddleware, stats: StatsCollector ) -> None: From ae68786210b99240b269125673704754a282ff5e Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 20:08:59 +0200 Subject: [PATCH 40/42] Document how to customize media pipeline file names from responses (#7909) --- docs/topics/media-pipeline.rst | 35 +++++++++++++++++-- tests/test_pipeline_files.py | 64 +++++++++++++++++++++++++++++++--- 2 files changed, 93 insertions(+), 6 deletions(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index b16066d0c..576feae7e 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -178,6 +178,37 @@ By overriding ``file_path`` like this: For more information about the ``file_path`` method, see :ref:`topics-media-pipeline-override`. +.. _file-naming-response: + +Naming files after the response +------------------------------- + +``file_path`` also receives the ``response``, which allows naming files after +response data. For example, to determine the file extension from the +``Content-Type`` header, for URLs that do not end in a file name: + +.. code-block:: python + + import mimetypes + + from scrapy.pipelines.files import FilesPipeline + + + class ContentTypeFilesPipeline(FilesPipeline): + def file_path(self, request, response=None, info=None, *, item=None): + path = super().file_path(request, response, info, item=item) + if response is None: + return path + content_type = response.headers["Content-Type"].decode() + return path + (mimetypes.guess_extension(content_type) or "") + +This requires setting :setting:`FILES_EXPIRES` to ``0``. To find out whether a +file has already been downloaded, Scrapy calls ``file_path`` before the +download, with ``response`` set to ``None``, and checks the age of the file at +the resulting path. A path that depends on the response can never match that +check, and :setting:`FILES_EXPIRES` set to ``0`` disables it, at the cost of +downloading every file on every run. + .. _topics-supported-storage: Supported Storage @@ -543,7 +574,7 @@ See here the methods that you can override in your custom Files Pipeline: return "files/" + PurePosixPath(urlparse_cached(request).path).name Similarly, you can use the ``item`` to determine the file path based on some item - property. + property, or the ``response``, see :ref:`file-naming-response`. By default the :meth:`file_path` method returns ``full/.``. @@ -693,7 +724,7 @@ See here the methods that you can override in your custom Images Pipeline: return "files/" + PurePosixPath(urlparse_cached(request).path).name Similarly, you can use the ``item`` to determine the file path based on some item - property. + property, or the ``response``, see :ref:`file-naming-response`. By default the :meth:`file_path` method returns ``full/.``. diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 3a8a3d0f7..f40619933 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -1,6 +1,7 @@ import base64 import dataclasses import logging +import mimetypes import random import re import time @@ -96,8 +97,14 @@ class TestFilesPipeline: def teardown_method(self): rmtree(self.tempdir) - def _create_pipeline(self, pipeline_cls: type[FilesPipeline]) -> FilesPipeline: - crawler = get_crawler(DefaultSpider, {"FILES_STORE": self.tempdir}) + def _create_pipeline( + self, + pipeline_cls: type[FilesPipeline], + settings: dict[str, Any] | None = None, + ) -> FilesPipeline: + crawler = get_crawler( + DefaultSpider, {"FILES_STORE": self.tempdir, **(settings or {})} + ) crawler.spider = crawler._create_spider() crawler.engine = MagicMock(download_async=mocked_download_func) pipeline = pipeline_cls.from_crawler(crawler) @@ -394,6 +401,47 @@ class TestFilesPipeline: request = Request("http://example.com") assert file_path(request, item=item) == "full/path-to-store-file" + @coroutine_test + async def test_file_path_from_response(self) -> None: + """file_path() may build the path out of the response, e.g. to get the + file extension from a response header, as long as FILES_EXPIRES is 0 to + disable the up-to-date check, which runs before the download and hence + cannot reach the same path.""" + + class ContentTypeFilesPipeline(FilesPipeline): + def file_path(self, request, response=None, info=None, *, item=None): + path = super().file_path(request, response, info, item=item) + if response is None: + return path + content_type = response.headers["Content-Type"].decode() + return path + (mimetypes.guess_extension(content_type) or "") + + item_url = "http://example.com/download?id=1" + item = _create_item_with_files(item_url) + pipeline = self._create_pipeline(ContentTypeFilesPipeline, {"FILES_EXPIRES": 0}) + request = _prepare_request_object( + item_url, headers={"Content-Type": "application/pdf"} + ) + with ( + mock.patch.object(FilesPipeline, "inc_stats", return_value=True), + # A fresh file at the response-less path is ignored thanks to + # FILES_EXPIRES being 0. + mock.patch.object( + FSFilesStore, + "stat_file", + return_value={"checksum": "abc", "last_modified": time.time()}, + ), + mock.patch.object( + FilesPipeline, "get_media_requests", return_value=[request] + ), + ): + result = await pipeline.process_item(item) + + file_info = result["files"][0] + assert file_info["status"] == "downloaded" + assert file_info["path"].endswith(".pdf") + assert (Path(self.tempdir) / file_info["path"]).read_bytes() == b"data" + def test_media_failed_filtered_request( self, caplog: pytest.LogCaptureFixture ) -> None: @@ -1133,10 +1181,18 @@ def _create_item_with_files(*files: str) -> ItemWithFiles: return item -def _prepare_request_object(item_url: str, flags: list[str] | None = None) -> Request: +def _prepare_request_object( + item_url: str, + flags: list[str] | None = None, + headers: dict[str, str] | None = None, +) -> Request: return Request( item_url, - meta={"response": Response(item_url, status=200, body=b"data", flags=flags)}, + meta={ + "response": Response( + item_url, status=200, body=b"data", flags=flags, headers=headers + ) + }, ) From 09c918115bb43d4f5246be2d2a6e5b719182a5c1 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 10 Aug 2026 08:43:57 +0200 Subject: [PATCH 41/42] Document response parsing memory use in the security page (#7930) --- docs/topics/security.rst | 71 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/docs/topics/security.rst b/docs/topics/security.rst index 2ca270045..5348aae23 100644 --- a/docs/topics/security.rst +++ b/docs/topics/security.rst @@ -36,6 +36,77 @@ their input in an unsafe way, such as :func:`eval`, :func:`exec`, or :func:`pickle.loads`, and be careful when writing response data to paths derived from the response itself. +.. _security-response-size: + +Memory use when parsing responses +================================= + +Parsing a response with :ref:`selectors ` builds an in-memory +tree of the whole response body, which takes several times as much memory as +the body itself. Scrapy parses without the size limits that libxml2 applies by +default, so the size of that tree is bound only by the size of the response, as +controlled by :setting:`DOWNLOAD_MAXSIZE` (default: 1 GiB). + +XML entities are left unresolved, so the tree stays proportional to the +response body even for input crafted as an `XML bomb +`_. A server can still +make a crawler allocate a lot of memory by returning a very large response, +though, so if you know the size of the responses you care about, lower the +limit: + +.. code-block:: python + + DOWNLOAD_MAXSIZE = 32 * 1024 * 1024 # 32 MiB + +* **Pro:** a server cannot make the crawler allocate more memory than the limit + allows, whether by returning a large response or by crafting one that is + expensive to parse. + +* **Con:** you can no longer scrape sites that legitimately serve responses + above the limit, as those responses are dropped. + +.. _security-parser-limits: + +Parser limits +------------- + +The limits that libxml2 applies by default, such as 256 nesting levels and +10 MB per text node, can be restored by overriding +:attr:`~scrapy.http.TextResponse.selector` in a response subclass and swapping +responses in a :ref:`downloader middleware `: + +.. code-block:: python + + from functools import cached_property + + from scrapy import Selector + from scrapy.http import HtmlResponse + + + class LimitedHtmlResponse(HtmlResponse): + @cached_property + def selector(self): + return Selector(self, huge_tree=False) + + + class LimitedParsingMiddleware: + def process_response(self, request, response, spider): + if isinstance(response, HtmlResponse): + return response.replace(cls=LimitedHtmlResponse) + return response + +Do the same with :class:`~scrapy.http.XmlResponse` if you also parse XML. + +These limits apply per node, so :setting:`DOWNLOAD_MAXSIZE` remains your bound +on total memory: a response made of many small elements is parsed in full and +uses as much memory either way. + +* **Pro:** deeply nested responses, and responses with very large individual + nodes, become cheaper to parse. + +* **Con:** parsing stops at those limits without raising, so a legitimate page + that exceeds them yields incomplete data and no error. + TLS connections =============== From 508bd7faec44975ac774aeff8bb5f4a3f57a0be5 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 10 Aug 2026 09:07:34 +0200 Subject: [PATCH 42/42] Type late Crawler attributes as always set instead of None (#7882) --- docs/topics/api.rst | 7 ++ scrapy/commands/parse.py | 1 - scrapy/core/engine.py | 24 +++---- scrapy/core/scraper.py | 11 +--- scrapy/crawler.py | 78 ++++++++++++++++++----- scrapy/downloadermiddlewares/httpcache.py | 1 - scrapy/downloadermiddlewares/offsite.py | 1 - scrapy/downloadermiddlewares/retry.py | 1 - scrapy/downloadermiddlewares/robotstxt.py | 19 ++---- scrapy/downloadermiddlewares/stats.py | 1 - scrapy/dupefilters.py | 2 - scrapy/extensions/closespider.py | 2 - scrapy/extensions/corestats.py | 1 - scrapy/extensions/debug.py | 1 - scrapy/extensions/feedexport.py | 1 - scrapy/extensions/httpcache.py | 2 - scrapy/extensions/logstats.py | 1 - scrapy/extensions/memdebug.py | 1 - scrapy/extensions/memusage.py | 26 +++----- scrapy/extensions/periodic_log.py | 1 - scrapy/extensions/statsmailer.py | 1 - scrapy/extensions/telnet.py | 1 - scrapy/extensions/throttle.py | 2 - scrapy/pipelines/files.py | 6 +- scrapy/pipelines/media.py | 2 - scrapy/pqueues.py | 1 - scrapy/shell.py | 2 - scrapy/spidermiddlewares/depth.py | 1 - scrapy/spidermiddlewares/httperror.py | 6 +- scrapy/spidermiddlewares/urllength.py | 1 - scrapy/utils/log.py | 1 - tests/test_crawler.py | 25 ++++++++ 32 files changed, 131 insertions(+), 100 deletions(-) diff --git a/docs/topics/api.rst b/docs/topics/api.rst index 7ff8d3464..598edfeb5 100644 --- a/docs/topics/api.rst +++ b/docs/topics/api.rst @@ -35,6 +35,13 @@ how you :ref:`configure the downloader middlewares :class:`scrapy.Spider` subclass and a :class:`scrapy.settings.Settings` object. + The :attr:`engine`, :attr:`extensions`, :attr:`logformatter`, + :attr:`request_fingerprinter` and :attr:`stats` attributes get their value + when the crawl starts, and raise :exc:`RuntimeError` when read before that. + + .. versionchanged:: VERSION + Those attributes used to be ``None`` before getting their value. + .. attribute:: request_fingerprinter The request fingerprint builder of this crawler. diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index 51caed57f..b6255ec7e 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -281,7 +281,6 @@ class Command(BaseRunSpiderCommand): ) -> list[Any]: items, requests, opts, depth, spider, callback = args if opts.pipelines: - assert self.pcrawler.engine itemproc = self.pcrawler.engine.scraper.itemproc if hasattr(itemproc, "process_item_async"): for item in items: diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index b7a1e0fcd..b63d136a4 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -112,7 +112,6 @@ class ExecutionEngine: self.crawler: Crawler = crawler self.settings: Settings = crawler.settings self.signals: SignalManager = crawler.signals - assert crawler.logformatter self.logformatter: LogFormatter = crawler.logformatter self._slot: _Slot | None = None self.spider: Spider | None = None @@ -299,7 +298,6 @@ class ExecutionEngine: response=None, spider=self.spider, ) - assert self.crawler.stats self.crawler.stats.inc_value("spider_exceptions/count") self.crawler.stats.inc_value( f"spider_exceptions/{type(exception).__name__}" @@ -563,17 +561,17 @@ class ExecutionEngine: if hasattr(scheduler, "open") and (d := scheduler.open(self.crawler.spider)): await maybe_deferred_to_future(d) await self.scraper.open_spider_async() - assert self.crawler.stats - if argument_is_required(self.crawler.stats.open_spider, "spider"): + stats = self.crawler.stats + if argument_is_required(stats.open_spider, "spider"): warnings.warn( - f"The open_spider() method of {global_object_name(type(self.crawler.stats))} requires a spider argument," + f"The open_spider() method of {global_object_name(type(stats))} requires a spider argument," f" this is deprecated and the argument will not be passed in future Scrapy versions.", ScrapyDeprecationWarning, stacklevel=2, ) - self.crawler.stats.open_spider(spider=self.crawler.spider) + stats.open_spider(spider=self.crawler.spider) else: - self.crawler.stats.open_spider() + stats.open_spider() await self.signals.send_catch_log_async( signals.spider_opened, spider=self.crawler.spider ) @@ -676,20 +674,18 @@ class ExecutionEngine: extra={"spider": spider}, ) - assert self.crawler.stats try: - if argument_is_required(self.crawler.stats.close_spider, "spider"): + stats = self.crawler.stats + if argument_is_required(stats.close_spider, "spider"): warnings.warn( - f"The close_spider() method of {global_object_name(type(self.crawler.stats))} requires a spider argument," + f"The close_spider() method of {global_object_name(type(stats))} requires a spider argument," f" this is deprecated and the argument will not be passed in future Scrapy versions.", ScrapyDeprecationWarning, stacklevel=2, ) - self.crawler.stats.close_spider( - spider=self.crawler.spider, reason=reason - ) + stats.close_spider(spider=self.crawler.spider, reason=reason) else: - self.crawler.stats.close_spider(reason=reason) + stats.close_spider(reason=reason) except Exception: logger.error("Stats close failure") diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 58e37ce5e..6426b1751 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -120,7 +120,6 @@ class Scraper: self.concurrent_items: int = crawler.settings.getint("CONCURRENT_ITEMS") self.crawler: Crawler = crawler self.signals: SignalManager = crawler.signals - assert crawler.logformatter self.logformatter: LogFormatter = crawler.logformatter def _check_deprecated_itemproc_method(self, method: str) -> None: @@ -355,7 +354,6 @@ class Scraper: assert self.crawler.spider exc = _failure.value if isinstance(exc, CloseSpider): - assert self.crawler.engine is not None # typing _schedule_coro( self.crawler.engine.close_spider_async(reason=exc.reason or "cancelled") ) @@ -374,11 +372,9 @@ class Scraper: response=response, spider=self.crawler.spider, ) - assert self.crawler.stats - self.crawler.stats.inc_value("spider_exceptions/count") - self.crawler.stats.inc_value( - f"spider_exceptions/{_failure.value.__class__.__name__}" - ) + stats = self.crawler.stats + stats.inc_value("spider_exceptions/count") + stats.inc_value(f"spider_exceptions/{_failure.value.__class__.__name__}") def handle_spider_output( self, @@ -456,7 +452,6 @@ class Scraper: Items are sent to the item pipelines, requests are scheduled. """ if isinstance(output, Request): - assert self.crawler.engine is not None # typing self.crawler.engine.crawl(request=output) return if output is not None: diff --git a/scrapy/crawler.py b/scrapy/crawler.py index e2f726519..44c4ffdcf 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -8,7 +8,7 @@ import signal import warnings from abc import ABC, abstractmethod from functools import partial -from typing import TYPE_CHECKING, Any, TypeVar +from typing import TYPE_CHECKING, Any, Generic, TypeVar, overload from twisted.internet.defer import Deferred, DeferredList, inlineCallbacks @@ -58,7 +58,55 @@ logger = logging.getLogger(__name__) _T = TypeVar("_T") +class _LateAttribute(Generic[_T]): + """Descriptor for a :class:`Crawler` attribute that only gets a value once + the crawl starts. + + The value is kept in an attribute of the same name prefixed with an + underscore, and reading it before it is set raises :exc:`RuntimeError`. + This way the public attribute can be annotated as always set, and its + users, both in Scrapy and in third-party code, do not need to narrow its + type on every use. Code that runs before the crawl starts reads the + underscore-prefixed attribute instead. + """ + + def __set_name__(self, owner: type[Crawler], name: str) -> None: + self._name = name + self._private_name = f"_{name}" + + @overload + def __get__(self, instance: None, owner: type[Crawler]) -> _LateAttribute[_T]: ... + + @overload + def __get__(self, instance: Crawler, owner: type[Crawler]) -> _T: ... + + def __get__( + self, instance: Crawler | None, owner: type[Crawler] + ) -> _LateAttribute[_T] | _T: + if instance is None: + return self + value: _T | None = getattr(instance, self._private_name) + if value is None: + raise RuntimeError( + f"Crawler.{self._name} is not set yet. It is set when the " + "crawl starts, so it can only be used from then on, e.g. " + "from the spider_opened signal handler onwards." + ) + return value + + def __set__(self, instance: Crawler, value: _T) -> None: + setattr(instance, self._private_name, value) + + class Crawler: + engine: _LateAttribute[ExecutionEngine] = _LateAttribute() + extensions: _LateAttribute[ExtensionManager] = _LateAttribute() + logformatter: _LateAttribute[LogFormatter] = _LateAttribute() + request_fingerprinter: _LateAttribute[RequestFingerprinterProtocol] = ( + _LateAttribute() + ) + stats: _LateAttribute[StatsCollector] = _LateAttribute() + def __init__( self, spidercls: type[Spider], @@ -83,12 +131,13 @@ class Crawler: self.crawling: bool = False self._started: bool = False - self.extensions: ExtensionManager | None = None - self.stats: StatsCollector | None = None - self.logformatter: LogFormatter | None = None - self.request_fingerprinter: RequestFingerprinterProtocol | None = None self.spider: Spider | None = None - self.engine: ExecutionEngine | None = None + + self._engine: ExecutionEngine | None = None + self._extensions: ExtensionManager | None = None + self._logformatter: LogFormatter | None = None + self._request_fingerprinter: RequestFingerprinterProtocol | None = None + self._stats: StatsCollector | None = None def _update_root_log_handler(self) -> None: if get_scrapy_root_handler() is not None: @@ -225,8 +274,8 @@ class Crawler: yield deferred_from_coro(self.engine.start_async()) except Exception: self.crawling = False - if self.engine is not None: - yield deferred_from_coro(self.engine.close_async()) + if self._engine is not None: + yield deferred_from_coro(self._engine.close_async()) raise async def crawl_async(self, *args: Any, **kwargs: Any) -> None: @@ -255,8 +304,8 @@ class Crawler: await self.engine.start_async() except Exception: self.crawling = False - if self.engine is not None: - await self.engine.close_async() + if self._engine is not None: + await self._engine.close_async() raise def _create_spider(self, *args: Any, **kwargs: Any) -> Spider: @@ -282,7 +331,6 @@ class Crawler: """ if self.crawling: self.crawling = False - assert self.engine if self.engine.running: await self.engine.stop_async() @@ -313,7 +361,7 @@ class Crawler: This method can only be called after the crawl engine has been created, e.g. at signals :signal:`engine_started` or :signal:`spider_opened`. """ - if not self.engine: + if self._engine is None: raise RuntimeError( "Crawler.get_downloader_middleware() can only be called after " "the crawl engine has been created." @@ -331,7 +379,7 @@ class Crawler: created, e.g. at signals :signal:`engine_started` or :signal:`spider_opened`. """ - if not self.extensions: + if self._extensions is None: raise RuntimeError( "Crawler.get_extension() can only be called after the " "extension manager has been created." @@ -348,7 +396,7 @@ class Crawler: This method can only be called after the crawl engine has been created, e.g. at signals :signal:`engine_started` or :signal:`spider_opened`. """ - if not self.engine: + if self._engine is None: raise RuntimeError( "Crawler.get_item_pipeline() can only be called after the " "crawl engine has been created." @@ -365,7 +413,7 @@ class Crawler: This method can only be called after the crawl engine has been created, e.g. at signals :signal:`engine_started` or :signal:`spider_opened`. """ - if not self.engine: + if self._engine is None: raise RuntimeError( "Crawler.get_spider_middleware() can only be called after the " "crawl engine has been created." diff --git a/scrapy/downloadermiddlewares/httpcache.py b/scrapy/downloadermiddlewares/httpcache.py index e7ca0ac0e..3ebd98027 100644 --- a/scrapy/downloadermiddlewares/httpcache.py +++ b/scrapy/downloadermiddlewares/httpcache.py @@ -55,7 +55,6 @@ class HttpCacheMiddleware: @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - assert crawler.stats o = cls(crawler.settings, crawler.stats) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py index 0d5c9e4c9..b9a26df3a 100644 --- a/scrapy/downloadermiddlewares/offsite.py +++ b/scrapy/downloadermiddlewares/offsite.py @@ -61,7 +61,6 @@ class OffsiteMiddleware: @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - assert crawler.stats o = cls(crawler.stats) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.request_scheduled, signal=signals.request_scheduled) diff --git a/scrapy/downloadermiddlewares/retry.py b/scrapy/downloadermiddlewares/retry.py index f910d07c8..dd2897ee4 100644 --- a/scrapy/downloadermiddlewares/retry.py +++ b/scrapy/downloadermiddlewares/retry.py @@ -94,7 +94,6 @@ def get_retry_request( retry-related job stats """ settings = spider.crawler.settings - assert spider.crawler.stats stats = spider.crawler.stats retry_times = request.meta.get("retry_times", 0) + 1 if max_retry_times is None: diff --git a/scrapy/downloadermiddlewares/robotstxt.py b/scrapy/downloadermiddlewares/robotstxt.py index 81a3a887f..d7aa8738c 100644 --- a/scrapy/downloadermiddlewares/robotstxt.py +++ b/scrapy/downloadermiddlewares/robotstxt.py @@ -27,6 +27,7 @@ if TYPE_CHECKING: from scrapy import Spider from scrapy.crawler import Crawler from scrapy.robotstxt import RobotParser + from scrapy.statscollectors import StatsCollector logger = logging.getLogger(__name__) @@ -41,6 +42,7 @@ class RobotsTxtMiddleware: self._default_useragent: str = crawler.settings["USER_AGENT"] self._robotstxt_useragent: str | None = crawler.settings["ROBOTSTXT_USER_AGENT"] self.crawler: Crawler = crawler + self._stats: StatsCollector = crawler.stats self._parsers: dict[str, RobotParser | Deferred[RobotParser | None] | None] = {} self._parserimpl: RobotParser = load_object( crawler.settings.get("ROBOTSTXT_PARSER") @@ -78,8 +80,7 @@ class RobotsTxtMiddleware: {"request": request}, extra={"spider": self.crawler.spider}, ) - assert self.crawler.stats - self.crawler.stats.inc_value("robotstxt/forbidden") + self._stats.inc_value("robotstxt/forbidden") raise IgnoreRequest("Forbidden by robots.txt") async def robot_parser(self, request: Request) -> RobotParser | None: @@ -95,8 +96,6 @@ class RobotsTxtMiddleware: meta={"dont_obey_robotstxt": True}, callback=NO_CALLBACK, ) - assert self.crawler.engine - assert self.crawler.stats try: resp = await self.crawler.engine.download_async(robotsreq) await self._parse_robots(resp, netloc, request) @@ -109,7 +108,7 @@ class RobotsTxtMiddleware: extra={"spider": self.crawler.spider}, ) self._robots_error(e, netloc) - self.crawler.stats.inc_value("robotstxt/request_count") + self._stats.inc_value("robotstxt/request_count") parser = self._parsers[netloc] if isinstance(parser, Deferred): @@ -119,11 +118,8 @@ class RobotsTxtMiddleware: async def _parse_robots( self, response: Response, netloc: str, request: Request ) -> None: - assert self.crawler.stats - self.crawler.stats.inc_value("robotstxt/response_count") - self.crawler.stats.inc_value( - f"robotstxt/response_status_count/{response.status}" - ) + self._stats.inc_value("robotstxt/response_count") + self._stats.inc_value(f"robotstxt/response_status_count/{response.status}") rp = self._parserimpl.from_crawler(self.crawler, response.body) await self.crawler.signals.send_catch_log_async( signal=signals.robots_parsed, @@ -138,8 +134,7 @@ class RobotsTxtMiddleware: 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) + self._stats.inc_value(key) rp_dfd = self._parsers[netloc] assert isinstance(rp_dfd, Deferred) self._parsers[netloc] = None diff --git a/scrapy/downloadermiddlewares/stats.py b/scrapy/downloadermiddlewares/stats.py index bafa931de..07de1c2e7 100644 --- a/scrapy/downloadermiddlewares/stats.py +++ b/scrapy/downloadermiddlewares/stats.py @@ -43,7 +43,6 @@ class DownloaderStats: def from_crawler(cls, crawler: Crawler) -> Self: if not crawler.settings.getbool("DOWNLOADER_STATS"): raise NotConfigured - assert crawler.stats return cls(crawler.stats) @_warn_spider_arg diff --git a/scrapy/dupefilters.py b/scrapy/dupefilters.py index 36fb0f97d..09f0be63b 100644 --- a/scrapy/dupefilters.py +++ b/scrapy/dupefilters.py @@ -95,7 +95,6 @@ class RFPDupeFilter(BaseDupeFilter): @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - assert crawler.request_fingerprinter debug = crawler.settings.getbool("DUPEFILTER_DEBUG") return cls( job_dir(crawler.settings), @@ -134,5 +133,4 @@ class RFPDupeFilter(BaseDupeFilter): self.logger.debug(msg, {"request": request}, extra={"spider": spider}) self.logdupes = False - assert spider.crawler.stats spider.crawler.stats.inc_value("dupefilter/filtered") diff --git a/scrapy/extensions/closespider.py b/scrapy/extensions/closespider.py index 9cb792e30..5d40e5f9f 100644 --- a/scrapy/extensions/closespider.py +++ b/scrapy/extensions/closespider.py @@ -102,7 +102,6 @@ class CloseSpider: self._close_spider("closespider_pagecount_no_item") def spider_opened(self, spider: Spider) -> None: - assert self.crawler.engine self.task = call_later( self.close_on["timeout"], self._close_spider, "closespider_timeout" ) @@ -146,5 +145,4 @@ class CloseSpider: self._close_spider("closespider_timeout_no_item") def _close_spider(self, reason: str) -> None: - assert self.crawler.engine _schedule_coro(self.crawler.engine.close_spider_async(reason=reason)) diff --git a/scrapy/extensions/corestats.py b/scrapy/extensions/corestats.py index 6a5e55992..b464942af 100644 --- a/scrapy/extensions/corestats.py +++ b/scrapy/extensions/corestats.py @@ -26,7 +26,6 @@ class CoreStats: @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - assert crawler.stats o = cls(crawler.stats) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) diff --git a/scrapy/extensions/debug.py b/scrapy/extensions/debug.py index 5def7509e..8802a3b58 100644 --- a/scrapy/extensions/debug.py +++ b/scrapy/extensions/debug.py @@ -45,7 +45,6 @@ class StackTraceDump: return cls(crawler) def dump_stacktrace(self, signum: int, frame: FrameType | None) -> None: - assert self.crawler.engine log_args = { "stackdumps": self._thread_stacks(), "enginestatus": format_engine_status(self.crawler.engine), diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index e4294ac90..32abc82d3 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -611,7 +611,6 @@ class FeedExporter: logmsg = f"{slot.format} feed ({slot.itemcount} items) in: {slot.uri}" slot_type = type(slot.storage).__name__ - assert self.crawler.stats try: await ensure_awaitable(slot.storage.store(self._get_file(slot))) except Exception: diff --git a/scrapy/extensions/httpcache.py b/scrapy/extensions/httpcache.py index dbb79b02d..d008d0f67 100644 --- a/scrapy/extensions/httpcache.py +++ b/scrapy/extensions/httpcache.py @@ -261,7 +261,6 @@ class DbmCacheStorage: extra={"spider": spider}, ) - assert spider.crawler.request_fingerprinter self._fingerprinter: RequestFingerprinterProtocol = ( spider.crawler.request_fingerprinter ) @@ -326,7 +325,6 @@ class FilesystemCacheStorage: extra={"spider": spider}, ) - assert spider.crawler.request_fingerprinter self._fingerprinter = spider.crawler.request_fingerprinter def close_spider(self, spider: Spider) -> None: diff --git a/scrapy/extensions/logstats.py b/scrapy/extensions/logstats.py index 6c94d947e..b818569ba 100644 --- a/scrapy/extensions/logstats.py +++ b/scrapy/extensions/logstats.py @@ -37,7 +37,6 @@ class LogStats: interval: float = crawler.settings.getfloat("LOGSTATS_INTERVAL") if not interval: raise NotConfigured - assert crawler.stats o = cls(crawler.stats, interval) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) diff --git a/scrapy/extensions/memdebug.py b/scrapy/extensions/memdebug.py index 1fde6b296..35ff90e3b 100644 --- a/scrapy/extensions/memdebug.py +++ b/scrapy/extensions/memdebug.py @@ -29,7 +29,6 @@ class MemoryDebugger: def from_crawler(cls, crawler: Crawler) -> Self: if not crawler.settings.getbool("MEMDEBUG_ENABLED"): raise NotConfigured - assert crawler.stats o = cls(crawler.stats) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) return o diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index e0e289ce8..ec761cfbf 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -27,6 +27,7 @@ if TYPE_CHECKING: from typing_extensions import Self from scrapy.crawler import Crawler + from scrapy.statscollectors import StatsCollector logger = logging.getLogger(__name__) @@ -43,6 +44,7 @@ class MemoryUsage: raise NotConfigured from exc self.crawler: Crawler = crawler + self._stats: StatsCollector = crawler.stats self.warned: bool = False self.notify_mails: list[str] = crawler.settings.getlist("MEMUSAGE_NOTIFY_MAIL") if self.notify_mails: # pragma: no cover @@ -77,8 +79,7 @@ class MemoryUsage: return size def engine_started(self) -> None: - assert self.crawler.stats - self.crawler.stats.set_value("memusage/startup", self.get_virtual_size()) + self._stats.set_value("memusage/startup", self.get_virtual_size()) self.tasks: list[AsyncioLoopingCall | LoopingCall] = [] tsk = create_looping_call(self.update) self.tasks.append(tsk) @@ -98,15 +99,12 @@ class MemoryUsage: tsk.stop() def update(self) -> None: - assert self.crawler.stats - self.crawler.stats.max_value("memusage/max", self.get_virtual_size()) + self._stats.max_value("memusage/max", self.get_virtual_size()) def _check_limit(self) -> None: - assert self.crawler.engine - assert self.crawler.stats peak_mem_usage = self.get_virtual_size() if peak_mem_usage > self.limit: - self.crawler.stats.set_value("memusage/limit_reached", 1) + self._stats.set_value("memusage/limit_reached", 1) mem = self.limit / 1024 / 1024 logger.error( "Memory usage exceeded %(memusage)dMiB. Shutting down Scrapy...", @@ -119,7 +117,7 @@ class MemoryUsage: f"memory usage exceeded {mem}MiB at {socket.gethostname()}" ) self._send_report(self.notify_mails, subj) - self.crawler.stats.set_value("memusage/limit_notified", 1) + self._stats.set_value("memusage/limit_notified", 1) if self.crawler.engine.spider is not None: _schedule_coro( @@ -136,9 +134,8 @@ class MemoryUsage: def _check_warning(self) -> None: if self.warned: # warn only once return - assert self.crawler.stats if self.get_virtual_size() > self.warning: - self.crawler.stats.set_value("memusage/warning_reached", 1) + self._stats.set_value("memusage/warning_reached", 1) self.crawler.signals.send_catch_log(signal=signals.memusage_warning_reached) mem = self.warning / 1024 / 1024 logger.warning( @@ -152,16 +149,13 @@ class MemoryUsage: f"memory usage reached {mem}MiB at {socket.gethostname()}" ) self._send_report(self.notify_mails, subj) - self.crawler.stats.set_value("memusage/warning_notified", 1) + self._stats.set_value("memusage/warning_notified", 1) self.warned = True def _send_report(self, rcpts: list[str], subject: str) -> None: # pragma: no cover """send notification mail with some additional useful info""" - assert self.crawler.engine - assert self.crawler.stats - stats = self.crawler.stats - s = f"Memory usage at engine startup : {stats.get_value('memusage/startup') / 1024 / 1024}M\r\n" - s += f"Maximum memory usage : {stats.get_value('memusage/max') / 1024 / 1024}M\r\n" + s = f"Memory usage at engine startup : {self._stats.get_value('memusage/startup') / 1024 / 1024}M\r\n" + s += f"Maximum memory usage : {self._stats.get_value('memusage/max') / 1024 / 1024}M\r\n" s += f"Current memory usage : {self.get_virtual_size() / 1024 / 1024}M\r\n" s += ( diff --git a/scrapy/extensions/periodic_log.py b/scrapy/extensions/periodic_log.py index cbcc8b70e..adffbcbc4 100644 --- a/scrapy/extensions/periodic_log.py +++ b/scrapy/extensions/periodic_log.py @@ -87,7 +87,6 @@ class PeriodicLog: ) if not (ext_stats or ext_delta or ext_timing_enabled): raise NotConfigured - assert crawler.stats assert ext_stats is not None assert ext_delta is not None o = cls( diff --git a/scrapy/extensions/statsmailer.py b/scrapy/extensions/statsmailer.py index f05595806..7647cf33d 100644 --- a/scrapy/extensions/statsmailer.py +++ b/scrapy/extensions/statsmailer.py @@ -42,7 +42,6 @@ class StatsMailer: if not recipients: raise NotConfigured mail: MailSender = MailSender.from_crawler(crawler) - assert crawler.stats o = cls(crawler.stats, recipients, mail) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) return o diff --git a/scrapy/extensions/telnet.py b/scrapy/extensions/telnet.py index 1506cb1ea..392f79299 100644 --- a/scrapy/extensions/telnet.py +++ b/scrapy/extensions/telnet.py @@ -108,7 +108,6 @@ class TelnetConsole(protocol.ServerFactory): def _get_telnet_vars(self) -> dict[str, Any]: # Note: if you add entries here also update topics/telnetconsole.rst - assert self.crawler.engine telnet_vars: dict[str, Any] = { "engine": self.crawler.engine, "spider": self.crawler.engine.spider, diff --git a/scrapy/extensions/throttle.py b/scrapy/extensions/throttle.py index cde73f12e..f44aee03d 100644 --- a/scrapy/extensions/throttle.py +++ b/scrapy/extensions/throttle.py @@ -45,7 +45,6 @@ class AutoThrottle: def _spider_opened(self, spider: Spider) -> None: self.mindelay = self._min_delay() self.maxdelay = self._max_delay() - assert self.crawler.engine self.crawler.engine.downloader._delay = self._start_delay() def _min_delay(self) -> float: @@ -98,7 +97,6 @@ class AutoThrottle: key: str | None = request.meta.get("download_slot") if key is None: return None, None - assert self.crawler.engine return key, self.crawler.engine.downloader.slots.get(key) def _adjust_delay(self, slot: Slot, latency: float, response: Response) -> None: diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 55a3676e5..e666f4ddd 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -697,9 +697,9 @@ class FilesPipeline(MediaPipeline): } def inc_stats(self, status: str) -> None: - assert self.crawler.stats - self.crawler.stats.inc_value("file_count") - self.crawler.stats.inc_value(f"file_status_count/{status}") + stats = self.crawler.stats + stats.inc_value("file_count") + stats.inc_value(f"file_status_count/{status}") async def _file_downloaded( self, diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 764f82a78..d4025bf3b 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -100,7 +100,6 @@ class MediaPipeline(ABC): stacklevel=2, ) self.crawler: Crawler = crawler - assert crawler.request_fingerprinter self._fingerprinter: RequestFingerprinterProtocol = ( crawler.request_fingerprinter ) @@ -228,7 +227,6 @@ class MediaPipeline(ABC): ) -> FileInfo: try: self._modify_media_request(request) - assert self.crawler.engine response = await self.crawler.engine.download_async(request) return await ensure_awaitable( self.media_downloaded(response, request, info, item=item) diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py index 8e9783f5a..f4c35c0fe 100644 --- a/scrapy/pqueues.py +++ b/scrapy/pqueues.py @@ -265,7 +265,6 @@ class ScrapyPriorityQueue: class DownloaderInterface: def __init__(self, crawler: Crawler): - assert crawler.engine self.downloader: Downloader = crawler.engine.downloader def stats(self, possible_slots: Iterable[str]) -> list[tuple[int, str]]: diff --git a/scrapy/shell.py b/scrapy/shell.py index dfea00c46..8f78af571 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -193,7 +193,6 @@ class Shell: """ if not self.spider: await self._open_spider(spider) - assert self.crawler.engine is not None # send the request to the engine self.crawler.engine.crawl(request) # this will fire when the request callback runs (via the callback hijacking in _request_deferred()) @@ -204,7 +203,6 @@ class Shell: spider = self.crawler.spider or self.crawler._create_spider() self.crawler.spider = spider - assert self.crawler.engine await self.crawler.engine.open_spider_async(close_if_idle=False) self.spider = spider diff --git a/scrapy/spidermiddlewares/depth.py b/scrapy/spidermiddlewares/depth.py index 1781543e3..49683168e 100644 --- a/scrapy/spidermiddlewares/depth.py +++ b/scrapy/spidermiddlewares/depth.py @@ -70,7 +70,6 @@ class DepthMiddleware(BaseSpiderMiddleware): maxdepth = settings.getint("DEPTH_LIMIT") verbose = settings.getbool("DEPTH_STATS_VERBOSE") prio = settings.getint("DEPTH_PRIORITY") - assert crawler.stats o = cls(maxdepth, crawler.stats, verbose, prio) o.crawler = crawler return o diff --git a/scrapy/spidermiddlewares/httperror.py b/scrapy/spidermiddlewares/httperror.py index 156b73e7e..116a02733 100644 --- a/scrapy/spidermiddlewares/httperror.py +++ b/scrapy/spidermiddlewares/httperror.py @@ -78,9 +78,9 @@ class HttpErrorMiddleware: self, response: Response, exception: Exception, spider: Spider | None = None ) -> Iterable[Any] | None: if isinstance(exception, HttpError): - assert self.crawler.stats - self.crawler.stats.inc_value("httperror/response_ignored_count") - self.crawler.stats.inc_value( + stats = self.crawler.stats + stats.inc_value("httperror/response_ignored_count") + stats.inc_value( f"httperror/response_ignored_status_count/{response.status}" ) logger.info( diff --git a/scrapy/spidermiddlewares/urllength.py b/scrapy/spidermiddlewares/urllength.py index f325ce7a0..86bdd2ed6 100644 --- a/scrapy/spidermiddlewares/urllength.py +++ b/scrapy/spidermiddlewares/urllength.py @@ -48,6 +48,5 @@ class UrlLengthMiddleware(BaseSpiderMiddleware): {"maxlength": self.maxlength, "url": request.url}, extra={"spider": self.crawler.spider}, ) - assert self.crawler.stats self.crawler.stats.inc_value("urllength/request_ignored_count") return None diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index ee357c031..bfa39169f 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -239,7 +239,6 @@ class LogCounterHandler(logging.Handler): def emit(self, record: logging.LogRecord) -> None: sname = f"log_count/{record.levelname}" - assert self.crawler.stats self.crawler.stats.inc_value(sname) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 358f20ed7..17ca02dea 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -74,6 +74,31 @@ class TestCrawler: assert not settings.frozen assert crawler.settings.frozen + @pytest.mark.parametrize( + "attr", + ["extensions", "logformatter", "request_fingerprinter", "stats"], + ) + def test_late_attr_before_apply_settings(self, attr: str) -> None: + crawler = get_raw_crawler(DefaultSpider) + with pytest.raises(RuntimeError, match=rf"Crawler\.{attr} is not set yet"): + getattr(crawler, attr) + crawler._apply_settings() + assert getattr(crawler, attr) is not None + + @pytest.mark.parametrize( + "attr", + ["engine", "extensions", "logformatter", "request_fingerprinter", "stats"], + ) + def test_late_attr_on_class(self, attr: str) -> None: + # Introspection tools such as help() read these off the class. + assert getattr(Crawler, attr) is getattr(Crawler, attr) + + def test_late_attr_engine_before_crawl(self) -> None: + crawler = get_raw_crawler(DefaultSpider) + crawler._apply_settings() + with pytest.raises(RuntimeError, match=r"Crawler\.engine is not set yet"): + _ = crawler.engine + @pytest.mark.parametrize( ("attr", "setting"), [