From d2290c35c23b9fe1973764f75a5f7cb72033108a Mon Sep 17 00:00:00 2001 From: Adnan Awan Date: Mon, 8 Jun 2026 18:44:30 +0500 Subject: [PATCH] Allow configuring the log level of the retry give-up message (#7567) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allow configuring the log level of the retry give-up message The "Gave up retrying ..." message was always logged at ERROR, which inflates the log_count/ERROR stat even when giving up on a request is expected (e.g. broad crawls hitting dead hosts). Add a RETRY_GIVE_UP_LOG_LEVEL setting, a give_up_log_level argument to get_retry_request(), and a give_up_log_level request meta key to override it per request. The value accepts a level name ("WARNING") or number (logging.WARNING). The default ("ERROR") preserves the previous behaviour. Fixes #5297, fixes #4622 Co-Authored-By: Claude Opus 4.8 (1M context) * Address Adrian's review feedback: simplify and reorganize docs - Move give_up_log_level reqmeta section before max_retry_times (alphabetical order) - Simplify give_up_log_level section in request-response.rst (brief, links to setting) - Simplify RETRY_GIVE_UP_LOG_LEVEL setting docs in downloader-middleware.rst - Change 'When initialized' to 'When set' in max_retry_times section - Docs now follow pattern of linking to complementary setting/meta key rather than duplicating information Per Adrian's feedback: keep docs concise and cross-link setting ↔ meta key * Address Adrian's code review feedback on RETRY_GIVE_UP_LOG_LEVEL feature - Fix alphabetical ordering of give_up_log_level in request-response.rst - Remove circular references: change 'see X for details' to 'see also X' pattern - Simplify docstring for give_up_log_level parameter (4 lines → 2 lines) - Update test domains from www.scrapytest.org to example.com * Apply suggestion from @AdrianAtZyte * Minor changes * Fix example formatting. --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Adrian Co-authored-by: Andrey Rakhmatullin --- docs/topics/downloader-middleware.rst | 15 +++ docs/topics/request-response.rst | 11 ++- scrapy/downloadermiddlewares/retry.py | 28 +++++- scrapy/settings/default_settings.py | 2 + tests/test_downloadermiddleware_retry.py | 114 +++++++++++++++++++++++ 5 files changed, 165 insertions(+), 5 deletions(-) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index a51e431d2..91ee38b85 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -1053,6 +1053,21 @@ has been exceeded (see :setting:`RETRY_TIMES`). To learn about uncaught exception propagation, see :meth:`~scrapy.downloadermiddlewares.DownloaderMiddleware.process_exception`. +.. setting:: RETRY_GIVE_UP_LOG_LEVEL + +RETRY_GIVE_UP_LOG_LEVEL +^^^^^^^^^^^^^^^^^^^^^^^ + +Default: ``"ERROR"`` + +:ref:`Logging level ` used for the message logged when a request +exceeds its retries. + +Can be a level name (e.g. ``"WARNING"``) or a number (e.g. ``logging.WARNING`` +or ``30``). + +See also: :reqmeta:`give_up_log_level`, :func:`get_retry_request`. + .. setting:: RETRY_PRIORITY_ADJUST RETRY_PRIORITY_ADJUST diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index a4f031803..8fd3de621 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -714,6 +714,7 @@ Those are: * :reqmeta:`download_timeout` * ``ftp_password`` (See :setting:`FTP_PASSWORD` for more info) * ``ftp_user`` (See :setting:`FTP_USER` for more info) +* :reqmeta:`give_up_log_level` * :reqmeta:`handle_httpstatus_all` * :reqmeta:`handle_httpstatus_list` * :reqmeta:`is_start_request` @@ -790,12 +791,20 @@ download_fail_on_dataloss Whether or not to fail on broken responses. See: :setting:`DOWNLOAD_FAIL_ON_DATALOSS`. +.. reqmeta:: give_up_log_level + +give_up_log_level +----------------- + +:ref:`Logging level ` used for the message logged when a request +exceeds its retries. See :setting:`RETRY_GIVE_UP_LOG_LEVEL` for details. + .. reqmeta:: max_retry_times max_retry_times --------------- -The meta key is used set retry times per request. When initialized, the +The meta key is used set retry times per request. When set, the :reqmeta:`max_retry_times` meta key takes higher precedence over the :setting:`RETRY_TIMES` setting. diff --git a/scrapy/downloadermiddlewares/retry.py b/scrapy/downloadermiddlewares/retry.py index d38b4b9db..5f125cae4 100644 --- a/scrapy/downloadermiddlewares/retry.py +++ b/scrapy/downloadermiddlewares/retry.py @@ -12,7 +12,7 @@ once the spider has finished crawling all regular (non-failed) pages. from __future__ import annotations -from logging import Logger, getLogger +from logging import Logger, getLevelName, getLogger from typing import TYPE_CHECKING from scrapy.exceptions import NotConfigured @@ -43,6 +43,7 @@ def get_retry_request( max_retry_times: int | None = None, priority_adjust: int | None = None, logger: Logger = retry_logger, + give_up_log_level: int | str | None = None, stats_base_key: str = "retry", ) -> Request | None: """ @@ -51,14 +52,16 @@ def get_retry_request( exhausted. For example, in a :class:`~scrapy.Spider` callback, you could use it as - follows:: + follows: + + .. code-block:: python def parse(self, response): if not response.text: new_request_or_none = get_retry_request( response.request, spider=self, - reason='empty', + reason="empty", ) return new_request_or_none @@ -82,6 +85,10 @@ def get_retry_request( *logger* is the logging.Logger object to be used when logging messages + *give_up_log_level* is the :ref:`logging level ` used for the + message logged when a request exceeds its retries. See + :setting:`RETRY_GIVE_UP_LOG_LEVEL` for details. + *stats_base_key* is a string to be used as the base key for the retry-related job stats """ @@ -114,8 +121,16 @@ def get_retry_request( stats.inc_value(f"{stats_base_key}/count") stats.inc_value(f"{stats_base_key}/reason_count/{reason}") return new_request + if give_up_log_level is None: + give_up_log_level = settings["RETRY_GIVE_UP_LOG_LEVEL"] + if isinstance(give_up_log_level, str): + level = getLevelName(give_up_log_level) + if not isinstance(level, int): + raise ValueError(f"Invalid give-up log level: {give_up_log_level!r}") + give_up_log_level = level stats.inc_value(f"{stats_base_key}/max_reached") - logger.error( + logger.log( + give_up_log_level, "Gave up retrying %(request)s (failed %(retry_times)d times): %(reason)s", {"request": request, "retry_times": retry_times, "reason": reason}, extra={"spider": spider}, @@ -132,6 +147,7 @@ class RetryMiddleware: self.max_retry_times = settings.getint("RETRY_TIMES") self.retry_http_codes = {int(x) for x in settings.getlist("RETRY_HTTP_CODES")} self.priority_adjust = settings.getint("RETRY_PRIORITY_ADJUST") + self.give_up_log_level = settings["RETRY_GIVE_UP_LOG_LEVEL"] self.exceptions_to_retry = tuple( load_object(x) if isinstance(x, str) else x for x in settings.getlist("RETRY_EXCEPTIONS") @@ -175,6 +191,9 @@ class RetryMiddleware: ) -> Request | None: max_retry_times = request.meta.get("max_retry_times", self.max_retry_times) priority_adjust = request.meta.get("priority_adjust", self.priority_adjust) + give_up_log_level = request.meta.get( + "give_up_log_level", self.give_up_log_level + ) assert self.crawler.spider return get_retry_request( request, @@ -182,4 +201,5 @@ class RetryMiddleware: spider=self.crawler.spider, max_retry_times=max_retry_times, priority_adjust=priority_adjust, + give_up_log_level=give_up_log_level, ) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index de03c0107..909a5fd5c 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -156,6 +156,7 @@ __all__ = [ "REQUEST_FINGERPRINTER_CLASS", "RETRY_ENABLED", "RETRY_EXCEPTIONS", + "RETRY_GIVE_UP_LOG_LEVEL", "RETRY_HTTP_CODES", "RETRY_PRIORITY_ADJUST", "RETRY_TIMES", @@ -469,6 +470,7 @@ RETRY_EXCEPTIONS = [ OSError, "scrapy.core.downloader.handlers.http11.TunnelError", ] +RETRY_GIVE_UP_LOG_LEVEL = "ERROR" RETRY_HTTP_CODES = [500, 502, 503, 504, 522, 524, 408, 429] RETRY_PRIORITY_ADJUST = -1 RETRY_TIMES = 2 # initial response + 2 retries = 3 requests diff --git a/tests/test_downloadermiddleware_retry.py b/tests/test_downloadermiddleware_retry.py index 50946899a..56d21a4d2 100644 --- a/tests/test_downloadermiddleware_retry.py +++ b/tests/test_downloadermiddleware_retry.py @@ -84,6 +84,39 @@ class TestRetry: ) assert self.crawler.stats.get_value("retry/count") == 2 + def test_give_up_log_level_setting(self): + crawler = get_crawler( + DefaultSpider, settings_dict={"RETRY_GIVE_UP_LOG_LEVEL": "WARNING"} + ) + crawler.spider = crawler._create_spider() + mw = RetryMiddleware.from_crawler(crawler) + mw.max_retry_times = 0 + req = Request("http://example.com/503") + rsp = Response("http://example.com/503", body=b"", status=503) + with LogCapture() as log: + assert mw.process_response(req, rsp) is rsp + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "WARNING", + f"Gave up retrying {req} (failed 1 times): 503 Service Unavailable", + ) + ) + + def test_give_up_log_level_meta(self): + self.mw.max_retry_times = 0 + req = Request("http://example.com/503", meta={"give_up_log_level": "WARNING"}) + rsp = Response("http://example.com/503", body=b"", status=503) + with LogCapture() as log: + assert self.mw.process_response(req, rsp) is rsp + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "WARNING", + f"Gave up retrying {req} (failed 1 times): 503 Service Unavailable", + ) + ) + def test_twistederrors(self): exceptions = [ ConnectError, @@ -612,6 +645,87 @@ class TestGetRetryRequest: ) ) + def test_give_up_log_level_default(self): + request = Request("https://example.com") + spider = self.get_spider() + with LogCapture() as log: + get_retry_request( + request, + spider=spider, + max_retry_times=0, + ) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "ERROR", + f"Gave up retrying {request} (failed 1 times): unspecified", + ) + ) + + def test_give_up_log_level_argument_name(self): + request = Request("https://example.com") + spider = self.get_spider() + with LogCapture() as log: + get_retry_request( + request, + spider=spider, + max_retry_times=0, + give_up_log_level="WARNING", + ) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "WARNING", + f"Gave up retrying {request} (failed 1 times): unspecified", + ) + ) + + def test_give_up_log_level_argument_number(self): + request = Request("https://example.com") + spider = self.get_spider() + with LogCapture() as log: + get_retry_request( + request, + spider=spider, + max_retry_times=0, + give_up_log_level=logging.WARNING, + ) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "WARNING", + f"Gave up retrying {request} (failed 1 times): unspecified", + ) + ) + + def test_give_up_log_level_setting(self): + request = Request("https://example.com") + spider = self.get_spider({"RETRY_GIVE_UP_LOG_LEVEL": "WARNING"}) + with LogCapture() as log: + get_retry_request( + request, + spider=spider, + max_retry_times=0, + ) + log.check_present( + ( + "scrapy.downloadermiddlewares.retry", + "WARNING", + f"Gave up retrying {request} (failed 1 times): unspecified", + ) + ) + + def test_give_up_log_level_invalid(self): + request = Request("https://example.com") + spider = self.get_spider() + with pytest.raises(ValueError, match="Invalid give-up log level"): + get_retry_request( + request, + spider=spider, + max_retry_times=0, + give_up_log_level="NOT_A_LEVEL", + ) + def test_custom_stats_key(self): request = Request("https://example.com") spider = self.get_spider()