From 9149b6e7fc47743cdbcb5d3471a9650a2163b09b Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 23 Jun 2025 20:42:11 +0500 Subject: [PATCH 001/298] Use new pytest for new Twisted. (#6893) --- tox.ini | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index 5680d98d1..d3dc7a3db 100644 --- a/tox.ini +++ b/tox.ini @@ -14,17 +14,17 @@ deps = pexpect >= 4.8.0 pyftpdlib >= 2.0.1 pygments - pytest != 8.2.* # https://github.com/pytest-dev/pytest/issues/12275 + pytest pytest-cov >= 4.0.0 pytest-xdist sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 testfixtures pywin32; sys_platform == "win32" - Twisted < 25.5.0 # https://github.com/twisted/twisted/issues/12467 [testenv] deps = {[test-requirements]deps} + pytest >= 8.4.1 # https://github.com/pytest-dev/pytest/pull/13502 # mitmproxy does not support PyPy mitmproxy; implementation_name != "pypy" @@ -97,6 +97,8 @@ commands = [pinned] basepython = python3.9 deps = + # pytest 8.4.1 adds support for Twisted 25.5.0 but drops support for Twisted < 24.10.0 + pytest==8.4.0 Protego==0.1.15 Twisted==21.7.0 cryptography==37.0.0 @@ -196,6 +198,7 @@ basepython = pypy3.10 deps = PyPyDispatcher==2.1.0 {[test-requirements]deps} + pytest==8.4.0 Protego==0.1.15 Twisted==21.7.0 cryptography==41.0.5 From 020bfa7e5fd98c53db17e0c72db88d43d6fbf1f6 Mon Sep 17 00:00:00 2001 From: Thalison Fernandes Date: Mon, 23 Jun 2025 12:49:57 -0300 Subject: [PATCH 002/298] Add .venv folder to .gitignore (#6901) --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 0a3f0ac1c..e02c2241d 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ docs/build *egg-info .tox venv +.venv build dist .idea From 843ad1afb1383a263b8c8da9004a899a71ea864d Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 23 Jun 2025 20:56:48 +0500 Subject: [PATCH 003/298] Replace Black with Ruff in contributing.rst. (#6903) --- docs/contributing.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 0172887d6..3976d34c2 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -251,10 +251,10 @@ Coding style Please follow these coding conventions when writing code for inclusion in Scrapy: -* We use `black `_ for code formatting. +* We use `Ruff `_ for code formatting. There is a hook in the pre-commit config that will automatically format your code before every commit. You can also - run black manually with ``tox -e pre-commit``. + run Ruff manually with ``tox -e pre-commit``. * Don't put your name in the code you contribute; git provides enough metadata to identify author of the code. From 5e20b46e35c3affc5df7b5f63778e4fb924b28a8 Mon Sep 17 00:00:00 2001 From: Thalison Fernandes Date: Mon, 23 Jun 2025 13:56:29 -0300 Subject: [PATCH 004/298] Fix FileFeedStorage handling of Windows paths without file:// scheme (#6897) --- scrapy/extensions/feedexport.py | 2 +- tests/test_feedexport.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 8bcd4e40d..4ac546765 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -185,7 +185,7 @@ class StdoutFeedStorage: @implementer(IFeedStorage) class FileFeedStorage: def __init__(self, uri: str, *, feed_options: dict[str, Any] | None = None): - self.path: str = file_uri_to_path(uri) + self.path: str = file_uri_to_path(uri) if uri.startswith("file://") else uri feed_options = feed_options or {} self.write_mode: OpenBinaryMode = ( "wb" if feed_options.get("overwrite", False) else "ab" diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 44cd10ec3..7c6425f19 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -143,6 +143,11 @@ class TestFileFeedStorage(unittest.TestCase): finally: path.unlink() + def test_preserves_windows_path_without_file_scheme(self): + path = r"C:\Users\user\Desktop\test.txt" + storage = FileFeedStorage(path) + assert storage.path == path + class TestFTPFeedStorage(unittest.TestCase): def get_test_spider(self, settings=None): From b53faacfcd64f6717645bf4cd33159cef87cd5d9 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 23 Jun 2025 21:58:54 +0500 Subject: [PATCH 005/298] Add .vscode to .gitignore. (#6907) --- .gitignore | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index e02c2241d..4100bcd97 100644 --- a/.gitignore +++ b/.gitignore @@ -5,15 +5,16 @@ _trial_temp* dropin.cache docs/build *egg-info -.tox -venv -.venv -build -dist -.idea +.tox/ +venv/ +.venv/ +build/ +dist/ +.idea/ +.vscode/ htmlcov/ -.coverage .pytest_cache/ +.coverage .coverage.* coverage.* *.junit.xml @@ -27,4 +28,4 @@ test-output.* Thumbs.db # OSX miscellaneous -.DS_Store \ No newline at end of file +.DS_Store From 7fbd56bc9baa33a4eb874f4337d740f5f25cfad3 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 23 Jun 2025 22:36:54 +0500 Subject: [PATCH 006/298] Handle exceptions in _start_request_processing(), cancel it on engine stop (#6900) --- scrapy/core/engine.py | 48 ++++++++++++++++++++++++++++++------------ scrapy/core/scraper.py | 1 - tests/test_engine.py | 23 +++++++++++++++++++- 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 0df9ad2b2..6c6d24d5c 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -7,12 +7,13 @@ For more information see docs/topics/architecture.rst from __future__ import annotations +import asyncio import logging from time import time from traceback import format_exc from typing import TYPE_CHECKING, Any, TypeVar, cast -from twisted.internet.defer import Deferred, inlineCallbacks, succeed +from twisted.internet.defer import CancelledError, Deferred, inlineCallbacks, succeed from twisted.internet.task import LoopingCall from twisted.python.failure import Failure @@ -102,6 +103,8 @@ class ExecutionEngine: ) self.start_time: float | None = None self._start: AsyncIterator[Any] | None = None + self._closewait: Deferred[None] | None = None + self._start_request_processing_dfd: Deferred[None] | None = None downloader_cls: type[Downloader] = load_object(self.settings["DOWNLOADER"]) try: self.scheduler_cls: type[BaseScheduler] = self._get_scheduler_class( @@ -133,9 +136,9 @@ class ExecutionEngine: self.signals.send_catch_log_deferred(signal=signals.engine_started) ) self.running = True - self._closewait: Deferred[None] = Deferred() + self._closewait = Deferred() if _start_request_processing: - self._start_request_processing() + self._start_request_processing_dfd = self._start_request_processing() await maybe_deferred_to_future(self._closewait) def stop(self) -> Deferred[None]: @@ -146,12 +149,16 @@ class ExecutionEngine: await maybe_deferred_to_future( self.signals.send_catch_log_deferred(signal=signals.engine_stopped) ) - self._closewait.callback(None) + if self._closewait: + self._closewait.callback(None) if not self.running: raise RuntimeError("Engine not running") self.running = False + if self._start_request_processing_dfd is not None: + self._start_request_processing_dfd.cancel() + self._start_request_processing_dfd = None dfd = ( self.close_spider(self.spider, reason="shutdown") if self.spider is not None @@ -213,17 +220,30 @@ class ExecutionEngine: # Starts the processing of scheduled requests, as well as a periodic # call to that processing method for scenarios where the scheduler # reports having pending requests but returns none. - assert self._slot is not None # typing - self._slot.nextcall.schedule() - self._slot.heartbeat.start(self._SLOT_HEARTBEAT_INTERVAL) + try: + assert self._slot is not None # typing + self._slot.nextcall.schedule() + self._slot.heartbeat.start(self._SLOT_HEARTBEAT_INTERVAL) - while self._start and self.spider: - await self._process_start_next() - if not self.needs_backout(): - # Give room for the outcome of self._process_start_next() to be - # processed before continuing with the next iteration. - self._slot.nextcall.schedule() - await self._slot.nextcall.wait() + while self._start and self.spider: + await self._process_start_next() + if not self.needs_backout(): + # Give room for the outcome of self._process_start_next() to be + # processed before continuing with the next iteration. + self._slot.nextcall.schedule() + await self._slot.nextcall.wait() + except (asyncio.exceptions.CancelledError, CancelledError): + # self.stop() has cancelled us, nothing to do + return + except Exception: + # an error happened, log it and stop the engine + self._start_request_processing_dfd = None + logger.error( + "Error while processing requests from start()", + exc_info=True, + extra={"spider": self.spider}, + ) + await maybe_deferred_to_future(self.stop()) def _start_scheduled_requests(self) -> None: if self._slot is None or self._slot.closing is not None or self.paused: diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index ac720e03f..a2fe281e0 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -139,7 +139,6 @@ class Scraper: def _check_if_closing(self) -> None: assert self.slot is not None # typing - assert self.crawler.spider if self.slot.closing and self.slot.is_idle(): assert self.crawler.spider self.slot.closing.callback(self.crawler.spider) diff --git a/tests/test_engine.py b/tests/test_engine.py index b2e436425..a3c5243d5 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -25,6 +25,7 @@ import attr import pytest from itemadapter import ItemAdapter from pydispatch import dispatcher +from testfixtures import LogCapture from twisted.internet import defer, reactor from twisted.trial import unittest from twisted.web import server, static, util @@ -448,7 +449,7 @@ class TestEngine(TestEngineBase): @defer.inlineCallbacks def test_start_already_running_exception(self): e = ExecutionEngine(get_crawler(MySpider), lambda _: None) - yield e.open_spider(MySpider(), []) + yield e.open_spider(MySpider()) e.start() def cb(exc: BaseException) -> None: @@ -459,6 +460,26 @@ class TestEngine(TestEngineBase): finally: yield e.stop() + @defer.inlineCallbacks + def test_start_request_processing_exception(self): + class BadRequestFingerprinter: + def fingerprint(self, request): + raise ValueError # to make Scheduler.enqueue_request() fail + + class SimpleSpider(Spider): + name = "simple" + + async def start(self): + yield Request("data:,") + + crawler = get_crawler( + SimpleSpider, {"REQUEST_FINGERPRINTER_CLASS": BadRequestFingerprinter} + ) + with LogCapture() as log: + yield crawler.crawl() + assert "Error while processing requests from start()" in str(log) + assert "Spider closed (shutdown)" in str(log) + def test_short_timeout(self): args = ( sys.executable, From ef01a953b108b6c60d5ed5dcf85d3a1eb7ae7a2a Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 25 Jun 2025 17:29:29 +0500 Subject: [PATCH 007/298] Fix setting ExecutionEngine._slot to None on close. (#6910) * Fix setting ExecutionEngine._slot to None on close. * Check self._slot in needs_backout(). --- scrapy/core/engine.py | 12 +++++++++--- tests/test_engine_loop.py | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 6c6d24d5c..b3a5e1906 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -262,10 +262,10 @@ class ExecutionEngine: See :ref:`start-requests-lazy` for an example. """ - assert self._slot is not None # typing assert self.scraper.slot is not None # typing return ( not self.running + or not self._slot or bool(self._slot.closing) or self.downloader.needs_backout() or self.scraper.slot.needs_backout() @@ -528,10 +528,16 @@ class ExecutionEngine: ) ) - dfd.addBoth(lambda _: setattr(self, "slot", None)) + def unassign_slot(_: Any) -> None: + self._slot = None + + dfd.addBoth(unassign_slot) dfd.addErrback(log_failure("Error while unassigning slot")) - dfd.addBoth(lambda _: setattr(self, "spider", None)) + def unassign_spider(_: Any) -> None: + self.spider = None + + dfd.addBoth(unassign_spider) dfd.addErrback(log_failure("Error while unassigning spider")) dfd.addBoth(lambda _: self._spider_closed_callback(spider)) diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index c7dbc82d4..8211667fb 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -112,7 +112,7 @@ class MainTestCase(TestCase): with LogCapture(level=ERROR) as log: await maybe_deferred_to_future(crawler.crawl()) - assert not log.records, f"{log.records=}" + assert not log.records finish_reason = crawler.stats.get_value("finish_reason") assert finish_reason == "shutdown", f"{finish_reason=}" expected_urls = [] From 5fbab843bd819da74d2c8cad88b2d3271592eb18 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 27 Jun 2025 16:05:02 +0500 Subject: [PATCH 008/298] Fix invalid XML samples in tests that break with lxml 6. (#6919) --- tests/spiders.py | 4 ++-- tests/test_linkextractors.py | 6 +++--- tests/test_spider.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/spiders.py b/tests/spiders.py index c47f2bd2b..43cdcaef3 100644 --- a/tests/spiders.py +++ b/tests/spiders.py @@ -418,7 +418,7 @@ class CrawlSpiderWithParseMethod(MockServerSpider, CrawlSpider): async def start(self): test_body = b""" - Page title<title></head> + <head><title>Page title

Item 200

Item 201

@@ -472,7 +472,7 @@ class CrawlSpiderWithErrback(CrawlSpiderWithParseMethod): async def start(self): test_body = b""" - Page title<title></head> + <head><title>Page title

Item 200

Item 201

diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 1bff369af..15d358d2a 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -152,7 +152,7 @@ class Base: def test_nofollow(self): """Test the extractor's behaviour for links with rel='nofollow'""" - html = b"""Page title<title> + html = b"""<html><head><title>Page title