From 19588b0c4e5b56700c08b21335fa846e441b8009 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 10 Jun 2026 16:11:27 +0200 Subject: [PATCH 1/4] Add a middleware to detect response.meta copies --- docs/topics/request-response.rst | 4 + docs/topics/spider-middleware.rst | 52 ++++++++++ scrapy/settings/default_settings.py | 1 + scrapy/spidermiddlewares/metacopy.py | 65 +++++++++++++ tests/test_spidermiddleware_metacopy.py | 121 ++++++++++++++++++++++++ 5 files changed, 243 insertions(+) create mode 100644 scrapy/spidermiddlewares/metacopy.py create mode 100644 tests/test_spidermiddleware_metacopy.py diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 8fd3de621..660dbd4cf 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -231,6 +231,10 @@ Request objects Also mind that the :meth:`copy` and :meth:`replace` request methods :doc:`shallow-copy ` request metadata. + .. seealso:: :ref:`unsafe-meta-copy` for a more detailed explanation + and :class:`~scrapy.spidermiddlewares.metacopy.MetaCopyDetectionMiddleware` + for a built-in middleware that warns about this issue at run time. + .. autoattribute:: dont_filter .. autoattribute:: Request.attributes diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 99bbdf292..197e2f183 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -316,6 +316,58 @@ Default: ``False`` Pass all responses, regardless of its status code. +MetaCopyDetectionMiddleware +--------------------------- + +.. module:: scrapy.spidermiddlewares.metacopy + :synopsis: Meta Copy Detection Spider Middleware + +.. class:: MetaCopyDetectionMiddleware + + Warns when a spider yields a request that contains internal meta keys which + should not be copied from :attr:`response.meta ` + into new requests, or when two requests share the same meta dict object. + + Each warning is emitted at most once per crawl to avoid noise, but includes + the source response and the target request so the issue is easy to locate. + + .. _unsafe-meta-copy: + + Why copying response.meta is unsafe + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + :attr:`response.meta ` is an alias for + :attr:`response.request.meta `, which is the meta + dict of the request that produced the response. That dict may contain keys + set by Scrapy's internal components — such as ``retry_times`` from + :class:`~scrapy.downloadermiddlewares.retry.RetryMiddleware` — that are + specific to that one request and must not be forwarded to unrelated + requests. Copying the entire dict (e.g. ``meta={**response.meta}``) or + passing the dict object directly (e.g. ``meta=response.meta``) propagates + those internal keys and can cause subtle bugs. For example, forwarding + ``retry_times`` reduces the number of retries available for the new request. + + To pass custom data between callbacks, prefer + :attr:`~scrapy.http.Request.cb_kwargs`. If you need to carry data through + :attr:`~scrapy.http.Request.meta`, copy only the specific keys your code + uses rather than the whole dict. See also `scrapy-sticky-meta-params + `_. + + MetaCopyDetectionMiddleware settings + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + .. setting:: META_COPY_WARN_SKIP_KEYS + + META_COPY_WARN_SKIP_KEYS + ^^^^^^^^^^^^^^^^^^^^^^^^ + + Default: ``[]`` + + A list of internal meta key names to exclude from the internal-keys check. + Use this when you intentionally copy one of the monitored keys and want to + suppress the resulting warning without disabling the middleware entirely. + + RefererMiddleware ----------------- diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 7d5612026..ce0544b1e 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -512,6 +512,7 @@ SPIDER_MIDDLEWARES_BASE = { "scrapy.spidermiddlewares.referer.RefererMiddleware": 700, "scrapy.spidermiddlewares.urllength.UrlLengthMiddleware": 800, "scrapy.spidermiddlewares.depth.DepthMiddleware": 900, + "scrapy.spidermiddlewares.metacopy.MetaCopyDetectionMiddleware": 1000, # Spider side } diff --git a/scrapy/spidermiddlewares/metacopy.py b/scrapy/spidermiddlewares/metacopy.py new file mode 100644 index 000000000..aa5a120c6 --- /dev/null +++ b/scrapy/spidermiddlewares/metacopy.py @@ -0,0 +1,65 @@ +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + +from scrapy.spidermiddlewares.base import BaseSpiderMiddleware + +if TYPE_CHECKING: + from scrapy.crawler import Crawler + from scrapy.http import Request, Response + + +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. + + Each warning is emitted at most once per crawl. + """ + + _INTERNAL_KEYS: frozenset[str] = frozenset( + { + "_auth_proxy", + "_dont_cache", + "_scheme_proxy", + "download_latency", + "redirect_reasons", + "redirect_times", + "redirect_ttl", + "redirect_urls", + "retry_times", + } + ) + + def __init__(self, crawler: Crawler) -> None: + super().__init__(crawler) + skip = frozenset(crawler.settings.getlist("META_COPY_WARN_SKIP_KEYS", [])) + self._keys: frozenset[str] = self._INTERNAL_KEYS - skip + self._warned: bool = False + + def get_processed_request( + self, request: Request, response: Response | None + ) -> Request | None: + if response is None: + return request + + if not self._warned: + found = self._keys & request.meta.keys() + if found: + spider_name = type(self.crawler.spider).__name__ + logger.warning( + f"{spider_name} yielded a request containing internal " + f"meta keys that were likely copied from response.meta " + f"and should not be forwarded to new requests: " + f"{sorted(found)}. See the MetaCopyDetectionMiddleware " + f"documentation for more information. Source response: " + f"{response}, target request: {request}", + extra={"spider": self.crawler.spider}, + ) + self._warned = True + + return request diff --git a/tests/test_spidermiddleware_metacopy.py b/tests/test_spidermiddleware_metacopy.py new file mode 100644 index 000000000..d137822cf --- /dev/null +++ b/tests/test_spidermiddleware_metacopy.py @@ -0,0 +1,121 @@ +from __future__ import annotations + +from logging import WARNING +from typing import TYPE_CHECKING + +import pytest + +from scrapy.http import Request, Response +from scrapy.spidermiddlewares.metacopy import MetaCopyDetectionMiddleware +from scrapy.spiders import Spider +from scrapy.utils.test import get_crawler + +if TYPE_CHECKING: + from scrapy.crawler import Crawler + + +def make_response(url: str = "https://example.com") -> Response: + response = Response(url) + response.request = Request(url) + return response + + +@pytest.fixture +def crawler() -> Crawler: + return get_crawler(Spider) + + +@pytest.fixture +def mw(crawler: Crawler) -> MetaCopyDetectionMiddleware: + return MetaCopyDetectionMiddleware.from_crawler(crawler) + + +def process( + mw: MetaCopyDetectionMiddleware, + requests: list[Request], + response: Response | None = None, +) -> list[Request]: + if response is None: + response = make_response() + return list(mw.process_spider_output(response, requests)) + + +class TestInternalKeysCheck: + def test_no_warning_for_clean_request( + self, mw: MetaCopyDetectionMiddleware, caplog: pytest.LogCaptureFixture + ) -> None: + req = Request("https://example.com/1", meta={"my_key": "value"}) + with caplog.at_level(WARNING): + process(mw, [req]) + assert not caplog.records + + def test_warns_on_internal_key( + self, mw: MetaCopyDetectionMiddleware, caplog: pytest.LogCaptureFixture + ) -> None: + req = Request("https://example.com/1", meta={"retry_times": 1}) + with caplog.at_level(WARNING): + process(mw, [req]) + assert len(caplog.records) == 1 + assert "retry_times" in caplog.text + assert "https://example.com/1" in caplog.text + + def test_warns_once_across_multiple_requests( + self, mw: MetaCopyDetectionMiddleware, caplog: pytest.LogCaptureFixture + ) -> None: + reqs = [ + Request("https://example.com/1", meta={"retry_times": 1}), + Request("https://example.com/2", meta={"retry_times": 2}), + ] + with caplog.at_level(WARNING): + process(mw, reqs) + assert len(caplog.records) == 1 + + def test_reports_all_found_keys( + self, mw: MetaCopyDetectionMiddleware, caplog: pytest.LogCaptureFixture + ) -> None: + req = Request( + "https://example.com/1", + meta={"retry_times": 1, "redirect_times": 2}, + ) + with caplog.at_level(WARNING): + process(mw, [req]) + assert "retry_times" in caplog.text + assert "redirect_times" in caplog.text + + def test_includes_source_response_in_message( + self, mw: MetaCopyDetectionMiddleware, caplog: pytest.LogCaptureFixture + ) -> None: + response = make_response("https://source.example.com") + req = Request("https://example.com/1", meta={"retry_times": 1}) + with caplog.at_level(WARNING): + process(mw, [req], response=response) + assert "https://source.example.com" in caplog.text + + def test_skip_keys_setting(self, caplog: pytest.LogCaptureFixture) -> None: + crawler = get_crawler(Spider, {"META_COPY_WARN_SKIP_KEYS": ["retry_times"]}) + mw = MetaCopyDetectionMiddleware.from_crawler(crawler) + req = Request("https://example.com/1", meta={"retry_times": 1}) + with caplog.at_level(WARNING): + process(mw, [req]) + assert not caplog.records + + def test_skip_keys_setting_partial(self, caplog: pytest.LogCaptureFixture) -> None: + crawler = get_crawler(Spider, {"META_COPY_WARN_SKIP_KEYS": ["retry_times"]}) + mw = MetaCopyDetectionMiddleware.from_crawler(crawler) + req = Request( + "https://example.com/1", + meta={"retry_times": 1, "redirect_times": 2}, + ) + with caplog.at_level(WARNING): + process(mw, [req]) + assert len(caplog.records) == 1 + assert "retry_times" not in caplog.text + assert "redirect_times" in caplog.text + + def test_no_warning_for_start_requests( + self, mw: MetaCopyDetectionMiddleware, caplog: pytest.LogCaptureFixture + ) -> None: + req = Request("https://example.com/1", meta={"retry_times": 1}) + with caplog.at_level(WARNING): + list(mw.process_spider_output(None, [req])) + assert not caplog.records From 712096bf62e7fdd68fc08f4ee03576ac0f4d1a13 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 10 Jun 2026 16:32:05 +0200 Subject: [PATCH 2/4] Address typing issues --- scrapy/spidermiddlewares/base.py | 7 +++++-- scrapy/spidermiddlewares/depth.py | 13 +++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/scrapy/spidermiddlewares/base.py b/scrapy/spidermiddlewares/base.py index e09f2d10e..28435da67 100644 --- a/scrapy/spidermiddlewares/base.py +++ b/scrapy/spidermiddlewares/base.py @@ -48,7 +48,10 @@ class BaseSpiderMiddleware: @_warn_spider_arg def process_spider_output( - self, response: Response, result: Iterable[Any], spider: Spider | None = None + self, + response: Response | None, + result: Iterable[Any], + spider: Spider | None = None, ) -> Iterable[Any]: for o in result: if (o := self._get_processed(o, response)) is not None: @@ -57,7 +60,7 @@ class BaseSpiderMiddleware: @_warn_spider_arg async def process_spider_output_async( self, - response: Response, + response: Response | None, result: AsyncIterator[Any], spider: Spider | None = None, ) -> AsyncIterator[Any]: diff --git a/scrapy/spidermiddlewares/depth.py b/scrapy/spidermiddlewares/depth.py index 9c387530c..054804119 100644 --- a/scrapy/spidermiddlewares/depth.py +++ b/scrapy/spidermiddlewares/depth.py @@ -55,19 +55,24 @@ class DepthMiddleware(BaseSpiderMiddleware): @_warn_spider_arg def process_spider_output( - self, response: Response, result: Iterable[Any], spider: Spider | None = None + self, + response: Response | None, + result: Iterable[Any], + spider: Spider | None = None, ) -> Iterable[Any]: - self._init_depth(response) + if response is not None: + self._init_depth(response) yield from super().process_spider_output(response, result) @_warn_spider_arg async def process_spider_output_async( self, - response: Response, + response: Response | None, result: AsyncIterator[Any], spider: Spider | None = None, ) -> AsyncIterator[Any]: - self._init_depth(response) + if response is not None: + self._init_depth(response) async for o in super().process_spider_output_async(response, result): yield o From d0d4b7cd9b56bfa098443d1e520274f66cbdfd4c Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 10 Jun 2026 16:39:06 +0200 Subject: [PATCH 3/4] Improve log capturing --- tests/test_spidermiddleware_metacopy.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/test_spidermiddleware_metacopy.py b/tests/test_spidermiddleware_metacopy.py index d137822cf..bfa27ac68 100644 --- a/tests/test_spidermiddleware_metacopy.py +++ b/tests/test_spidermiddleware_metacopy.py @@ -92,21 +92,21 @@ class TestInternalKeysCheck: assert "https://source.example.com" in caplog.text def test_skip_keys_setting(self, caplog: pytest.LogCaptureFixture) -> None: - crawler = get_crawler(Spider, {"META_COPY_WARN_SKIP_KEYS": ["retry_times"]}) - mw = MetaCopyDetectionMiddleware.from_crawler(crawler) - req = Request("https://example.com/1", meta={"retry_times": 1}) with caplog.at_level(WARNING): + crawler = get_crawler(Spider, {"META_COPY_WARN_SKIP_KEYS": ["retry_times"]}) + mw = MetaCopyDetectionMiddleware.from_crawler(crawler) + req = Request("https://example.com/1", meta={"retry_times": 1}) process(mw, [req]) assert not caplog.records def test_skip_keys_setting_partial(self, caplog: pytest.LogCaptureFixture) -> None: - crawler = get_crawler(Spider, {"META_COPY_WARN_SKIP_KEYS": ["retry_times"]}) - mw = MetaCopyDetectionMiddleware.from_crawler(crawler) - req = Request( - "https://example.com/1", - meta={"retry_times": 1, "redirect_times": 2}, - ) with caplog.at_level(WARNING): + crawler = get_crawler(Spider, {"META_COPY_WARN_SKIP_KEYS": ["retry_times"]}) + mw = MetaCopyDetectionMiddleware.from_crawler(crawler) + req = Request( + "https://example.com/1", + meta={"retry_times": 1, "redirect_times": 2}, + ) process(mw, [req]) assert len(caplog.records) == 1 assert "retry_times" not in caplog.text From 72685e065979f504dcb4827ee99f817793a0e960 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 10 Jun 2026 20:23:03 +0200 Subject: [PATCH 4/4] Simplify docs --- docs/topics/request-response.rst | 3 +-- docs/topics/spider-middleware.rst | 27 ++------------------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 660dbd4cf..da66d9d8c 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -231,8 +231,7 @@ Request objects Also mind that the :meth:`copy` and :meth:`replace` request methods :doc:`shallow-copy ` request metadata. - .. seealso:: :ref:`unsafe-meta-copy` for a more detailed explanation - and :class:`~scrapy.spidermiddlewares.metacopy.MetaCopyDetectionMiddleware` + .. seealso:: :class:`~scrapy.spidermiddlewares.metacopy.MetaCopyDetectionMiddleware` for a built-in middleware that warns about this issue at run time. .. autoattribute:: dont_filter diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 197e2f183..e1e263fc2 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -326,32 +326,9 @@ MetaCopyDetectionMiddleware Warns when a spider yields a request that contains internal meta keys which should not be copied from :attr:`response.meta ` - into new requests, or when two requests share the same meta dict object. + into new requests. See :attr:`~scrapy.http.Request.meta` to learn why. - Each warning is emitted at most once per crawl to avoid noise, but includes - the source response and the target request so the issue is easy to locate. - - .. _unsafe-meta-copy: - - Why copying response.meta is unsafe - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - - :attr:`response.meta ` is an alias for - :attr:`response.request.meta `, which is the meta - dict of the request that produced the response. That dict may contain keys - set by Scrapy's internal components — such as ``retry_times`` from - :class:`~scrapy.downloadermiddlewares.retry.RetryMiddleware` — that are - specific to that one request and must not be forwarded to unrelated - requests. Copying the entire dict (e.g. ``meta={**response.meta}``) or - passing the dict object directly (e.g. ``meta=response.meta``) propagates - those internal keys and can cause subtle bugs. For example, forwarding - ``retry_times`` reduces the number of retries available for the new request. - - To pass custom data between callbacks, prefer - :attr:`~scrapy.http.Request.cb_kwargs`. If you need to carry data through - :attr:`~scrapy.http.Request.meta`, copy only the specific keys your code - uses rather than the whole dict. See also `scrapy-sticky-meta-params - `_. + Only 1 warning is emitted per crawl. MetaCopyDetectionMiddleware settings ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~