Add errback parameter to scrapy.spiders.crawl.Rule (#4000)

* Add errback parameter to scrapy.spiders.crawl.Rule

* CrawlSpider: optimize by reducing iterations

* [test] Rule.errback

* [doc] Rule.errback

* [doc] Use autoclass in docs/topics/spiders.rst

Co-Authored-By: Adrián Chaves <adrian@chaves.io>

* Rule.process_links takes a list

* Fix aesthetic issue reported by Flake8

docs/topics/spiders.rst

@@ -414,6 +414,12 @@ Crawling rules
    from which the request originated as second argument. It must return a
    ``Request`` object or ``None`` (to filter out the request).
 
+   ``errback`` is a callable or a string (in which case a method from the spider
+   object with that name will be used) to be called if any exception is
+   raised while processing a request generated by the rule.
+   It receives a :class:`Twisted Failure <twisted.python.failure.Failure>`
+   instance as first parameter.
+
 CrawlSpider example
 ~~~~~~~~~~~~~~~~~~~
 

scrapy/spiders/crawl.py

@@ -16,7 +16,11 @@ from scrapy.utils.python import get_func_args
 from scrapy.utils.spider import iterate_spider_output
 
 
-def _identity(request, response):
+def _identity(x):
+    return x
+
+
+def _identity_process_request(request, response):
     return request
 
 
@@ -32,17 +36,20 @@ _default_link_extractor = LinkExtractor()
 
 class Rule(object):
 
-    def __init__(self, link_extractor=None, callback=None, cb_kwargs=None, follow=None, process_links=None, process_request=None):
+    def __init__(self, link_extractor=None, callback=None, cb_kwargs=None, follow=None,
+                 process_links=None, process_request=None, errback=None):
         self.link_extractor = link_extractor or _default_link_extractor
         self.callback = callback
+        self.errback = errback
         self.cb_kwargs = cb_kwargs or {}
-        self.process_links = process_links
-        self.process_request = process_request or _identity
+        self.process_links = process_links or _identity
+        self.process_request = process_request or _identity_process_request
         self.process_request_argcount = None
         self.follow = follow if follow is not None else not callback
 
     def _compile(self, spider):
         self.callback = _get_method(self.callback, spider)
+        self.errback = _get_method(self.errback, spider)
         self.process_links = _get_method(self.process_links, spider)
         self.process_request = _get_method(self.process_request, spider)
         self.process_request_argcount = len(get_func_args(self.process_request))
@@ -76,48 +83,59 @@ class CrawlSpider(Spider):
     def process_results(self, response, results):
         return results
 
-    def _build_request(self, rule, link):
-        r = Request(url=link.url, callback=self._response_downloaded)
-        r.meta.update(rule=rule, link_text=link.text)
-        return r
+    def _build_request(self, rule_index, link):
+        return Request(
+            url=link.url,
+            callback=self._callback,
+            errback=self._errback,
+            meta=dict(rule=rule_index, link_text=link.text),
+        )
 
     def _requests_to_follow(self, response):
         if not isinstance(response, HtmlResponse):
             return
         seen = set()
-        for n, rule in enumerate(self._rules):
+        for rule_index, rule in enumerate(self._rules):
             links = [lnk for lnk in rule.link_extractor.extract_links(response)
                      if lnk not in seen]
-            if links and rule.process_links:
-                links = rule.process_links(links)
-            for link in links:
+            for link in rule.process_links(links):
                 seen.add(link)
-                request = self._build_request(n, link)
+                request = self._build_request(rule_index, link)
                 yield rule._process_request(request, response)
 
-    def _response_downloaded(self, response):
+    def _callback(self, response):
         rule = self._rules[response.meta['rule']]
         return self._parse_response(response, rule.callback, rule.cb_kwargs, rule.follow)
 
+    def _errback(self, failure):
+        rule = self._rules[failure.request.meta['rule']]
+        return self._handle_failure(failure, rule.errback)
+
     def _parse_response(self, response, callback, cb_kwargs, follow=True):
         if callback:
             cb_res = callback(response, **cb_kwargs) or ()
             cb_res = self.process_results(response, cb_res)
-            for requests_or_item in iterate_spider_output(cb_res):
-                yield requests_or_item
+            for request_or_item in iterate_spider_output(cb_res):
+                yield request_or_item
 
         if follow and self._follow_links:
             for request_or_item in self._requests_to_follow(response):
                 yield request_or_item
 
+    def _handle_failure(self, failure, errback):
+        if errback:
+            results = errback(failure) or ()
+            for request_or_item in iterate_spider_output(results):
+                yield request_or_item
+
     def _compile_rules(self):
-        self._rules = [copy.copy(r) for r in self.rules]
-        for rule in self._rules:
-            rule._compile(self)
+        self._rules = []
+        for rule in self.rules:
+            self._rules.append(copy.copy(rule))
+            self._rules[-1]._compile(self)
 
     @classmethod
     def from_crawler(cls, crawler, *args, **kwargs):
         spider = super(CrawlSpider, cls).from_crawler(crawler, *args, **kwargs)
-        spider._follow_links = crawler.settings.getbool(
-            'CRAWLSPIDER_FOLLOW_LINKS', True)
+        spider._follow_links = crawler.settings.getbool('CRAWLSPIDER_FOLLOW_LINKS', True)
         return spider

tests/mockserver.py

@@ -164,6 +164,12 @@ class Drop(Partial):
             request.finish()
 
 
+class ArbitraryLengthPayloadResource(LeafResource):
+
+    def render(self, request):
+        return request.content.read()
+
+
 class Root(Resource):
 
     def __init__(self):
@@ -177,6 +183,7 @@ class Root(Resource):
         self.putChild(b"echo", Echo())
         self.putChild(b"payload", PayloadResource())
         self.putChild(b"xpayload", EncodingResourceWrapper(PayloadResource(), [GzipEncoderFactory()]))
+        self.putChild(b"alpayload", ArbitraryLengthPayloadResource())
         try:
             from tests import tests_datadir
             self.putChild(b"files", File(os.path.join(tests_datadir, 'test_site/files/')))

tests/spiders.py

@@ -1,14 +1,14 @@
 """
 Some spiders used for testing and benchmarking
 """
-
 import time
 from urllib.parse import urlencode
 
-from scrapy.spiders import Spider
 from scrapy.http import Request
 from scrapy.item import Item
 from scrapy.linkextractors import LinkExtractor
+from scrapy.spiders import Spider
+from scrapy.spiders.crawl import CrawlSpider, Rule
 
 
 class MockServerSpider(Spider):
@@ -184,3 +184,35 @@ class DuplicateStartRequestsSpider(MockServerSpider):
 
     def parse(self, response):
         self.visited += 1
+
+
+class CrawlSpiderWithErrback(MockServerSpider, CrawlSpider):
+    name = 'crawl_spider_with_errback'
+    custom_settings = {
+        'RETRY_HTTP_CODES': [],  # no need to retry
+    }
+    rules = (
+        Rule(LinkExtractor(), callback='callback', errback='errback', follow=True),
+    )
+
+    def start_requests(self):
+        test_body = b"""
+        <html>
+            <head><title>Page title<title></head>
+            <body>
+                <p><a href="/status?n=200">Item 200</a></p>  <!-- callback -->
+                <p><a href="/status?n=201">Item 201</a></p>  <!-- callback -->
+                <p><a href="/status?n=404">Item 404</a></p>  <!-- errback -->
+                <p><a href="/status?n=500">Item 500</a></p>  <!-- errback -->
+                <p><a href="/status?n=501">Item 501</a></p>  <!-- errback -->
+            </body>
+        </html>
+        """
+        url = self.mockserver.url("/alpayload")
+        yield Request(url, method="POST", body=test_body)
+
+    def callback(self, response):
+        self.logger.info('[callback] status %i', response.status)
+
+    def errback(self, failure):
+        self.logger.info('[errback] status %i', failure.value.response.status)

tests/test_crawl.py

@@ -5,12 +5,12 @@ from testfixtures import LogCapture
 from twisted.internet import defer
 from twisted.trial.unittest import TestCase
 
-from scrapy.http import Request
 from scrapy.crawler import CrawlerRunner
+from scrapy.http import Request
 from scrapy.utils.python import to_unicode
-from tests.spiders import FollowAllSpider, DelaySpider, SimpleSpider, \
-    BrokenStartRequestsSpider, SingleRequestSpider, DuplicateStartRequestsSpider
 from tests.mockserver import MockServer
+from tests.spiders import (FollowAllSpider, DelaySpider, SimpleSpider, BrokenStartRequestsSpider,
+                           SingleRequestSpider, DuplicateStartRequestsSpider, CrawlSpiderWithErrback)
 
 
 class CrawlTestCase(TestCase):
@@ -277,3 +277,15 @@ with multiples lines
 
         self._assert_retried(log)
         self.assertIn("Got response 200", str(log))
+
+    @defer.inlineCallbacks
+    def test_crawlspider_with_errback(self):
+        self.runner.crawl(CrawlSpiderWithErrback, mockserver=self.mockserver)
+
+        with LogCapture() as log:
+            yield self.runner.join()
+
+        self.assertIn("[callback] status 200", str(log))
+        self.assertIn("[callback] status 201", str(log))
+        self.assertIn("[errback] status 404", str(log))
+        self.assertIn("[errback] status 500", str(log))