Repository: scrapy/scrapy
Pull request #6434 (2024-07-09 19:09:20)

Feature/closespider pagecount no item
Recently I had a use case where I needed to ensure that the crawler didn't perform too many requests without getting items in return. Even though `CLOSESPIDER_ERRORCOUNT` and `CLOSESPIDER_TIMEOUT_NO_ITEM` exist, they didn't cover all possible cases.
- `CLOSESPIDER_ERRORCOUNT`: Requests that don't raise errors but don't produce items.
- `CLOSESPIDER_TIMEOUT_NO_ITEM`: Maybe requests are slower or faster than expected, especially when rendering pages or performing complex actions. 

Would it be a good idea to add this as a possible setting? 

The changes add the possibility for a `CLOSESPIDER_PAGECOUNT_NO_ITEM: int` setting. Every `response_received` a counter is incremented and every `item_scraped` it's reset to 0. When the counter reaches or passes the setting value the spider is closed, since it crawled n pages with no items scraped.


Patch:
diff --git a/docs/topics/extensions.rst b/docs/topics/extensions.rst
index f7b2f3799..7b34a19d5 100644
--- a/docs/topics/extensions.rst
+++ b/docs/topics/extensions.rst
@@ -317,6 +317,19 @@ crawls more than that, the spider will be closed with the reason
 ``closespider_pagecount``. If zero (or non set), spiders won't be closed by
 number of crawled responses.
 
