diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index f060710a4..acbf7ddef 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -338,15 +338,22 @@ spider_error .. signal:: spider_error .. function:: spider_error(failure, response, spider) - Sent when a spider callback generates an error (i.e. raises an exception). + Sent when a spider callback or the :meth:`~scrapy.Spider.start` method of a + spider generates an error (i.e. raises an exception). + + .. versionchanged:: VERSION + Exceptions from :meth:`~scrapy.Spider.start` are also reported, see + :ref:`start-error`. This signal does not support :ref:`asynchronous handlers `. :param failure: the exception raised :type failure: twisted.python.failure.Failure - :param response: the response being processed when the exception was raised - :type response: :class:`~scrapy.http.Response` object + :param response: the response being processed when the exception was + raised, or ``None`` if the exception came from + :meth:`~scrapy.Spider.start`. + :type response: :class:`~scrapy.http.Response` | ``None`` :param spider: the spider which raised the exception :type spider: :class:`~scrapy.Spider` object diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index e68c208b7..95c80d5dc 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -411,6 +411,38 @@ scheduled requests: await self.crawler.signals.wait_for(signals.scheduler_empty) yield item_or_request +.. _start-error: + +Handling start errors +--------------------- + +An exception raised by :meth:`~scrapy.Spider.start` ends its iteration, so any +remaining start items and requests are never sent. Scrapy logs the exception, +sends the :signal:`spider_error` signal, and, once the already scheduled +requests are done, closes the spider with the ``start_error`` +:stat:`finish_reason`. + +.. versionchanged:: VERSION + The close reason used to be ``finished``, and neither the + :signal:`spider_error` signal nor the :stat:`spider_exceptions/count` stat + reported the exception. + +To keep the iteration going, catch the exception yourself: + +.. code-block:: python + + async def start(self): + for url in self.start_urls: + try: + request = Request(url) + except ValueError: + self.logger.exception(f"Skipping start URL {url}") + else: + yield request + +To stop the crawl instead, and choose your own :stat:`finish_reason`, raise +:exc:`~scrapy.exceptions.CloseSpider`. + .. _builtin-spiders: Generic Spiders diff --git a/docs/topics/stats.rst b/docs/topics/stats.rst index b558c1cf2..6fb7783d1 100644 --- a/docs/topics/stats.rst +++ b/docs/topics/stats.rst @@ -301,6 +301,10 @@ one per actual value of the placeholder. - ``shutdown``: the crawl was interrupted, e.g. by a system signal such as ``SIGINT`` (:kbd:`Ctrl-C`). + - ``start_error``: :meth:`~scrapy.Spider.start` raised an exception, so + some :ref:`start requests ` may never have been sent, + see :ref:`start-error`. + Third-party components and your own code may use any other reason, e.g. by raising :exc:`~scrapy.exceptions.CloseSpider` with it. @@ -721,18 +725,21 @@ one per actual value of the placeholder. .. stat:: spider_exceptions/count ``spider_exceptions/count`` - Number of unhandled exceptions raised by spider callbacks. + Number of unhandled exceptions raised by spider callbacks or by + :meth:`~scrapy.Spider.start`. - Set by the :ref:`scraper `. + Set by the :ref:`engine ` and the :ref:`scraper + `. .. stat:: spider_exceptions/{exception} ``spider_exceptions/{exception}`` - Number of unhandled exceptions raised by spider callbacks, per exception, - where ``{exception}`` is the class name of the exception, e.g. + Same as :stat:`spider_exceptions/count`, per exception, where + ``{exception}`` is the class name of the exception, e.g. ``spider_exceptions/ValueError``. - Set by the :ref:`scraper `. + Set by the :ref:`engine ` and the :ref:`scraper + `. .. stat:: start_time diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index efac5f71c..b7a1e0fcd 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -125,6 +125,9 @@ class ExecutionEngine: ] = spider_closed_callback self.start_time: float | None = None self._start: AsyncIterator[Any] | None = None + # Whether Spider.start() raised, i.e. some start items or requests may + # never have reached the engine. + self._start_error: bool = False self._closewait: Deferred[None] | None = None self._start_request_processing_awaitable: ( asyncio.Future[None] | Deferred[None] | None @@ -277,13 +280,30 @@ class ExecutionEngine: item_or_request = await anext(self._start) except StopAsyncIteration: self._start = None + except CloseSpider as exception: + self._start = None + _schedule_coro( + self.close_spider_async(reason=exception.reason or "cancelled") + ) except Exception as exception: self._start = None + self._start_error = True exception_traceback = format_exc() logger.error( f"Error while reading start items and requests: {exception}.\n{exception_traceback}", exc_info=True, ) + self.signals.send_catch_log( + signal=signals.spider_error, + failure=Failure(), + response=None, + spider=self.spider, + ) + assert self.crawler.stats + self.crawler.stats.inc_value("spider_exceptions/count") + self.crawler.stats.inc_value( + f"spider_exceptions/{type(exception).__name__}" + ) else: if not self.spider: return # spider already closed @@ -579,7 +599,8 @@ class ExecutionEngine: if DontCloseSpider in detected_ex: return if self.spider_is_idle(): - ex = detected_ex.get(CloseSpider, CloseSpider(reason="finished")) + default_reason = "start_error" if self._start_error else "finished" + ex = detected_ex.get(CloseSpider, CloseSpider(reason=default_reason)) assert isinstance(ex, CloseSpider) # typing _schedule_coro(self.close_spider_async(reason=ex.reason)) diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index ccaccddf2..fa5bc1c1e 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -56,8 +56,12 @@ class DontCloseSpider(Exception): class CloseSpider(Exception): - """Raised from a :ref:`spider callback ` to request the - spider to be closed/stopped. + """Raised from a :ref:`spider callback ` or from + :meth:`~scrapy.Spider.start` to request the spider to be closed/stopped. + + .. versionchanged:: VERSION + Raising it from :meth:`~scrapy.Spider.start` closes the spider, instead + of being reported as a start error. *reason* is a string with the reason for closing. diff --git a/tests/test_crawler_subprocess.py b/tests/test_crawler_subprocess.py index 240482586..defee1041 100644 --- a/tests/test_crawler_subprocess.py +++ b/tests/test_crawler_subprocess.py @@ -400,7 +400,7 @@ class TestAsyncCrawlerProcessSubprocess(TestCrawlerProcessSubprocessBase): def test_reactorless_import_hook(self) -> None: log = self.run_script("reactorless_import_hook.py") assert "Not using a Twisted reactor" in log - assert "Spider closed (finished)" in log + assert "Spider closed (start_error)" in log assert "ImportError: Import of twisted.internet.reactor is forbidden" in log def test_reactorless_import_hook_uninstall(self) -> None: diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index 14ec3d184..1ecf8b8de 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING, Any from scrapy import Request, Spider, signals from scrapy.core.scheduler import BaseScheduler +from scrapy.exceptions import CloseSpider from scrapy.utils.asyncio import call_later, sleep from scrapy.utils.test import get_crawler from tests.mockserver.http import MockServer @@ -140,6 +141,74 @@ class TestMain: assert crawler.stats.get_value("finish_reason") == "shutdown" assert not actual_urls + @coroutine_test + async def test_start_error(self, caplog: pytest.LogCaptureFixture) -> None: + class TestSpider(Spider): + name = "test" + + async def start(self): + yield Request("data:,a") + raise ValueError + + def parse(self, response): + pass + + actual_urls = [] + errors = [] + + def track_url(request, spider): + actual_urls.append(request.url) + + def track_error(failure, response, spider): + errors.append((failure, response)) + + settings = {"SCHEDULER": MemoryScheduler} + crawler = get_crawler(TestSpider, settings_dict=settings) + crawler.signals.connect(track_url, signals.request_reached_downloader) + crawler.signals.connect(track_error, signals.spider_error) + + caplog.clear() + with caplog.at_level(ERROR): + await crawler.crawl_async() + + # The requests yielded before the exception are still crawled. + assert actual_urls == ["data:,a"] + assert len(caplog.records) == 1 + assert len(errors) == 1 + failure, response = errors[0] + assert isinstance(failure.value, ValueError) + assert response is None + assert crawler.stats + assert crawler.stats.get_value("finish_reason") == "start_error" + assert crawler.stats.get_value("spider_exceptions/count") == 1 + assert crawler.stats.get_value("spider_exceptions/ValueError") == 1 + + @coroutine_test + async def test_close_spider_from_start( + self, caplog: pytest.LogCaptureFixture + ) -> None: + class TestSpider(Spider): + name = "test" + + async def start(self): + yield Request("data:,a") + raise CloseSpider("my_reason") + + def parse(self, response): + pass + + settings = {"SCHEDULER": MemoryScheduler} + crawler = get_crawler(TestSpider, settings_dict=settings) + + caplog.clear() + with caplog.at_level(ERROR): + await crawler.crawl_async() + + assert not caplog.records + assert crawler.stats + assert crawler.stats.get_value("finish_reason") == "my_reason" + assert crawler.stats.get_value("spider_exceptions/count") is None + class TestRequestSendOrder: seconds = 0.1 # increase if flaky