From c8bbe4d968c3affbbbb634ee0209ae27840fc846 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io>
Date: Sat, 15 Mar 2025 12:29:37 +0100
Subject: [PATCH] Log a traceback and assume a None return value if
 Scheduler.next_request raises an exception

---
 scrapy/core/engine.py     | 10 +++++++++-
 tests/test_engine_loop.py | 33 +++++++++++++++++++++++++++++++++
 tests/test_scheduler.py   |  2 +-
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py
index d27897316..6eaf6e9f6 100644
--- a/scrapy/core/engine.py
+++ b/scrapy/core/engine.py
@@ -22,6 +22,7 @@ from scrapy.http import Request, Response
 from scrapy.utils.defer import deferred_from_coro
 from scrapy.utils.log import failure_to_exc_info, logformatter_adapter
 from scrapy.utils.misc import build_from_crawler, load_object
+from scrapy.utils.python import global_object_name
 from scrapy.utils.reactor import CallLaterOnce
 
 from ._seeding import SeedingPolicy
@@ -312,7 +313,14 @@ class ExecutionEngine:
         assert self._slot is not None  # typing
         assert self.spider is not None  # typing
 
-        request = self._slot.scheduler.next_request()
+        try:
+            request = self._slot.scheduler.next_request()
+        except Exception as exception:
+            exception_traceback = format_exc()
+            logger.exception(
+                f"{global_object_name(self._slot.scheduler.next_request)} raised an exception: {exception}\n{exception_traceback}"
+            )
+            return None
         if request is None:
             return None
 
diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py
index db59fdfea..7fb6298be 100644
--- a/tests/test_engine_loop.py
+++ b/tests/test_engine_loop.py
@@ -309,6 +309,39 @@ class MainTestCase(TestCase):
         expected_urls = ["data:,a", "data:,b", "data:,c"]
         assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}"
 
+    @deferred_f_from_coro_f
+    async def test_scheduler_next_request_exception(self):
+        class TestScheduler(MemoryScheduler):
+            queue = ["data:,b", RuntimeError(), "data:,a"]
+
+            def next_request(self):
+                request = super().next_request()
+                if isinstance(request, Exception):
+                    raise request
+                return request
+
+        class TestSpider(Spider):
+            name = "test"
+            start_urls = ["data:,c"]
+
+            def parse(self, response):
+                pass
+
+        actual_urls = []
+
+        def track_url(request, spider):
+            actual_urls.append(request.url)
+
+        settings = {"SCHEDULER": TestScheduler, "SEEDING_POLICY": "lazy"}
+        crawler = get_crawler(TestSpider, settings_dict=settings)
+        crawler.signals.connect(track_url, signals.request_reached_downloader)
+        with LogCapture() as log:
+            await maybe_deferred_to_future(crawler.crawl())
+        assert crawler.stats.get_value("finish_reason") == "finished"
+        expected_urls = ["data:,a", "data:,b", "data:,c"]
+        assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}"
+        assert "in next_request\n    raise request" in str(log), log
+
 
 class MockServerTestCase(TestCase):
     # If requests are too fast, test_idle will fail because the outcome will
diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py
index 559bd7571..afd4ecc80 100644
--- a/tests/test_scheduler.py
+++ b/tests/test_scheduler.py
@@ -27,7 +27,7 @@ class MemoryScheduler(BaseScheduler):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.queue = deque(
-            value if isinstance(value, Request) else Request(value)
+            Request(value) if isinstance(value, str) else value
             for value in getattr(self, "queue", [])
         )