diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index cea5e4564..8d3ea51f3 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -829,7 +829,7 @@ REDIRECT_MAX_TIMES Default: ``20`` The maximum number of redirections that will be followed for a single request. -After this maximum the request's response is returned as is. +After this maximum, the request's response is returned as is. MetaRefreshMiddleware --------------------- diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 83c7b1f19..5a03dcdf7 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -40,14 +40,14 @@ class DownloaderMiddlewareManager(MiddlewareManager): % (method.__self__.__class__.__name__, response.__class__.__name__) ) if response: - defer.returnValue(response) - defer.returnValue((yield download_func(request=request, spider=spider))) + return response + return (yield download_func(request=request, spider=spider)) @defer.inlineCallbacks def process_response(response): assert response is not None, 'Received None in process_response' if isinstance(response, Request): - defer.returnValue(response) + return response for method in self.methods['process_response']: response = yield deferred_from_coro(method(request=request, response=response, spider=spider)) @@ -57,12 +57,12 @@ class DownloaderMiddlewareManager(MiddlewareManager): % (method.__self__.__class__.__name__, type(response)) ) if isinstance(response, Request): - defer.returnValue(response) - defer.returnValue(response) + return response + return response @defer.inlineCallbacks - def process_exception(_failure): - exception = _failure.value + def process_exception(failure): + exception = failure.value for method in self.methods['process_exception']: response = yield deferred_from_coro(method(request=request, exception=exception, spider=spider)) if response is not None and not isinstance(response, (Response, Request)): @@ -71,8 +71,8 @@ class DownloaderMiddlewareManager(MiddlewareManager): % (method.__self__.__class__.__name__, type(response)) ) if response: - defer.returnValue(response) - defer.returnValue(_failure) + return response + return failure deferred = mustbe_deferred(process_request, request) deferred.addErrback(process_exception) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index a6d99fa99..8a0636264 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -1,7 +1,7 @@ import functools import logging from collections import defaultdict -from twisted.internet.defer import Deferred, DeferredList, _DefGen_Return +from twisted.internet.defer import Deferred, DeferredList from twisted.python.failure import Failure from scrapy.settings import Settings @@ -141,24 +141,26 @@ class MediaPipeline: # This code fixes a memory leak by avoiding to keep references to # the Request and Response objects on the Media Pipeline cache. # - # Twisted inline callbacks pass return values using the function - # twisted.internet.defer.returnValue, which encapsulates the return - # value inside a _DefGen_Return base exception. - # - # What happens when the media_downloaded callback raises another + # What happens when the media_downloaded callback raises an # exception, for example a FileException('download-error') when - # the Response status code is not 200 OK, is that it stores the - # _DefGen_Return exception on the FileException context. + # the Response status code is not 200 OK, is that the original + # StopIteration exception (which in turn contains the failed + # Response and by extension, the original Request) gets encapsulated + # within the FileException context. + # + # Originally, Scrapy was using twisted.internet.defer.returnValue + # inside functions decorated with twisted.internet.defer.inlineCallbacks, + # encapsulating the returned Response in a _DefGen_Return exception + # instead of a StopIteration. # # To avoid keeping references to the Response and therefore Request # objects on the Media Pipeline cache, we should wipe the context of - # the exception encapsulated by the Twisted Failure when its a - # _DefGen_Return instance. + # the encapsulated exception when it is a StopIteration instance # # This problem does not occur in Python 2.7 since we don't have # Exception Chaining (https://www.python.org/dev/peps/pep-3134/). context = getattr(result.value, '__context__', None) - if isinstance(context, _DefGen_Return): + if isinstance(context, StopIteration): setattr(result.value, '__context__', None) info.downloading.remove(fp) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index c5589e52f..e02b0b840 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -433,7 +433,7 @@ class FeedExportTest(unittest.TestCase): for file_path in FEEDS.keys(): os.remove(str(file_path)) - defer.returnValue(content) + return content @defer.inlineCallbacks def exported_data(self, items, settings): @@ -448,7 +448,7 @@ class FeedExportTest(unittest.TestCase): yield item data = yield self.run_and_export(TestSpider, settings) - defer.returnValue(data) + return data @defer.inlineCallbacks def exported_no_data(self, settings): @@ -462,7 +462,7 @@ class FeedExportTest(unittest.TestCase): pass data = yield self.run_and_export(TestSpider, settings) - defer.returnValue(data) + return data @defer.inlineCallbacks def assertExportedCsv(self, items, header, rows, settings=None, ordered=True): diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index ee1441227..e6e21601b 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -2,7 +2,7 @@ from testfixtures import LogCapture from twisted.trial import unittest from twisted.python.failure import Failure from twisted.internet import reactor -from twisted.internet.defer import Deferred, inlineCallbacks, returnValue +from twisted.internet.defer import Deferred, inlineCallbacks from scrapy.http import Request, Response from scrapy.settings import Settings @@ -124,9 +124,8 @@ class BaseMediaPipelineTestCase(unittest.TestCase): # Simulate the Media Pipeline behavior to produce a Twisted Failure try: # Simulate a Twisted inline callback returning a Response - # The returnValue method raises an exception encapsulating the value - returnValue(response) - except BaseException as exc: + raise StopIteration(response) + except StopIteration as exc: def_gen_return_exc = exc try: # Simulate the media_downloaded callback raising a FileException @@ -140,7 +139,7 @@ class BaseMediaPipelineTestCase(unittest.TestCase): # The Failure should encapsulate a FileException ... self.assertEqual(failure.value, file_exc) - # ... and it should have the returnValue exception set as its context + # ... and it should have the StopIteration exception set as its context self.assertEqual(failure.value.__context__, def_gen_return_exc) # Let's calculate the request fingerprint and fake some runtime data... @@ -155,7 +154,7 @@ class BaseMediaPipelineTestCase(unittest.TestCase): self.assertEqual(info.downloaded[fp], failure) # ... encapsulating the original FileException ... self.assertEqual(info.downloaded[fp].value, file_exc) - # ... but it should not store the returnValue exception on its context + # ... but it should not store the StopIteration exception on its context context = getattr(info.downloaded[fp].value, '__context__', None) self.assertIsNone(context) diff --git a/tests/test_spidermiddleware_output_chain.py b/tests/test_spidermiddleware_output_chain.py index b26353d6c..ad4d6fb98 100644 --- a/tests/test_spidermiddleware_output_chain.py +++ b/tests/test_spidermiddleware_output_chain.py @@ -292,7 +292,7 @@ class TestSpiderMiddleware(TestCase): crawler = get_crawler(spider) with LogCapture() as log: yield crawler.crawl(mockserver=self.mockserver) - raise defer.returnValue(log) + return log @defer.inlineCallbacks def test_recovery(self):