Fix bad exception handling, add tests

scrapy/core/spidermw.py

@@ -66,6 +66,8 @@ class SpiderMiddlewareManager(MiddlewareManager):
                     if result is not None:
                         raise _InvalidOutput('Middleware {} must return None or raise an exception, got {}' \
                                              .format(fname(method), type(result)))
+                except _InvalidOutput:
+                    raise
                 except:
                     return scrape_func(Failure(), request, spider)
             return scrape_func(response, request, spider)

tests/test_spidermiddleware_invalid_values.py

@@ -0,0 +1,82 @@
+from twisted.trial.unittest import TestCase
+from twisted.python.failure import Failure
+
+from scrapy.spiders import Spider
+from scrapy.http import Request, Response
+from scrapy.exceptions import _InvalidOutput
+from scrapy.utils.test import get_crawler
+from scrapy.core.spidermw import SpiderMiddlewareManager
+from tests import mock
+
+
+class SpiderMiddlewareTestCase(TestCase):
+
+    def setUp(self):
+        self.request = Request('http://example.com/index.html')
+        self.response = Response(self.request.url, request=self.request)
+        self.crawler = get_crawler(Spider)
+        self.spider = self.crawler._create_spider('foo')
+        self.mwman = SpiderMiddlewareManager.from_crawler(self.crawler)
+
+    def _scrape_response(self):
+        """Execute spider mw manager's scrape_response method and return the result.
+        Raise exception in case of failure.
+        """
+        scrape_func = mock.MagicMock()
+        dfd = self.mwman.scrape_response(scrape_func, self.response, self.request, self.spider)
+        # catch deferred result and return the value
+        results = []
+        dfd.addBoth(results.append)
+        self._wait(dfd)
+        ret = results[0]
+        return ret
+
+
+class ProcessSpiderInputInvalidOutput(SpiderMiddlewareTestCase):
+    """Invalid return value for process_spider_input method"""
+
+    def test_invalid_process_spider_input(self):
+
+        class InvalidProcessSpiderInputMiddleware:
+            def process_spider_input(self, response, spider):
+                return 1
+
+        self.mwman._add_middleware(InvalidProcessSpiderInputMiddleware())
+        result = self._scrape_response()
+        self.assertIsInstance(result, Failure)
+        self.assertIsInstance(result.value, _InvalidOutput)
+
+
+class ProcessSpiderOutputInvalidOutput(SpiderMiddlewareTestCase):
+    """Invalid return value for process_spider_output method"""
+
+    def test_invalid_process_spider_output(self):
+
+        class InvalidProcessSpiderOutputMiddleware:
+            def process_spider_output(self, response, result, spider):
+                return 1
+
+        self.mwman._add_middleware(InvalidProcessSpiderOutputMiddleware())
+        result = self._scrape_response()
+        self.assertIsInstance(result, Failure)
+        self.assertIsInstance(result.value, _InvalidOutput)
+
+
+class ProcessSpiderExceptionInvalidOutput(SpiderMiddlewareTestCase):
+    """Invalid return value for process_spider_exception method"""
+
+    def test_invalid_process_spider_exception(self):
+
+        class InvalidProcessSpiderOutputExceptionMiddleware:
+            def process_spider_exception(self, response, exception, spider):
+                return 1
+
+        class RaiseExceptionProcessSpiderOutputMiddleware:
+            def process_spider_output(self, response, result, spider):
+                raise Exception()
+
+        self.mwman._add_middleware(InvalidProcessSpiderOutputExceptionMiddleware())
+        self.mwman._add_middleware(RaiseExceptionProcessSpiderOutputMiddleware())
+        result = self._scrape_response()
+        self.assertIsInstance(result, Failure)
+        self.assertIsInstance(result.value, _InvalidOutput)