Repository: scrapy/scrapy
Pull request: Improve http status all on http error middleware
Fixes [#3851](https://github.com/scrapy/scrapy/issues/3851)

on this spider 
```
from scrapy import Spider, Request


class HttpStatusSpider(Spider):
    name = "http_status"

    def start_requests(self):
        http_status = [200, 300, 400, 500]
        urls = [
            f"http://httpbin.org/status/{status}" for status in http_status
        ]
        for url in urls:
            yield Request(
                url=url,
                callback=self.parse,
                meta={
                    "handle_httpstatus_all": False
                }
            )

    def parse(self, response, **kwargs):
        print(f"response url: {response.url}")
```
previous the fix was printing all elements on the list, after the fix just print 200 status

Problem statement:
HttpErrorMiddleware not honoring handle_httpstatus_all meta as documented
```
    def process_spider_input(self, response, spider):
        if 200 <= response.status < 300:  # common case
            return
        meta = response.meta
        if 'handle_httpstatus_all' in meta:
            return
```

Shouldn't that be more like:

```
        if 'handle_httpstatus_all' in meta and meta['handle_httpstatus_all']:
            return
```

As I read the code, setting meta['handle_httpstatus_all'] = False would likely be treated as if it was set to `True`.

Hints from issue discussion:
+1 to fix it, though I think current [docs](https://docs.scrapy.org/en/latest/topics/spider-middleware.html#std:reqmeta-handle_httpstatus_all) are technically correct, and match the current behavior:

> The handle_httpstatus_list key of Request.meta can also be used to specify which response codes to allow on a per-request basis. You can also set the meta key handle_httpstatus_all to True if you want to allow any response code for a request.
@Gallaecio Can I try this bug?

Commit message: Improve http status all on http error middleware

Patch to review:
diff --git a/scrapy/spidermiddlewares/httperror.py b/scrapy/spidermiddlewares/httperror.py
index 375042340d0..bf908d2f75c 100644
--- a/scrapy/spidermiddlewares/httperror.py
+++ b/scrapy/spidermiddlewares/httperror.py
@@ -32,7 +32,7 @@ def process_spider_input(self, response, spider):
         if 200 <= response.status < 300:  # common case
             return
         meta = response.meta
-        if 'handle_httpstatus_all' in meta:
+        if meta.get('handle_httpstatus_all', False):
             return
         if 'handle_httpstatus_list' in meta:
             allowed_statuses = meta['handle_httpstatus_list']
diff --git a/tests/test_spidermiddleware_httperror.py b/tests/test_spidermiddleware_httperror.py
index e032b247c7b..f3e5478c4dc 100644
--- a/tests/test_spidermiddleware_httperror.py
+++ b/tests/test_spidermiddleware_httperror.py
@@ -139,6 +139,19 @@ def test_meta_overrides_settings(self):
         self.assertIsNone(self.mw.process_spider_input(res404, self.spider))
         self.assertRaises(HttpError, self.mw.process_spider_input, res402, self.spider)
 
+    def test_httperror_allow_all_false(self):
+        crawler = get_crawler(_HttpErrorSpider)
+        mw = HttpErrorMiddleware.from_crawler(crawler)
+        request_httpstatus_false = Request('http://scrapytest.org', meta={'handle_httpstatus_all': False})
+        request_httpstatus_true = Request('http://scrapytest.org', meta={'handle_httpstatus_all': True})
+        res404 = self.res404.copy()
+        res404.request = request_httpstatus_false
+        res402 = self.res402.copy()
+        res402.request = request_httpstatus_true
+
+        self.assertRaises(HttpError, mw.process_spider_input, res404, self.spider)
+        self.assertIsNone(mw.process_spider_input(res402, self.spider))
+
 
 class TestHttpErrorMiddlewareIntegrational(TrialTestCase):
     def setUp(self):


Code review comments:
diff_hunk: @@ -255,7 +255,8 @@ this::
 The ``handle_httpstatus_list`` key of :attr:`Request.meta
 <scrapy.http.Request.meta>` can also be used to specify which response codes to
 allow on a per-request basis. You can also set the meta key ``handle_httpstatus_all``
-to ``True`` if you want to allow any response code for a request.
+to ``True`` if you want to allow any response code for a request, and to ``False``
+to disable it.
path: docs/topics/spider-middleware.rst
text: @user1:
I think ‘disable it’ here is slightly ambiguous. Maybe ‘disable the effects of the ``handle_httpstatus_list`` key’?

@author:
I agree, I am changing it

diff_hunk: @@ -255,7 +255,8 @@ this::
 The ``handle_httpstatus_list`` key of :attr:`Request.meta
 <scrapy.http.Request.meta>` can also be used to specify which response codes to
 allow on a per-request basis. You can also set the meta key ``handle_httpstatus_all``
-to ``True`` if you want to allow any response code for a request.
+to ``True`` if you want to allow any response code for a request, and ``False`` to
+disable the effects of the ``handle_httpstatus_list`` key.
path: docs/topics/spider-middleware.rst
text: @user1:
My bad, sorry!

```suggestion
disable the effects of the ``handle_httpstatus_all`` key.
```

@author:
fixed