+.. setting:: CLOSESPIDER_PAGECOUNT_NO_ITEM
+
+CLOSESPIDER_PAGECOUNT_NO_ITEM
+"""""""""""""""""""""""""""""
+
+Default: ``0``
+
+An integer which specifies the maximum number of consecutive responses to crawl
+without items scraped. If the spider crawls more consecutive responses than that
+and no items are scraped in the meantime, the spider will be closed with the
+reason ``closespider_pagecount_no_item``. If zero (or not set), spiders won't be
+closed by number of crawled responses with no items.
+
 .. setting:: CLOSESPIDER_ERRORCOUNT
 
 CLOSESPIDER_ERRORCOUNT
diff --git a/scrapy/extensions/closespider.py b/scrapy/extensions/closespider.py
index 4627e7f98..cef5527b7 100644
--- a/scrapy/extensions/closespider.py
+++ b/scrapy/extensions/closespider.py
@@ -36,6 +36,9 @@ class CloseSpider:
             "pagecount": crawler.settings.getint("CLOSESPIDER_PAGECOUNT"),
             "errorcount": crawler.settings.getint("CLOSESPIDER_ERRORCOUNT"),
             "timeout_no_item": crawler.settings.getint("CLOSESPIDER_TIMEOUT_NO_ITEM"),
+            "pagecount_no_item": crawler.settings.getint(
+                "CLOSESPIDER_PAGECOUNT_NO_ITEM"
+            ),
         }
 
         if not any(self.close_on.values()):
@@ -45,11 +48,11 @@ class CloseSpider:
 
         if self.close_on.get("errorcount"):
             crawler.signals.connect(self.error_count, signal=signals.spider_error)
-        if self.close_on.get("pagecount"):
+        if self.close_on.get("pagecount") or self.close_on.get("pagecount_no_item"):
             crawler.signals.connect(self.page_count, signal=signals.response_received)
         if self.close_on.get("timeout"):
             crawler.signals.connect(self.spider_opened, signal=signals.spider_opened)
-        if self.close_on.get("itemcount"):
+        if self.close_on.get("itemcount") or self.close_on.get("pagecount_no_item"):
             crawler.signals.connect(self.item_scraped, signal=signals.item_scraped)
         if self.close_on.get("timeout_no_item"):
             self.timeout_no_item: int = self.close_on["timeout_no_item"]
@@ -60,6 +63,7 @@ class CloseSpider:
             crawler.signals.connect(
                 self.item_scraped_no_item, signal=signals.item_scraped
             )
+
         crawler.signals.connect(self.spider_closed, signal=signals.spider_closed)
 
     @classmethod
@@ -74,9 +78,17 @@ class CloseSpider:
 
     def page_count(self, response: Response, request: Request, spider: Spider) -> None:
         self.counter["pagecount"] += 1
+        self.counter["pagecount_since_last_item"] += 1
         if self.counter["pagecount"] == self.close_on["pagecount"]:
             assert self.crawler.engine
             self.crawler.engine.close_spider(spider, "closespider_pagecount")
+            return
+        if self.close_on["pagecount_no_item"] and (
+            self.counter["pagecount_since_last_item"]
+            >= self.close_on["pagecount_no_item"]
+        ):
+            assert self.crawler.engine
+            self.crawler.engine.close_spider(spider, "closespider_pagecount_no_item")
 
     def spider_opened(self, spider: Spider) -> None:
         from twisted.internet import reactor
@@ -91,6 +103,7 @@ class CloseSpider:
 
     def item_scraped(self, item: Any, spider: Spider) -> None:
         self.counter["itemcount"] += 1
+        self.counter["pagecount_since_last_item"] = 0
         if self.counter["itemcount"] == self.close_on["itemcount"]:
             assert self.crawler.engine
             self.crawler.engine.close_spider(spider, "closespider_itemcount")


Test patch:
diff --git a/tests/spiders.py b/tests/spiders.py
index ea419afbd..743811893 100644
--- a/tests/spiders.py
+++ b/tests/spiders.py
@@ -283,6 +283,24 @@ class ItemSpider(FollowAllSpider):
             yield {}
 
 
+class MaxItemsAndRequestsSpider(FollowAllSpider):
+    def __init__(self, max_items=10, max_requests=10, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.max_items = max_items
+        self.max_requests = max_requests
+
+    def parse(self, response):
+        self.items_scraped = 0
+        self.pages_crawled = 1  # account for the start url
+        for request in super().parse(response):
+            if self.pages_crawled < self.max_requests:
+                yield request
+                self.pages_crawled += 1
+            if self.items_scraped < self.max_items:
+                yield Item()
+                self.items_scraped += 1
+
+
 class DefaultError(Exception):
     pass
 
diff --git a/tests/test_closespider.py b/tests/test_closespider.py
index 38ede70e4..9a837350f 100644
--- a/tests/test_closespider.py
+++ b/tests/test_closespider.py
@@ -3,7 +3,13 @@ from twisted.trial.unittest import TestCase
 
 from scrapy.utils.test import get_crawler
 from tests.mockserver import MockServer
-from tests.spiders import ErrorSpider, FollowAllSpider, ItemSpider, SlowSpider
+from tests.spiders import (
+    ErrorSpider,
+    FollowAllSpider,
+    ItemSpider,
+    MaxItemsAndRequestsSpider,
+    SlowSpider,
+)
 
 
 class TestCloseSpider(TestCase):
@@ -34,6 +40,43 @@ class TestCloseSpider(TestCase):
         pagecount = crawler.stats.get_value("response_received_count")
         self.assertTrue(pagecount >= close_on)
 
+    @defer.inlineCallbacks
+    def test_closespider_pagecount_no_item(self):
+        close_on = 5
+        max_items = 5
+        max_requests = close_on + max_items
+        crawler = get_crawler(
+            MaxItemsAndRequestsSpider,
+            {
+                "CLOSESPIDER_PAGECOUNT_NO_ITEM": close_on,
+            },
+        )
+        yield crawler.crawl(
+            max_items=max_items, max_requests=max_requests, mockserver=self.mockserver
+        )
+        reason = crawler.spider.meta["close_reason"]
+        self.assertEqual(reason, "closespider_pagecount_no_item")
+        pagecount = crawler.stats.get_value("response_received_count")
+        itemcount = crawler.stats.get_value("item_scraped_count")
+        self.assertLessEqual(pagecount, close_on + itemcount)
+
+    @defer.inlineCallbacks
+    def test_closespider_pagecount_no_item_with_pagecount(self):
+        close_on_pagecount_no_item = 5
+        close_on_pagecount = 20
+        crawler = get_crawler(
+            FollowAllSpider,
+            {
+                "CLOSESPIDER_PAGECOUNT_NO_ITEM": close_on_pagecount_no_item,
+                "CLOSESPIDER_PAGECOUNT": close_on_pagecount,
+            },
+        )
+        yield crawler.crawl(mockserver=self.mockserver)
+        reason = crawler.spider.meta["close_reason"]
+        self.assertEqual(reason, "closespider_pagecount_no_item")
+        pagecount = crawler.stats.get_value("response_received_count")
+        self.assertLess(pagecount, close_on_pagecount)
+
     @defer.inlineCallbacks
     def test_closespider_errorcount(self):
         close_on = 5