Merge 76484fcb5a into e28e56aa61

docs/topics/practices.rst

@@ -56,6 +56,14 @@ Here's an example showing how to run a single spider with it.
     process.crawl(MySpider)
     process.start()  # the script will block here until the crawling is finished
 
+Exceptions that interrupt a crawl, such as one raised by the ``__init__``
+method of your spider or by the ``from_crawler`` method of one of its
+components, are logged as critical errors by these classes, which also set
+:attr:`~scrapy.crawler.AsyncCrawlerProcess.bootstrap_failed` to ``True``. You
+can check that attribute once
+:meth:`~scrapy.crawler.AsyncCrawlerProcess.start` returns, for example to
+choose the exit code of your script.
+
 You can define :ref:`settings <topics-settings>` within the dictionary passed
 to :class:`~scrapy.crawler.AsyncCrawlerProcess`. Make sure to check the
 :class:`~scrapy.crawler.AsyncCrawlerProcess`
@@ -108,6 +116,9 @@ use :func:`twisted.internet.task.react` to start and stop the reactor, though
 it may be easier to just use :class:`~scrapy.crawler.AsyncCrawlerProcess` or
 :class:`~scrapy.crawler.CrawlerProcess` instead.
 
+Exceptions that interrupt a crawl reach the code that awaits that task or that
+adds callbacks to that Deferred, which is where you should handle them.
+
 Here's an example of using :class:`~scrapy.crawler.AsyncCrawlerRunner` together
 with simple reactor management code:
 

scrapy/crawler.py

@@ -438,6 +438,8 @@ class CrawlerRunnerBase(ABC):
         self.spider_loader: SpiderLoaderProtocol = get_spider_loader(settings)
         self._crawlers: set[Crawler] = set()
         self.bootstrap_failed = False
+        """``True`` if any crawl started by :meth:`crawl` was interrupted by an
+        exception, including a failure to create the spider object."""
 
     @property
     def crawlers(self) -> set[Crawler]:
@@ -445,6 +447,11 @@ class CrawlerRunnerBase(ABC):
         :meth:`crawl` and managed by this class."""
         return self._crawlers
 
+    def _handle_crawl_error(self, crawler: Crawler, exc: BaseException) -> bool:
+        # Returns whether exc has been reported and can be dropped instead of
+        # being propagated to the caller of crawl().
+        return False
+
     def create_crawler(
         self, crawler_or_spidercls: type[Spider] | str | Crawler
     ) -> Crawler:
@@ -555,9 +562,10 @@ class CrawlerRunner(CrawlerRunnerBase):
         failed = False
         try:
             yield d
-        except Exception:
+        except Exception as exc:
             failed = True
-            raise
+            if not self._handle_crawl_error(crawler, exc):
+                raise
         finally:
             self.crawlers.discard(crawler)
             self._active.discard(d)
@@ -666,9 +674,10 @@ class AsyncCrawlerRunner(CrawlerRunnerBase):
     ) -> None:
         try:
             await crawler.crawl_async(*args, **kwargs)
-        except Exception:
+        except Exception as exc:
             self.bootstrap_failed = True
-            raise  # re-raise so asyncio still logs it to stderr naturally
+            if not self._handle_crawl_error(crawler, exc):
+                raise
 
     def _done(self, task: asyncio.Task[None], crawler: Crawler) -> None:
         self._active.discard(task)
@@ -723,6 +732,13 @@ class CrawlerProcessBase(CrawlerRunnerBase):
     ) -> None:
         raise NotImplementedError
 
+    def _handle_crawl_error(self, crawler: Crawler, exc: BaseException) -> bool:
+        # These classes run the reactor or the event loop themselves, so the
+        # caller of crawl() is not expected to handle crawl exceptions.
+        name = getattr(crawler.spidercls, "name", None) or crawler.spidercls.__name__
+        logger.critical(f"Error running spider {name}", exc_info=exc)
+        return True
+
     def _signal_shutdown(self, signum: int, _: Any) -> None:
         from twisted.internet import reactor
 

tests/AsyncCrawlerProcess/crawl_error.py

@@ -0,0 +1,18 @@
+import sys
+
+import scrapy
+from scrapy.crawler import AsyncCrawlerProcess
+
+
+class BoomSpider(scrapy.Spider):
+    name = "boom"
+
+    def __init__(self, *args, **kwargs):
+        raise ValueError("boom")
+
+
+process = AsyncCrawlerProcess(settings={})
+
+process.crawl(BoomSpider)
+process.start()
+print(f"bootstrap_failed: {process.bootstrap_failed}", file=sys.stderr)

tests/CrawlerProcess/crawl_error.py

@@ -0,0 +1,18 @@
+import sys
+
+import scrapy
+from scrapy.crawler import CrawlerProcess
+
+
+class BoomSpider(scrapy.Spider):
+    name = "boom"
+
+    def __init__(self, *args, **kwargs):
+        raise ValueError("boom")
+
+
+process = CrawlerProcess(settings={})
+
+process.crawl(BoomSpider)
+process.start()
+print(f"bootstrap_failed: {process.bootstrap_failed}", file=sys.stderr)

tests/test_crawler_subprocess.py

@@ -69,6 +69,17 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin):
         )
         assert "ReactorAlreadyInstalledError" not in log
 
+    def test_crawl_error(self) -> None:
+        log = self.run_script("crawl_error.py")
+        assert log.count("ValueError: boom") == 1
+        assert "Unhandled error in Deferred" not in log
+        assert "Task exception was never retrieved" not in log
+        assert re.search(
+            r"CRITICAL: Error running spider boom.+bootstrap_failed: True",
+            log,
+            re.DOTALL,
+        )
+
     def test_reactor_default(self) -> None:
         log = self.run_script("reactor_default.py")
         assert "Spider closed (finished)" not in log