Merge branch 'main' into close-inactive-contexts

Author: elacuesta

.bumpversion.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 0.0.29
+current_version = 0.0.30
 commit = True
 tag = True
 

README.md

@@ -22,7 +22,7 @@ to integrate `asyncio`-based projects such as `Playwright`.
 
 ### Minimum required versions
 
-* Python >= 3.7
+* Python >= 3.8
 * Scrapy >= 2.0 (!= 2.4.0)
 * Playwright >= 1.15
 

docs/changelog.md

@@ -1,6 +1,12 @@
 # scrapy-playwright changelog
 
 
+### [v0.0.30](https://github.com/scrapy-plugins/scrapy-playwright/releases/tag/v0.0.30) (2023-08-17)
+
+* Fix page_init_callback duplication (#222)
+* Bump minimum Python version from 3.7 to 3.8 (#223)
+
+
 ### [v0.0.29](https://github.com/scrapy-plugins/scrapy-playwright/releases/tag/v0.0.29) (2023-08-11)
 
 * Set exc_info=True for warning log records (#219)

scrapy_playwright/__init__.py

@@ -1 +1 @@
-__version__ = "0.0.29"
+__version__ = "0.0.30"

scrapy_playwright/_utils.py

@@ -1,7 +1,7 @@
 import logging
-from typing import Awaitable, Iterator, Optional, Tuple
+from typing import Awaitable, Iterator, Optional, Tuple, Union
 
-from playwright.async_api import Error, Page
+from playwright.async_api import Error, Page, Request, Response
 from scrapy import Spider
 from scrapy.http.headers import Headers
 from scrapy.settings import Settings
@@ -88,3 +88,13 @@ def _read_float_setting(settings: Settings, key: str) -> Optional[float]:
     except (KeyError, TypeError, ValueError):
         pass
     return None
+
+
+async def _get_header_value(
+    resource: Union[Request, Response],
+    header_name: str,
+) -> Optional[str]:
+    try:
+        return await resource.header_value(header_name)
+    except Exception:
+        return None

scrapy_playwright/handler.py

@@ -32,6 +32,7 @@
 from scrapy_playwright.page import PageMethod
 from scrapy_playwright._utils import (
     _encode_body,
+    _get_header_value,
     _get_page_content,
     _is_safe_close_error,
     _maybe_await,
@@ -239,27 +240,6 @@ async def _create_page(self, request: Request, spider: Spider) -> Page:
         self._set_max_concurrent_page_count()
         if self.default_navigation_timeout is not None:
             page.set_default_navigation_timeout(self.default_navigation_timeout)
-        page_init_callback = request.meta.get("playwright_page_init_callback")
-        if page_init_callback:
-            try:
-                page_init_callback = load_object(page_init_callback)
-                await page_init_callback(page, request)
-            except Exception as ex:
-                logger.warning(
-                    "[Context=%s] Page init callback exception for %s exc_type=%s exc_msg=%s",
-                    context_name,
-                    repr(request),
-                    type(ex),
-                    str(ex),
-                    extra={
-                        "spider": spider,
-                        "context_name": context_name,
-                        "scrapy_request_url": request.url,
-                        "scrapy_request_method": request.method,
-                        "exception": ex,
-                    },
-                    exc_info=True,
-                )
 
         page.on("close", self._make_close_page_callback(context_name))
         page.on("crash", self._make_close_page_callback(context_name))
@@ -399,10 +379,6 @@ async def _download_request_with_page(
         )
         request.meta["download_latency"] = time() - start_time
 
-        if not request.meta.get("playwright_include_page"):
-            await page.close()
-            self.stats.inc_value("playwright/page_count/closed")
-
         server_ip_address = None
         with suppress(AttributeError, KeyError, TypeError, ValueError):
             server_addr = await response.server_addr()
@@ -411,6 +387,10 @@ async def _download_request_with_page(
         with suppress(AttributeError):
             request.meta["playwright_security_details"] = await response.security_details()
 
+        if not request.meta.get("playwright_include_page"):
+            await page.close()
+            self.stats.inc_value("playwright/page_count/closed")
+
         body, encoding = _encode_body(headers=headers, text=body_str)
         respcls = responsetypes.from_args(headers=headers, url=page.url, body=body)
         return respcls(
@@ -683,19 +663,22 @@ async def _maybe_execute_page_init_callback(
 
 def _make_request_logger(context_name: str, spider: Spider) -> Callable:
     async def _log_request(request: PlaywrightRequest) -> None:
-        referrer = await request.header_value("referer")
+        log_args = [context_name, request.method.upper(), request.url, request.resource_type]
+        referrer = await _get_header_value(request, "referer")
+        if referrer:
+            log_args.append(referrer)
+            log_msg = "[Context=%s] Request: <%s %s> (resource type: %s, referrer: %s)"
+        else:
+            log_msg = "[Context=%s] Request: <%s %s> (resource type: %s)"
         logger.debug(
-            "[Context=%s] Request: <%s %s> (resource type: %s, referrer: %s)",
-            context_name,
-            request.method.upper(),
-            request.url,
-            request.resource_type,
-            referrer,
+            log_msg,
+            *log_args,
             extra={
                 "spider": spider,
                 "context_name": context_name,
                 "playwright_request_url": request.url,
                 "playwright_request_method": request.method,
+                "playwright_resource_type": request.resource_type,
             },
         )
 
@@ -704,16 +687,15 @@ async def _log_request(request: PlaywrightRequest) -> None:
 
 def _make_response_logger(context_name: str, spider: Spider) -> Callable:
     async def _log_response(response: PlaywrightResponse) -> None:
-        referrer = await response.header_value("referer")
-        log_args = [context_name, response.status, response.url, referrer]
-        if 300 <= response.status < 400:
-            location = await response.header_value("location")
+        log_args = [context_name, response.status, response.url]
+        location = await _get_header_value(response, "location")
+        if location:
             log_args.append(location)
-            msg = "[Context=%s] Response: <%i %s> (referrer: %s, location: %s)"
+            log_msg = "[Context=%s] Response: <%i %s> (location: %s)"
         else:
-            msg = "[Context=%s] Response: <%i %s> (referrer: %s)"
+            log_msg = "[Context=%s] Response: <%i %s>"
         logger.debug(
-            msg,
+            log_msg,
             *log_args,
             extra={
                 "spider": spider,

tests/tests_asyncio/test_utils.py

@@ -6,7 +6,13 @@
 from playwright.async_api import Error as PlaywrightError
 from scrapy import Spider
 from scrapy.http.headers import Headers
-from scrapy_playwright._utils import _get_page_content, _NAVIGATION_ERROR_MSG, _encode_body
+from scrapy_playwright._utils import (
+    _NAVIGATION_ERROR_MSG,
+    _encode_body,
+    _get_header_value,
+    _get_page_content,
+    _maybe_await,
+)
 
 
 class TestPageContent(IsolatedAsyncioTestCase):
@@ -119,3 +125,35 @@ async def test_encode_mismatch(self):
         )
         assert encoding == "gb18030"
         assert body == text.encode(encoding)
+
+
+class TestHeaderValue(IsolatedAsyncioTestCase):
+    @pytest.mark.asyncio
+    async def test_get_header_ok(self):
+        async def _identity(x):
+            return x
+
+        resource = AsyncMock()
+        resource.header_value = _identity
+        assert "asdf" == await _get_header_value(resource, "asdf")
+        assert "qwerty" == await _get_header_value(resource, "qwerty")
+
+    async def test_get_header_exception(self):
+        resource = AsyncMock()
+        resource.header_value.side_effect = Exception("nope")
+        assert await _get_header_value(resource, "asdf") is None
+        assert await _get_header_value(resource, "qwerty") is None
+
+
+class TestMaybeAwait(IsolatedAsyncioTestCase):
+    @pytest.mark.asyncio
+    async def test_maybe_await(self):
+        async def _awaitable_identity(x):
+            return x
+
+        assert await _maybe_await(_awaitable_identity("asdf")) == "asdf"
+        assert await _maybe_await(_awaitable_identity("qwerty")) == "qwerty"
+        assert await _maybe_await(_awaitable_identity(1234)) == 1234
+        assert await _maybe_await("foo") == "foo"
+        assert await _maybe_await("bar") == "bar"
+        assert await _maybe_await(1234) == 1234

tox.ini

@@ -7,7 +7,6 @@ deps =
     pytest_asyncio==0.21.1
     pytest_cov==4.1.0
     pytest_twisted==1.14
-    playwright==1.36.0
 commands =
     playwright install
     py.test -vv --reactor=asyncio \