Replace asserts with custom checks in Scrapy subpackage (#174)

Author: vdusek

CHANGELOG.md

@@ -4,7 +4,9 @@
 
 ### Internal changes
 
+- Create a new subpackage for Scrapy pipelines
 - Remove some noqas thanks to the new Ruff release
+- Replace asserts with custom checks in Scrapy subpackage
 
 ### Fixed
 

src/apify/scrapy/middlewares/apify_proxy.py

@@ -1,10 +1,11 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING
 from urllib.parse import ParseResult, urlparse
 
 try:
+    from scrapy import Request, Spider  # noqa: TCH002
     from scrapy.core.downloader.handlers.http11 import TunnelError
+    from scrapy.crawler import Crawler  # noqa: TCH002
     from scrapy.exceptions import NotConfigured
 except ImportError as exc:
     raise ImportError(
@@ -15,10 +16,6 @@
 from ...proxy_configuration import ProxyConfiguration
 from ..utils import get_basic_auth_header
 
-if TYPE_CHECKING:
-    from scrapy import Request, Spider
-    from scrapy.crawler import Crawler
-
 
 class ApifyHttpProxyMiddleware:
     """Apify HTTP proxy middleware for Scrapy.

src/apify/scrapy/middlewares/apify_retry.py

@@ -4,7 +4,9 @@
 from typing import TYPE_CHECKING, Any
 
 try:
+    from scrapy import Spider  # noqa: TCH002
     from scrapy.downloadermiddlewares.retry import RetryMiddleware
+    from scrapy.http import Request, Response  # noqa: TCH002
     from scrapy.utils.response import response_status_message
 except ImportError as exc:
     raise ImportError(
@@ -15,9 +17,6 @@
 from ..utils import nested_event_loop, open_queue_with_custom_client, to_apify_request
 
 if TYPE_CHECKING:
-    from scrapy import Spider
-    from scrapy.http import Request, Response
-
     from ...storages import RequestQueue
 
 

src/apify/scrapy/scheduler.py

@@ -55,7 +55,9 @@ def has_pending_requests(self: ApifyScheduler) -> bool:
         Returns:
             True if the scheduler has any pending requests, False otherwise.
         """
-        assert isinstance(self._rq, RequestQueue)  # noqa: S101
+        if not isinstance(self._rq, RequestQueue):
+            raise TypeError('self._rq must be an instance of the RequestQueue class')
+
         try:
             is_finished = nested_event_loop.run_until_complete(self._rq.is_finished())
         except BaseException:
@@ -76,10 +78,14 @@ def enqueue_request(self: ApifyScheduler, request: Request) -> bool:
         call_id = crypto_random_object_id(8)
         Actor.log.debug(f'[{call_id}]: ApifyScheduler.enqueue_request was called (scrapy_request={request})...')
 
-        assert isinstance(self.spider, Spider)  # noqa: S101
+        if not isinstance(self.spider, Spider):
+            raise TypeError('self.spider must be an instance of the Spider class')
+
         apify_request = to_apify_request(request, spider=self.spider)
         Actor.log.debug(f'[{call_id}]: scrapy_request was transformed to apify_request (apify_request={apify_request})')
-        assert isinstance(self._rq, RequestQueue)  # noqa: S101
+
+        if not isinstance(self._rq, RequestQueue):
+            raise TypeError('self._rq must be an instance of the RequestQueue class')
 
         try:
             result = nested_event_loop.run_until_complete(self._rq.add_request(apify_request))
@@ -98,7 +104,9 @@ def next_request(self: ApifyScheduler) -> Request | None:
         """
         call_id = crypto_random_object_id(8)
         Actor.log.debug(f'[{call_id}]: ApifyScheduler.next_request was called...')
-        assert isinstance(self._rq, RequestQueue)  # noqa: S101
+
+        if not isinstance(self._rq, RequestQueue):
+            raise TypeError('self._rq must be an instance of the RequestQueue class')
 
         try:
             apify_request = nested_event_loop.run_until_complete(self._rq.fetch_next_request())
@@ -111,7 +119,9 @@ def next_request(self: ApifyScheduler) -> Request | None:
         if apify_request is None:
             return None
 
-        assert isinstance(self.spider, Spider)  # noqa: S101
+        if not isinstance(self.spider, Spider):
+            raise TypeError('self.spider must be an instance of the Spider class')
+
         scrapy_request = to_scrapy_request(apify_request, spider=self.spider)
         Actor.log.debug(
             f'[{call_id}]: apify_request was transformed to the scrapy_request which is gonna be returned (scrapy_request={scrapy_request})',

src/apify/scrapy/utils.py

@@ -6,10 +6,9 @@
 from base64 import b64encode
 from urllib.parse import unquote
 
-from scrapy.utils.python import to_bytes
-
 try:
     from scrapy import Request, Spider
+    from scrapy.utils.python import to_bytes
     from scrapy.utils.request import request_from_dict
 except ImportError as exc:
     raise ImportError(
@@ -51,7 +50,8 @@ def to_apify_request(scrapy_request: Request, spider: Spider) -> dict:
     Returns:
         The converted Apify request.
     """
-    assert isinstance(scrapy_request, Request)  # noqa: S101
+    if not isinstance(scrapy_request, Request):
+        raise TypeError('scrapy_request must be an instance of the scrapy.Request class')
 
     call_id = crypto_random_object_id(8)
     Actor.log.debug(f'[{call_id}]: to_apify_request was called (scrapy_request={scrapy_request})...')
@@ -91,11 +91,14 @@ def to_scrapy_request(apify_request: dict, spider: Spider) -> Request:
     Returns:
         The converted Scrapy request.
     """
-    assert isinstance(apify_request, dict)  # noqa: S101
-    assert 'url' in apify_request  # noqa: S101
-    assert 'method' in apify_request  # noqa: S101
-    assert 'id' in apify_request  # noqa: S101
-    assert 'uniqueKey' in apify_request  # noqa: S101
+    if not isinstance(apify_request, dict):
+        raise TypeError('apify_request must be a dictionary')
+
+    required_keys = ['url', 'method', 'id', 'uniqueKey']
+    missing_keys = [key for key in required_keys if key not in apify_request]
+
+    if missing_keys:
+        raise ValueError(f"apify_request must contain {', '.join(map(repr, missing_keys))} key(s)")
 
     call_id = crypto_random_object_id(8)
     Actor.log.debug(f'[{call_id}]: to_scrapy_request was called (apify_request={apify_request})...')
@@ -106,14 +109,19 @@ def to_scrapy_request(apify_request: dict, spider: Spider) -> Request:
         #   - This process involves decoding the base64-encoded request data and reconstructing
         #     the Scrapy Request object from its dictionary representation.
         Actor.log.debug(f'[{call_id}]: Restoring the Scrapy Request from the apify_request...')
+
         scrapy_request_dict_encoded = apify_request['userData']['scrapy_request']
-        assert isinstance(scrapy_request_dict_encoded, str)  # noqa: S101
+        if not isinstance(scrapy_request_dict_encoded, str):
+            raise TypeError('scrapy_request_dict_encoded must be a string')
 
         scrapy_request_dict = pickle.loads(codecs.decode(scrapy_request_dict_encoded.encode(), 'base64'))
-        assert isinstance(scrapy_request_dict, dict)  # noqa: S101
+        if not isinstance(scrapy_request_dict, dict):
+            raise TypeError('scrapy_request_dict must be a dictionary')
 
         scrapy_request = request_from_dict(scrapy_request_dict, spider=spider)
-        assert isinstance(scrapy_request, Request)  # noqa: S101
+        if not isinstance(scrapy_request, Request):
+            raise TypeError('scrapy_request must be an instance of the Request class')
+
         Actor.log.debug(f'[{call_id}]: Scrapy Request successfully reconstructed (scrapy_request={scrapy_request})...')
 
         # Update the meta field with the meta field from the apify_request