Add a middleware to detect response.meta copies

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 <library/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

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 <scrapy.http.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 <scrapy.http.Response.meta>` is an alias for
+   :attr:`response.request.meta <scrapy.http.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
+   <https://pypi.org/project/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
 -----------------
 

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
 }
 

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

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