Deprecate Crawler.stop(). (#6999)

scrapy/core/engine.py

@@ -10,6 +10,7 @@ from __future__ import annotations
 import asyncio
 import logging
 import warnings
+from collections.abc import AsyncIterator, Callable, Coroutine, Generator
 from time import time
 from traceback import format_exc
 from typing import TYPE_CHECKING, Any
@@ -44,8 +45,6 @@ from scrapy.utils.python import global_object_name
 from scrapy.utils.reactor import CallLaterOnce
 
 if TYPE_CHECKING:
-    from collections.abc import AsyncIterator, Callable, Generator
-
     from twisted.internet.task import LoopingCall
 
     from scrapy.core.downloader import Downloader
@@ -102,7 +101,9 @@ class ExecutionEngine:
     def __init__(
         self,
         crawler: Crawler,
-        spider_closed_callback: Callable[[Spider], Deferred[None] | None],
+        spider_closed_callback: Callable[
+            [Spider], Coroutine[Any, Any, None] | Deferred[None] | None
+        ],
     ) -> None:
         self.crawler: Crawler = crawler
         self.settings: Settings = crawler.settings
@@ -113,9 +114,9 @@ class ExecutionEngine:
         self.spider: Spider | None = None
         self.running: bool = False
         self.paused: bool = False
-        self._spider_closed_callback: Callable[[Spider], Deferred[None] | None] = (
-            spider_closed_callback
-        )
+        self._spider_closed_callback: Callable[
+            [Spider], Coroutine[Any, Any, None] | Deferred[None] | None
+        ] = spider_closed_callback
         self.start_time: float | None = None
         self._start: AsyncIterator[Any] | None = None
         self._closewait: Deferred[None] | None = None
@@ -625,7 +626,11 @@ class ExecutionEngine:
         self.spider = None
 
         try:
-            if (d := self._spider_closed_callback(spider)) is not None:
-                await maybe_deferred_to_future(d)
+            aw = self._spider_closed_callback(spider)
+            # TODO: replace with ensure_awaitable() when we add it
+            if isinstance(aw, Coroutine):
+                await aw
+            elif isinstance(aw, Deferred):
+                await maybe_deferred_to_future(aw)
         except Exception:
             log_failure("Error running spider_closed_callback")

scrapy/crawler.py

@@ -5,6 +5,7 @@ import contextlib
 import logging
 import pprint
 import signal
+import warnings
 from abc import ABC, abstractmethod
 from typing import TYPE_CHECKING, Any, TypeVar
 
@@ -13,12 +14,13 @@ from twisted.internet.defer import Deferred, DeferredList, inlineCallbacks
 from scrapy import Spider, signals
 from scrapy.addons import AddonManager
 from scrapy.core.engine import ExecutionEngine
+from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.extension import ExtensionManager
 from scrapy.settings import Settings, overridden_settings
 from scrapy.signalmanager import SignalManager
 from scrapy.spiderloader import SpiderLoaderProtocol, get_spider_loader
 from scrapy.utils.asyncio import is_asyncio_available
-from scrapy.utils.defer import deferred_from_coro, deferred_to_future
+from scrapy.utils.defer import deferred_from_coro
 from scrapy.utils.log import (
     LogCounterHandler,
     configure_logging,
@@ -207,30 +209,28 @@ class Crawler:
         return self.spidercls.from_crawler(self, *args, **kwargs)
 
     def _create_engine(self) -> ExecutionEngine:
-        return ExecutionEngine(self, lambda _: self.stop())
+        return ExecutionEngine(self, lambda _: self.stop_async())
 
-    @inlineCallbacks
-    def stop(self) -> Generator[Deferred[Any], Any, None]:
+    def stop(self) -> Deferred[None]:
         """Start a graceful stop of the crawler and return a deferred that is
         fired when the crawler is stopped."""
-        if self.crawling:
-            self.crawling = False
-            assert self.engine
-            if self.engine.running:
-                yield deferred_from_coro(self.engine.stop_async())
+        warnings.warn(
+            "Crawler.stop() is deprecated, use stop_async() instead",
+            ScrapyDeprecationWarning,
+            stacklevel=2,
+        )
+        return deferred_from_coro(self.stop_async())
 
     async def stop_async(self) -> None:
         """Start a graceful stop of the crawler and complete when the crawler is stopped.
 
         .. versionadded:: VERSION
-
-        This function requires
-        :class:`~twisted.internet.asyncioreactor.AsyncioSelectorReactor` to be
-        installed.
         """
-        if not is_asyncio_available():
-            raise RuntimeError("Crawler.stop_async() requires AsyncioSelectorReactor.")
-        await deferred_to_future(self.stop())
+        if self.crawling:
+            self.crawling = False
+            assert self.engine
+            if self.engine.running:
+                await self.engine.stop_async()
 
     @staticmethod
     def _get_component(
@@ -447,7 +447,7 @@ class CrawlerRunner(CrawlerRunnerBase):
 
         Returns a deferred that is fired when they all have ended.
         """
-        return DeferredList(c.stop() for c in self.crawlers)
+        return DeferredList(deferred_from_coro(c.stop_async()) for c in self.crawlers)
 
     @inlineCallbacks
     def join(self) -> Generator[Deferred[Any], Any, None]:

scrapy/extensions/memusage.py

@@ -115,7 +115,7 @@ class MemoryUsage:
                     self.crawler.engine.close_spider_async(reason="memusage_exceeded")
                 )
             else:
-                self.crawler.stop()
+                _schedule_coro(self.crawler.stop_async())
         else:
             logger.info(
                 "Peak memory usage is %(virtualsize)dMiB",

tests/test_crawler.py

@@ -31,7 +31,11 @@ from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.extensions.throttle import AutoThrottle
 from scrapy.settings import Settings, default_settings
 from scrapy.utils.asyncio import call_later
-from scrapy.utils.defer import deferred_f_from_coro_f, deferred_from_coro
+from scrapy.utils.defer import (
+    deferred_f_from_coro_f,
+    deferred_from_coro,
+    maybe_deferred_to_future,
+)
 from scrapy.utils.log import configure_logging, get_scrapy_root_handler
 from scrapy.utils.spider import DefaultSpider
 from scrapy.utils.test import get_crawler, get_reactor_settings
@@ -1174,3 +1178,14 @@ def test_log_scrapy_info(settings, items, caplog):
         f"{item}': '[^']+('\n +'[^']+)*" for item in items
     )
     assert re.search(r"^Versions:\n{'" + expected_items_pattern + "'}$", version_string)
+
+
+@deferred_f_from_coro_f
+async def test_deprecated_crawler_stop() -> None:
+    crawler = get_crawler(DefaultSpider)
+    d = crawler.crawl()
+    await maybe_deferred_to_future(d)
+    with pytest.warns(
+        ScrapyDeprecationWarning, match=r"Crawler.stop\(\) is deprecated"
+    ):
+        await maybe_deferred_to_future(crawler.stop())

tests/test_engine.py

@@ -180,12 +180,12 @@ class CrawlerRun:
         dispatcher.connect(self.stop, signals.engine_stopped)
         await maybe_deferred_to_future(self.deferred)
 
-    def stop(self):
+    async def stop(self):
         for name, signal in vars(signals).items():
             if not name.startswith("_"):
                 disconnect_all(signal)
         self.deferred.callback(None)
-        return self.crawler.stop()
+        await self.crawler.stop_async()
 
     def geturl(self, path: str) -> str:
         return self.mockserver.url(path)
@@ -726,3 +726,15 @@ class TestEngineCloseSpider:
         await engine.open_spider_async()
         await engine.close_spider_async()
         assert "Error running spider_closed_callback" in caplog.text
+
+    @deferred_f_from_coro_f
+    async def test_exception_async_callback(
+        self, crawler: Crawler, caplog: pytest.LogCaptureFixture
+    ) -> None:
+        async def cb(_):
+            raise ValueError
+
+        engine = ExecutionEngine(crawler, cb)
+        await engine.open_spider_async()
+        await engine.close_spider_async()
+        assert "Error running spider_closed_callback" in caplog.text

tests/test_scheduler.py

@@ -15,6 +15,7 @@ from scrapy.core.scheduler import BaseScheduler, Scheduler
 from scrapy.crawler import Crawler
 from scrapy.http import Request
 from scrapy.spiders import Spider
+from scrapy.utils.defer import _schedule_coro
 from scrapy.utils.httpobj import urlparse_cached
 from scrapy.utils.misc import load_object
 from scrapy.utils.test import get_crawler
@@ -114,7 +115,7 @@ class SchedulerHandler(ABC):
 
     def close_scheduler(self):
         self.scheduler.close("finished")
-        self.mock_crawler.stop()
+        _schedule_coro(self.mock_crawler.stop_async())
         self.mock_crawler.engine.downloader.close()
 
     def setup_method(self):