More tests

scrapy/core/spidermw.py

@@ -64,8 +64,8 @@ class SpiderMiddlewareManager(MiddlewareManager):
                 try:
                     result = method(response=response, spider=spider)
                     if result is not None:
-                        raise _InvalidOutput('Middleware {} must return None or raise ' \
-                            'an exception, got {}'.format(fname(method), type(result)))
+                        raise _InvalidOutput('Middleware {} must return None or raise an exception, got {}' \
+                                             .format(fname(method), type(result)))
                 except:
                     return scrape_func(Failure(), request, spider)
             return scrape_func(response, request, spider)
@@ -78,8 +78,8 @@ class SpiderMiddlewareManager(MiddlewareManager):
             for method in self.methods['process_spider_exception'][index:]:
                 if method is None:
                     continue
-                result = method(response=response, exception=exception, spider=spider)
                 index += 1
+                result = method(response=response, exception=exception, spider=spider)
                 if _isiterable(result):
                     # stop exception handling by handing control over to the
                     # process_spider_output chain if an iterable has been returned
@@ -96,9 +96,9 @@ class SpiderMiddlewareManager(MiddlewareManager):
             # chain, they went through it already from the process_spider_exception method
             recovered = MutableChain()
 
-            def evaluate_result(result_iterable, index):
+            def evaluate_iterable(iterable, index):
                 try:
-                    for r in result_iterable:
+                    for r in iterable:
                         yield r
                 except Exception as ex:
                     exception_result = process_spider_exception(Failure(ex), index+1)
@@ -109,10 +109,17 @@ class SpiderMiddlewareManager(MiddlewareManager):
             for method in self.methods['process_spider_output'][index:]:
                 if method is None:
                     continue
-                result = method(response=response, result=result, spider=spider)
                 index += 1
+                # the following might fail directly if the output value is not a generator
+                try:
+                    result = method(response=response, result=result, spider=spider)
+                except Exception as ex:
+                    exception_result = process_spider_exception(Failure(ex), index+1)
+                    if exception_result is None or isinstance(exception_result, Failure):
+                        raise
+                    return exception_result
                 if _isiterable(result):
-                    result = evaluate_result(result, index)
+                    result = evaluate_iterable(result, index)
                 else:
                     raise _InvalidOutput('Middleware {} must return an iterable, got {}' \
                                          .format(fname(method), type(result)))

tests/test_spidermiddleware.py

@@ -98,8 +98,8 @@ class GeneratorCallbackSpider(Spider):
 
 # ================================================================================
 # (3) exceptions from a spider callback (not a generator)
