From a3af0bfd56770aab0a056ae6e29efffa8b7d88c4 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 18 Jul 2018 15:15:55 -0300 Subject: [PATCH] More tests --- scrapy/core/spidermw.py | 21 +++++--- tests/test_spidermiddleware.py | 99 ++++++++++++++++++++++++++++++++-- 2 files changed, 110 insertions(+), 10 deletions(-) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index da51bc974..96488806d 100644 --- a/scrapy/core/spidermw.py +++ b/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))) diff --git a/tests/test_spidermiddleware.py b/tests/test_spidermiddleware.py index 9bb7f62fd..2f431ddc7 100644 --- a/tests/test_spidermiddleware.py +++ b/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))