From 492197e4773798fcb19ba934c03d5b48e0b7c1f2 Mon Sep 17 00:00:00 2001
From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com>
Date: Wed, 27 May 2020 10:45:07 -0300
Subject: [PATCH] StopDownload: store response in the exception instead of the
 failure

---
 docs/topics/exceptions.rst                |  9 ++++++---
 scrapy/core/downloader/handlers/http11.py |  8 ++------
 scrapy/core/scraper.py                    | 10 +---------
 tests/test_crawl.py                       | 10 +++++++---
 4 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/docs/topics/exceptions.rst b/docs/topics/exceptions.rst
index 10b23edc4..583a50ab8 100644
--- a/docs/topics/exceptions.rst
+++ b/docs/topics/exceptions.rst
@@ -91,9 +91,12 @@ indicate that no further bytes should be downloaded for a response.
 The ``fail`` boolean parameter controls which method will handle the resulting
 response:
 
-* If ``fail=True``, the request errback is called. The response object is
-  available as the ``response`` attribute of the received
-  :class:`~twisted.python.failure.Failure` object. This is the default behaviour.
+* If ``fail=True`` (default), the request errback is called. The response object is
+  available as the ``response`` attribute of the ``StopDownload`` exception,
+  which is in turn stored as the ``value`` attribute of the received
+  :class:`~twisted.python.failure.Failure` object. This means that in an errback
+  defined as ``def errback(self, failure)``, the response can be accessed though
+  ``failure.value.response``.
 
 * If ``fail=False``, the request callback is called instead.
 
diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py
index a710761c5..22c9ac520 100644
--- a/scrapy/core/downloader/handlers/http11.py
+++ b/scrapy/core/downloader/handlers/http11.py
@@ -442,12 +442,8 @@ class ScrapyAgent:
             ip_address=result["ip_address"],
         )
         if result.get("failure"):
-            # This failure is not the same object that will reach the errback,
-            # so we need to temporarily store the response in the exception.
-            # It will be moved to the failure in core/scraper.py
-            failure = result["failure"]
-            failure.value.response = response
-            return failure
+            result["failure"].value.response = response
+            return result["failure"]
         return response
 
 
diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py
index c52ada83b..6785e103d 100644
--- a/scrapy/core/scraper.py
+++ b/scrapy/core/scraper.py
@@ -11,7 +11,7 @@ from scrapy.utils.defer import defer_result, defer_succeed, parallel, iter_errba
 from scrapy.utils.spider import iterate_spider_output
 from scrapy.utils.misc import load_object, warn_on_generator_with_return_value
 from scrapy.utils.log import logformatter_adapter, failure_to_exc_info
-from scrapy.exceptions import CloseSpider, DropItem, IgnoreRequest, StopDownload
+from scrapy.exceptions import CloseSpider, DropItem, IgnoreRequest
 from scrapy import signals
 from scrapy.http import Request, Response
 from scrapy.item import _BaseItem
@@ -147,14 +147,6 @@ class Scraper:
 
     def call_spider(self, result, request, spider):
         result.request = request
-        # StopDownload exceptions: make the partial response an attribute of the failure
-        if (
-            isinstance(result, Failure)
-            and isinstance(result.value, StopDownload)
-            and hasattr(result.value, "response")
-        ):
-            result.response = result.value.response
-            delattr(result.value, "response")
         dfd = defer_result(result)
         callback = request.callback or spider.parse
         warn_on_generator_with_return_value(spider, callback)
diff --git a/tests/test_crawl.py b/tests/test_crawl.py
index e2426e0da..0115b8fb9 100644
--- a/tests/test_crawl.py
+++ b/tests/test_crawl.py
@@ -488,6 +488,10 @@ with multiples lines
         self.assertIsNone(crawler.spider.meta.get("response"))
         self.assertIsInstance(crawler.spider.meta["failure"], Failure)
         self.assertIsInstance(crawler.spider.meta["failure"].value, StopDownload)
-        self.assertIsInstance(crawler.spider.meta["failure"].response, Response)
-        self.assertEqual(crawler.spider.meta["failure"].response.body, crawler.spider.meta.get("bytes_received"))
-        self.assertLess(len(crawler.spider.meta["failure"].response.body), crawler.spider.full_response_length)
+        self.assertIsInstance(crawler.spider.meta["failure"].value.response, Response)
+        self.assertEqual(
+            crawler.spider.meta["failure"].value.response.body,
+            crawler.spider.meta.get("bytes_received"))
+        self.assertLess(
+            len(crawler.spider.meta["failure"].value.response.body),
+            crawler.spider.full_response_length)