-class NotAGeneratorCallbackSpider(Spider):
-    name = 'NotAGeneratorCallbackSpider'
+class NotGeneratorCallbackSpider(Spider):
+    name = 'NotGeneratorCallbackSpider'
     custom_settings = {
         'SPIDER_MIDDLEWARES': {
             __name__ + '.LogExceptionMiddleware': 10,
@@ -178,6 +178,76 @@ class GeneratorDoNothingAfterRecoveryMiddleware(_GeneratorDoNothingMiddleware):
     pass
 
 
+# ================================================================================
+# (5) exceptions from a middleware process_spider_output method (not generator)
+class NotGeneratorOutputChainSpider(Spider):
+    name = 'NotGeneratorOutputChainSpider'
+    custom_settings = {
+        'SPIDER_MIDDLEWARES': {
+            __name__ + '.NotGeneratorFailMiddleware': 10,
+            __name__ + '.NotGeneratorDoNothingAfterFailureMiddleware': 8,
+            __name__ + '.NotGeneratorRecoverMiddleware': 5,
+            __name__ + '.NotGeneratorDoNothingAfterRecoveryMiddleware': 3,
+        },
+    }
+
+    def start_requests(self):
+        return [Request(self.mockserver.url('/status?n=200'))]
+
+    def parse(self, response):
+        return [{'processed': ['parse-first-item']}, {'processed': ['parse-second-item']}]
+
+
+class _NotGeneratorDoNothingMiddleware:
+    def process_spider_output(self, response, result, spider):
+        out = []
+        for r in result:
+            r['processed'].append('{}.process_spider_output'.format(self.__class__.__name__))
+            out.append(r)
+        return out
+
+    def process_spider_exception(self, response, exception, spider):
+        method = '{}.process_spider_exception'.format(self.__class__.__name__)
+        logging.info('%s: %s caught', method, exception.__class__.__name__)
+        return None
+
+
+class NotGeneratorFailMiddleware:
+    def process_spider_output(self, response, result, spider):
+        out = []
+        for r in result:
+            r['processed'].append('{}.process_spider_output'.format(self.__class__.__name__))
+            out.append(r)
+        raise ReferenceError()
+        return out
+
+    def process_spider_exception(self, response, exception, spider):
+        method = '{}.process_spider_exception'.format(self.__class__.__name__)
+        logging.info('%s: %s caught', method, exception.__class__.__name__)
+        return [{'processed': [method]}]
+
+
+class NotGeneratorDoNothingAfterFailureMiddleware(_NotGeneratorDoNothingMiddleware):
+    pass
+
+
+class NotGeneratorRecoverMiddleware:
+    def process_spider_output(self, response, result, spider):
+        out = []
+        for r in result:
+            r['processed'].append('{}.process_spider_output'.format(self.__class__.__name__))
+            out.append(r)
+        return out
+
+    def process_spider_exception(self, response, exception, spider):
+        method = '{}.process_spider_exception'.format(self.__class__.__name__)
+        logging.info('%s: %s caught', method, exception.__class__.__name__)
+        return [{'processed': [method]}]
+
+class NotGeneratorDoNothingAfterRecoveryMiddleware(_NotGeneratorDoNothingMiddleware):
+    pass
+
+
 # ================================================================================
 class TestSpiderMiddleware(TestCase):
     @classmethod
@@ -240,7 +310,7 @@ class TestSpiderMiddleware(TestCase):
         (3) An exception from a spider callback (returning a list) should
         be caught by the process_spider_exception chain. No items should be processed.
         """
-        log3 = yield self.crawl_log(NotAGeneratorCallbackSpider)
+        log3 = yield self.crawl_log(NotGeneratorCallbackSpider)
         self.assertIn("Middleware: ZeroDivisionError exception caught", str(log3))
         self.assertNotIn("item_scraped_count", str(log3))
 
@@ -272,3 +342,26 @@ class TestSpiderMiddleware(TestCase):
         self.assertIn(str(item_from_callback), str(log4))
         self.assertIn(str(item_recovered), str(log4))
         self.assertNotIn('parse-second-item', str(log4))
+
+    @defer.inlineCallbacks
+    def test_not_a_generator_output_chain(self):
+        """
+        (5) An exception from a middleware's process_spider_output method should be sent
+        to the process_spider_exception method from the next middleware in the chain.
+        The result of the recovery by the process_spider_exception method should be handled
+        by the process_spider_output method from the next middleware.
+        The final item count should be 1 (from the process_spider_exception chain, the items
+        from the spider callback are lost)
+        """
+        log5 = yield self.crawl_log(NotGeneratorOutputChainSpider)
+        self.assertIn("'item_scraped_count': 1", str(log5))
+        self.assertIn("GeneratorRecoverMiddleware.process_spider_exception: ReferenceError caught", str(log5))
+        self.assertIn("GeneratorDoNothingAfterFailureMiddleware.process_spider_exception: ReferenceError caught", str(log5))
+        self.assertNotIn("GeneratorFailMiddleware.process_spider_exception: ReferenceError caught", str(log5))
+        self.assertNotIn("GeneratorDoNothingAfterRecoveryMiddleware.process_spider_exception: ReferenceError caught", str(log5))
+        item_recovered = {'processed': [
+            'NotGeneratorRecoverMiddleware.process_spider_exception',
+            'NotGeneratorDoNothingAfterRecoveryMiddleware.process_spider_output']}
+        self.assertIn(str(item_recovered), str(log5))
+        self.assertNotIn('parse-first-item', str(log5))
+        self.assertNotIn('parse-second-item', str(log5))