diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 097a374bf..dd9b3c376 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -3,13 +3,14 @@ Spider Middleware manager See documentation in docs/topics/spider-middleware.rst """ -from itertools import chain, islice +from itertools import islice from twisted.python.failure import Failure + from scrapy.exceptions import _InvalidOutput from scrapy.middleware import MiddlewareManager -from scrapy.utils.defer import mustbe_deferred from scrapy.utils.conf import build_component_list +from scrapy.utils.defer import mustbe_deferred from scrapy.utils.python import MutableChain @@ -17,6 +18,13 @@ def _isiterable(possible_iterator): return hasattr(possible_iterator, '__iter__') +def _fname(f): + return "%s.%s".format( + f.__self__.__class__.__name__, + f.__func__.__name__ + ) + + class SpiderMiddlewareManager(MiddlewareManager): component_name = 'spider middleware' @@ -31,27 +39,36 @@ class SpiderMiddlewareManager(MiddlewareManager): self.methods['process_spider_input'].append(mw.process_spider_input) if hasattr(mw, 'process_start_requests'): self.methods['process_start_requests'].appendleft(mw.process_start_requests) - self.methods['process_spider_output'].appendleft(getattr(mw, 'process_spider_output', None)) - self.methods['process_spider_exception'].appendleft(getattr(mw, 'process_spider_exception', None)) + process_spider_output = getattr(mw, 'process_spider_output', None) + self.methods['process_spider_output'].appendleft(process_spider_output) + process_spider_exception = getattr(mw, 'process_spider_exception', None) + self.methods['process_spider_exception'].appendleft(process_spider_exception) def scrape_response(self, scrape_func, response, request, spider): - fname = lambda f: '%s.%s' % ( - f.__self__.__class__.__name__, - f.__func__.__name__) def process_spider_input(response): for method in self.methods['process_spider_input']: 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))) + msg = "Middleware {} must return None or raise an exception, got {}" + raise _InvalidOutput(msg.format(_fname(method), type(result))) except _InvalidOutput: raise except Exception: return scrape_func(Failure(), request, spider) return scrape_func(response, request, spider) + def _evaluate_iterable(iterable, exception_processor_index, recover_to): + try: + for r in iterable: + yield r + except Exception as ex: + exception_result = process_spider_exception(Failure(ex), exception_processor_index) + if isinstance(exception_result, Failure): + raise + recover_to.extend(exception_result) + def process_spider_exception(_failure, start_index=0): exception = _failure.value # don't handle _InvalidOutput exception @@ -69,8 +86,8 @@ class SpiderMiddlewareManager(MiddlewareManager): elif result is None: continue else: - raise _InvalidOutput('Middleware {} must return None or an iterable, got {}' - .format(fname(method), type(result))) + msg = "Middleware {} must return None or an iterable, got {}" + raise _InvalidOutput(msg.format(_fname(method), type(result))) return _failure def process_spider_output(result, start_index=0): @@ -78,22 +95,12 @@ class SpiderMiddlewareManager(MiddlewareManager): # chain, they went through it already from the process_spider_exception method recovered = MutableChain() - def evaluate_iterable(iterable, index): - try: - for r in iterable: - yield r - except Exception as ex: - exception_result = process_spider_exception(Failure(ex), index+1) - if isinstance(exception_result, Failure): - raise - recovered.extend(exception_result) - method_list = islice(self.methods['process_spider_output'], start_index, None) for method_index, method in enumerate(method_list, start=start_index): if method is None: continue - # the following might fail directly if the output value is not a generator try: + # might fail directly if the output value is not a generator result = method(response=response, result=result, spider=spider) except Exception as ex: exception_result = process_spider_exception(Failure(ex), method_index+1) @@ -101,15 +108,20 @@ class SpiderMiddlewareManager(MiddlewareManager): raise return exception_result if _isiterable(result): - result = evaluate_iterable(result, method_index) + result = _evaluate_iterable(result, method_index+1, recovered) else: - raise _InvalidOutput('Middleware {} must return an iterable, got {}' - .format(fname(method), type(result))) + msg = "Middleware {} must return an iterable, got {}" + raise _InvalidOutput(msg.format(_fname(method), type(result))) - return chain(result, recovered) + return MutableChain(result, recovered) + + def process_callback_output(result): + recovered = MutableChain() + result = _evaluate_iterable(result, 0, recovered) + return MutableChain(process_spider_output(result), recovered) dfd = mustbe_deferred(process_spider_input, response) - dfd.addCallbacks(callback=process_spider_output, errback=process_spider_exception) + dfd.addCallbacks(callback=process_callback_output, errback=process_spider_exception) return dfd def process_start_requests(self, start_requests, spider): diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 8d829c5a5..e5582cc18 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -1,15 +1,15 @@ """ This module contains essential stuff that should've come with Python itself ;) """ +import errno import gc +import inspect import os import re -import inspect +import sys import weakref -import errno from functools import partial, wraps from itertools import chain -import sys from scrapy.utils.decorators import deprecated @@ -371,10 +371,11 @@ else: gc.collect() -class MutableChain(object): +class MutableChain: """ Thin wrapper around itertools.chain, allowing to add iterables "in-place" """ + def __init__(self, *args): self.data = chain(*args) diff --git a/tests/test_spidermiddleware_output_chain.py b/tests/test_spidermiddleware_output_chain.py index 739cf1c2d..b19a74609 100644 --- a/tests/test_spidermiddleware_output_chain.py +++ b/tests/test_spidermiddleware_output_chain.py @@ -1,10 +1,10 @@ - from testfixtures import LogCapture -from twisted.trial.unittest import TestCase from twisted.internet import defer +from twisted.trial.unittest import TestCase -from scrapy import Spider, Request +from scrapy import Request, Spider from scrapy.utils.test import get_crawler + from tests.mockserver import MockServer @@ -74,7 +74,7 @@ class ProcessSpiderInputSpiderWithErrback(ProcessSpiderInputSpiderWithoutErrback name = 'ProcessSpiderInputSpiderWithErrback' def start_requests(self): - yield Request(url=self.mockserver.url('/status?n=200'), callback=self.parse, errback=self.errback) + yield Request(self.mockserver.url('/status?n=200'), self.parse, errback=self.errback) def errback(self, failure): self.logger.info('Got a Failure on the Request errback') @@ -100,6 +100,17 @@ class GeneratorCallbackSpider(Spider): raise ImportError() +# ================================================================================ +# (2.1) exceptions from a spider callback (generator, middleware right after callback) +class GeneratorCallbackSpiderMiddlewareRightAfterSpider(GeneratorCallbackSpider): + name = 'GeneratorCallbackSpiderMiddlewareRightAfterSpider' + custom_settings = { + 'SPIDER_MIDDLEWARES': { + __name__ + '.LogExceptionMiddleware': 100000, + }, + } + + # ================================================================================ # (3) exceptions from a spider callback (not a generator) class NotGeneratorCallbackSpider(Spider): @@ -117,6 +128,17 @@ class NotGeneratorCallbackSpider(Spider): return [{'test': 1}, {'test': 1/0}] +# ================================================================================ +# (3.1) exceptions from a spider callback (not a generator, middleware right after callback) +class NotGeneratorCallbackSpiderMiddlewareRightAfterSpider(NotGeneratorCallbackSpider): + name = 'NotGeneratorCallbackSpiderMiddlewareRightAfterSpider' + custom_settings = { + 'SPIDER_MIDDLEWARES': { + __name__ + '.LogExceptionMiddleware': 100000, + }, + } + + # ================================================================================ # (4) exceptions from a middleware process_spider_output method (generator) class GeneratorOutputChainSpider(Spider): @@ -320,6 +342,16 @@ class TestSpiderMiddleware(TestCase): self.assertIn("Middleware: ImportError exception caught", str(log2)) self.assertIn("'item_scraped_count': 2", str(log2)) + @defer.inlineCallbacks + def test_generator_callback_right_after_callback(self): + """ + (2.1) Special case of (2): Exceptions should be caught + even if the middleware is placed right after the spider + """ + log21 = yield self.crawl_log(GeneratorCallbackSpiderMiddlewareRightAfterSpider) + self.assertIn("Middleware: ImportError exception caught", str(log21)) + self.assertIn("'item_scraped_count': 2", str(log21)) + @defer.inlineCallbacks def test_not_a_generator_callback(self): """ @@ -330,6 +362,16 @@ class TestSpiderMiddleware(TestCase): self.assertIn("Middleware: ZeroDivisionError exception caught", str(log3)) self.assertNotIn("item_scraped_count", str(log3)) + @defer.inlineCallbacks + def test_not_a_generator_callback_right_after_callback(self): + """ + (3.1) Special case of (3): Exceptions should be caught + even if the middleware is placed right after the spider + """ + log31 = yield self.crawl_log(NotGeneratorCallbackSpiderMiddlewareRightAfterSpider) + self.assertIn("Middleware: ZeroDivisionError exception caught", str(log31)) + self.assertNotIn("item_scraped_count", str(log31)) + @defer.inlineCallbacks def test_generator_output_chain(self): """