From f03b47db05e623189cf7719e647d18e8457494f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io>
Date: Fri, 27 Jan 2023 17:35:32 +0100
Subject: [PATCH] Make NO_CALLBACK a callable

---
 scrapy/http/request/__init__.py | 52 +++++++++++++++------------------
 setup.py                        |  1 -
 tests/test_http_request.py      |  8 +++--
 3 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py
index 068a4baa3..de13cf264 100644
--- a/scrapy/http/request/__init__.py
+++ b/scrapy/http/request/__init__.py
@@ -5,10 +5,8 @@ requests in Scrapy.
 See documentation in docs/topics/request-response.rst
 """
 import inspect
-from enum import Enum
 from typing import Callable, List, Optional, Tuple, Type, TypeVar, Union
 
-from typing_extensions import Final
 from w3lib.url import safe_url_string
 
 import scrapy
@@ -23,21 +21,22 @@ from scrapy.utils.url import escape_ajax
 RequestTypeVar = TypeVar("RequestTypeVar", bound="Request")
 
 
-# https://github.com/python/typing/issues/689#issuecomment-561425237
-class NoCallbackType(Enum):
-    NO_CALLBACK = 0
+def NO_CALLBACK(*args, **kwargs):
+    """When assigned to the ``callback`` parameter of
+    :class:`~scrapy.http.Request`, it indicates that the request is not meant
+    to have a spider callback at all.
 
-
-#: When assigned to the ``callback`` parameter of
-#: :class:`~scrapy.http.Request`, it indicates that the request is not meant to
-#: have a spider callback at all.
-#:
-#: This value should be used by :ref:`components <topics-components>`
-#: that create and handle their own requests, e.g. through
-#: :meth:`scrapy.core.engine.ExecutionEngine.download`, so that download
-#: middlewares handling such requests can treat them differently from requests
-#: intended for the :meth:`~scrapy.Spider.parse` callback.
-NO_CALLBACK: Final = NoCallbackType.NO_CALLBACK
+    This value should be used by :ref:`components <topics-components>` that
+    create and handle their own requests, e.g. through
+    :meth:`scrapy.core.engine.ExecutionEngine.download`, so that download
+    middlewares handling such requests can treat them differently from requests
+    intended for the :meth:`~scrapy.Spider.parse` callback.
+    """
+    raise RuntimeError(
+        "The NO_CALLBACK callback has been called. This is a special callback "
+        "value intended for requests whose callback is never meant to be "
+        "called."
+    )
 
 
 class Request(object_ref):
@@ -67,8 +66,6 @@ class Request(object_ref):
     Currently used by :meth:`Request.replace`, :meth:`Request.to_dict` and
     :func:`~scrapy.utils.request.request_from_dict`.
     """
-    callback: Union[None, NoCallbackType, Callable]
-    errback: Optional[Callable]
 
     def __init__(
         self,
@@ -94,8 +91,14 @@ class Request(object_ref):
             raise TypeError(f"Request priority not an integer: {priority!r}")
         self.priority = priority
 
-        self._set_xback("callback", callback)
-        self._set_xback("errback", errback)
+        if not (callable(callback) or callback is None):
+            raise TypeError(
+                f"callback must be a callable, got {type(callback).__name__}"
+            )
+        if not (callable(errback) or errback is None):
+            raise TypeError(f"errback must be a callable, got {type(errback).__name__}")
+        self.callback = callback
+        self.errback = errback
 
         self.cookies = cookies or {}
         self.headers = Headers(headers or {}, encoding=encoding)
@@ -105,15 +108,6 @@ class Request(object_ref):
         self._cb_kwargs = dict(cb_kwargs) if cb_kwargs else None
         self.flags = [] if flags is None else list(flags)
 
-    def _set_xback(self, name: str, value: Optional[Callable]) -> None:
-        if not (
-            callable(value)
-            or value is None
-            or (name == "callback" and value is NO_CALLBACK)
-        ):
-            raise TypeError(f"{name} must be a callable, got {type(value).__name__}")
-        setattr(self, name, value)
-
     @property
     def cb_kwargs(self) -> dict:
         if self._cb_kwargs is None:
diff --git a/setup.py b/setup.py
index 9150dac0b..f53334d4e 100644
--- a/setup.py
+++ b/setup.py
@@ -34,7 +34,6 @@ install_requires = [
     "packaging",
     "tldextract",
     "lxml>=4.3.0",
-    "typing-extensions>=3.10.0.0",
 ]
 extras_require = {}
 cpython_dependencies = [
diff --git a/tests/test_http_request.py b/tests/test_http_request.py
index 233a5f0b2..e800f427f 100644
--- a/tests/test_http_request.py
+++ b/tests/test_http_request.py
@@ -314,8 +314,10 @@ class RequestTest(unittest.TestCase):
         r5 = self.request_class(
             url="http://example.com",
             callback=NO_CALLBACK,
+            errback=NO_CALLBACK,
         )
         self.assertIs(r5.callback, NO_CALLBACK)
+        self.assertIs(r5.errback, NO_CALLBACK)
 
     def test_callback_and_errback_type(self):
         with self.assertRaises(TypeError):
@@ -328,8 +330,10 @@ class RequestTest(unittest.TestCase):
                 callback="a_function",
                 errback="a_function",
             )
-        with self.assertRaises(TypeError):
-            self.request_class("http://example.com", errback=NO_CALLBACK)
+
+    def test_no_callback(self):
+        with self.assertRaises(RuntimeError):
+            NO_CALLBACK()
 
     def test_from_curl(self):
         # Note: more curated tests regarding curl conversion are in