From 40449fa0eb707bac1ae2b78f0f812372e90f17b7 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 3 Aug 2018 18:20:25 -0300 Subject: [PATCH] Update docs, add tests, remove FIXME comment --- docs/topics/spider-middleware.rst | 3 +- scrapy/core/scraper.py | 1 - tests/test_spidermiddleware.py | 4 +- tests/test_spidermiddleware_output_chain.py | 43 ++++++++++++++------- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 915833c54..7db623cf4 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -78,7 +78,8 @@ following methods: If it raises an exception, Scrapy won't bother calling any other spider middleware :meth:`process_spider_input` and will call the request - errback. The output of the errback is chained back in the other + errback if there is one, otherwise it will start the :meth:`process_spider_exception` + chain. The output of the errback is chained back in the other direction for :meth:`process_spider_output` to process it, or :meth:`process_spider_exception` if it raised an exception. diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index ee1e95a0c..d7fe721fb 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -135,7 +135,6 @@ class Scraper(object): return self.spidermw.scrape_response( self.call_spider, request_result, request, spider) else: - # FIXME: don't ignore errors in spider middleware dfd = self.call_spider(request_result, request, spider) return dfd.addErrback( self._log_download_errors, request_result, request, spider) diff --git a/tests/test_spidermiddleware.py b/tests/test_spidermiddleware.py index 54756f2ff..832fd3330 100644 --- a/tests/test_spidermiddleware.py +++ b/tests/test_spidermiddleware.py @@ -87,7 +87,7 @@ class ProcessSpiderExceptionReRaise(SpiderMiddlewareTestCase): def test_process_spider_exception_return_none(self): - class ProcessSpiderOutputExceptionReturnNoneMiddleware: + class ProcessSpiderExceptionReturnNoneMiddleware: def process_spider_exception(self, response, exception, spider): return None @@ -95,7 +95,7 @@ class ProcessSpiderExceptionReRaise(SpiderMiddlewareTestCase): def process_spider_output(self, response, result, spider): 1/0 - self.mwman._add_middleware(ProcessSpiderOutputExceptionReturnNoneMiddleware()) + self.mwman._add_middleware(ProcessSpiderExceptionReturnNoneMiddleware()) self.mwman._add_middleware(RaiseExceptionProcessSpiderOutputMiddleware()) result = self._scrape_response() self.assertIsInstance(result, Failure) diff --git a/tests/test_spidermiddleware_output_chain.py b/tests/test_spidermiddleware_output_chain.py index 0f5646a72..6f8727a15 100644 --- a/tests/test_spidermiddleware_output_chain.py +++ b/tests/test_spidermiddleware_output_chain.py @@ -45,8 +45,13 @@ class RecoveryMiddleware: # ================================================================================ # (1) exceptions from a spider middleware's process_spider_input method -class ProcessSpiderInputSpider(Spider): - name = 'ProcessSpiderInputSpider' +class FailProcessSpiderInputMiddleware: + def process_spider_input(self, response, spider): + spider.logger.info('Middleware: will raise IndexError') + raise IndexError() + +class ProcessSpiderInputSpiderWithoutErrback(Spider): + name = 'ProcessSpiderInputSpiderWithoutErrback' custom_settings = { 'SPIDER_MIDDLEWARES': { # spider @@ -58,23 +63,23 @@ class ProcessSpiderInputSpider(Spider): } def start_requests(self): - yield Request(url=self.mockserver.url('/status?n=200'), - callback=self.parse, errback=self.errback) + yield Request(url=self.mockserver.url('/status?n=200'), callback=self.parse) def parse(self, response): return {'from': 'callback'} + +class ProcessSpiderInputSpiderWithErrback(ProcessSpiderInputSpiderWithoutErrback): + name = 'ProcessSpiderInputSpiderWithErrback' + + def start_requests(self): + yield Request(url=self.mockserver.url('/status?n=200'), callback=self.parse, errback=self.errback) + def errback(self, failure): self.logger.info('Got a Failure on the Request errback') return {'from': 'errback'} -class FailProcessSpiderInputMiddleware: - def process_spider_input(self, response, spider): - spider.logger.info('Middleware: will raise IndexError') - raise IndexError() - - # ================================================================================ # (2) exceptions from a spider callback (generator) class GeneratorCallbackSpider(Spider): @@ -278,12 +283,22 @@ class TestSpiderMiddleware(TestCase): self.assertIn("'item_scraped_count': 3", str(log)) @defer.inlineCallbacks - def test_process_spider_input_errback(self): + def test_process_spider_input_without_errback(self): """ - (1) An exception from the process_spider_input chain should not be caught by the - process_spider_exception chain, it should go directly to the Request errback + (1.1) An exception from the process_spider_input chain should be caught by the + process_spider_exception chain from the start if the Request has no errback """ - log1 = yield self.crawl_log(ProcessSpiderInputSpider) + log1 = yield self.crawl_log(ProcessSpiderInputSpiderWithoutErrback) + self.assertIn("Middleware: will raise IndexError", str(log1)) + self.assertIn("Middleware: IndexError exception caught", str(log1)) + + @defer.inlineCallbacks + def test_process_spider_input_with_errback(self): + """ + (1.2) An exception from the process_spider_input chain should not be caught by the + process_spider_exception chain if the Request has an errback + """ + log1 = yield self.crawl_log(ProcessSpiderInputSpiderWithErrback) self.assertNotIn("Middleware: IndexError exception caught", str(log1)) self.assertIn("Middleware: will raise IndexError", str(log1)) self.assertIn("Got a Failure on the Request errback", str(log1))