Repository: scrapy/scrapy

Issue:
Make certain args of `ScrapyAgent` and `TunnelingAgent` required
`scrapy.core.downloader.handlers.http11.ScrapyAgent` and `scrapy.core.downloader.handlers.http11.TunnelingAgent` take some args (`contextFactory` in both and `crawler` in `ScrapyAgent`) that are currently declared optional but in fact need to be passed as the code assumes they aren't None. So ideally they should be declared as required (but that requires reordering args and/or making all args of those constructors keyword-only so there is no clear and simple fix).

Discussion:

Patch:
diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst
index 1abbc4968..d4cd062fe 100644
--- a/docs/topics/downloader-middleware.rst
+++ b/docs/topics/downloader-middleware.rst
@@ -884,6 +884,10 @@ Meta tags within these tags are ignored.
    The default value of :setting:`METAREFRESH_IGNORE_TAGS` changed from
    ``['script', 'noscript']`` to ``[]``.
 
+.. versionchanged:: VERSION
+   The default value of :setting:`METAREFRESH_IGNORE_TAGS` changed from
+   ``[]`` to ``['noscript']``.
+
 .. setting:: METAREFRESH_MAXDELAY
 
 METAREFRESH_MAXDELAY
diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py
index 40ae1921a..5e84be6ba 100644
--- a/scrapy/core/downloader/handlers/http11.py
+++ b/scrapy/core/downloader/handlers/http11.py
@@ -239,15 +239,14 @@ class TunnelingAgent(Agent):
 
     def __init__(
         self,
+        *,
         reactor: ReactorBase,
         proxyConf: Tuple[str, int, Optional[bytes]],
-        contextFactory: Optional[IPolicyForHTTPS] = None,
+        contextFactory: IPolicyForHTTPS,
         connectTimeout: Optional[float] = None,
         bindAddress: Optional[bytes] = None,
         pool: Optional[HTTPConnectionPool] = None,
     ):
-        # TODO make this arg required instead
-        assert contextFactory is not None
         super().__init__(reactor, contextFactory, connectTimeout, bindAddress, pool)
         self._proxyConf: Tuple[str, int, Optional[bytes]] = proxyConf
         self._contextFactory: IPolicyForHTTPS = contextFactory
@@ -335,18 +334,16 @@ class ScrapyAgent:
 
     def __init__(
         self,
-        contextFactory: Optional[IPolicyForHTTPS] = None,
+        *,
+        contextFactory: IPolicyForHTTPS,
         connectTimeout: float = 10,
         bindAddress: Optional[bytes] = None,
         pool: Optional[HTTPConnectionPool] = None,
         maxsize: int = 0,
         warnsize: int = 0,
         fail_on_dataloss: bool = True,
-        crawler: Optional[Crawler] = None,
+        crawler: Crawler,
     ):
-        # TODO make these args required instead
-        assert contextFactory is not None
-        assert crawler is not None
         self._contextFactory: IPolicyForHTTPS = contextFactory
         self._connectTimeout: float = connectTimeout
         self._bindAddress: Optional[bytes] = bindAddress
diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py
index 2b3d95a0e..d7ac7ec35 100644
--- a/scrapy/settings/default_settings.py
+++ b/scrapy/settings/default_settings.py
@@ -239,7 +239,7 @@ MEMUSAGE_NOTIFY_MAIL = []
 MEMUSAGE_WARNING_MB = 0
 
 METAREFRESH_ENABLED = True
-METAREFRESH_IGNORE_TAGS = []
+METAREFRESH_IGNORE_TAGS = ["noscript"]
 METAREFRESH_MAXDELAY = 100
 
 NEWSPIDER_MODULE = ""


Test patch:
diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py
index 10b8ca9af..83ff25982 100644
--- a/tests/test_downloadermiddleware_redirect.py
+++ b/tests/test_downloadermiddleware_redirect.py
@@ -395,9 +395,8 @@ class MetaRefreshMiddlewareTest(unittest.TestCase):
             """content="0;URL='http://example.org/newpage'"></noscript>"""
         )
         rsp = HtmlResponse(req.url, body=body.encode())
-        req2 = self.mw.process_response(req, rsp, self.spider)
-        assert isinstance(req2, Request)
-        self.assertEqual(req2.url, "http://example.org/newpage")
+        response = self.mw.process_response(req, rsp, self.spider)
+        assert isinstance(response, Response)
 
     def test_ignore_tags_1_x_list(self):
         """Test that Scrapy 1.x behavior remains possible"""