From 5bf921b3087f900cc765202b7132918490f24a46 Mon Sep 17 00:00:00 2001 From: karesansui Date: Tue, 17 Mar 2026 00:53:52 +0900 Subject: [PATCH] fix: preserve HTTP method across retries in spider session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SessionManager.fetch() pops `method` from `_session_kwargs`, which mutates the original request dict. When the engine retries a blocked request via request.copy(), the copy no longer has `method`, so it defaults to GET. Steps to reproduce: 1. Yield Request(url, method="POST", data=...) 2. Target returns a response that triggers is_blocked() 3. Engine retries via request.copy() → second fetch uses GET Fix: copy the kwargs dict before popping, so the original request stays intact. --- scrapling/spiders/session.py | 6 ++-- tests/spiders/test_session.py | 57 +++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/scrapling/spiders/session.py b/scrapling/spiders/session.py index cc07042..536be6d 100644 --- a/scrapling/spiders/session.py +++ b/scrapling/spiders/session.py @@ -112,10 +112,12 @@ class SessionManager: client = session._client if isinstance(client, _ASyncSessionLogic): + kwargs = request._session_kwargs.copy() + method = cast(SUPPORTED_HTTP_METHODS, kwargs.pop("method", "GET")) response = await client._make_request( - method=cast(SUPPORTED_HTTP_METHODS, request._session_kwargs.pop("method", "GET")), + method=method, url=request.url, - **request._session_kwargs, + **kwargs, ) else: # Sync session or other types - shouldn't happen in async context diff --git a/tests/spiders/test_session.py b/tests/spiders/test_session.py index c1eed5d..0c422ef 100644 --- a/tests/spiders/test_session.py +++ b/tests/spiders/test_session.py @@ -1,9 +1,12 @@ """Tests for the SessionManager class.""" +from unittest.mock import AsyncMock, PropertyMock + from scrapling.core._types import Any import pytest from scrapling.spiders.session import SessionManager +from scrapling.spiders.request import Request class MockSession: # type: ignore[type-arg] @@ -350,3 +353,57 @@ class TestSessionManagerIntegration: # After close - all inactive await manager.close() assert all(not s._is_alive for s in sessions) + + +class TestSessionManagerFetch: + """Test SessionManager fetch behavior.""" + + @pytest.mark.asyncio + async def test_fetch_preserves_request_method(self): + """Test that fetch does not mutate request._session_kwargs. + + Previously, fetch() used pop("method") which removed the method + key from the original request dict. This caused retried requests + (via request.copy()) to lose their HTTP method and fall back to GET. + """ + from scrapling.engines.static import _ASyncSessionLogic + from scrapling.fetchers import FetcherSession + from scrapling.engines.toolbelt.custom import Response + + mock_response = Response( + url="https://example.com", + content=b"ok", + status=200, + reason="OK", + cookies={}, + headers={"content-type": "text/html"}, + request_headers={}, + ) + mock_response.meta = {} + + mock_client = AsyncMock(spec=_ASyncSessionLogic) + mock_client._make_request = AsyncMock(return_value=mock_response) + + mock_session = AsyncMock(spec=FetcherSession) + mock_session._client = mock_client + mock_session._is_alive = True + + manager = SessionManager() + manager._sessions["default"] = mock_session + manager._default_session_id = "default" + manager._started = True + + request = Request("https://example.com", method="POST", data={"key": "value"}) + + assert request._session_kwargs["method"] == "POST" + + await manager.fetch(request) + + # method must still be present after fetch + assert "method" in request._session_kwargs + assert request._session_kwargs["method"] == "POST" + + # verify the correct method was passed to _make_request + mock_client._make_request.assert_called_once() + call_kwargs = mock_client._make_request.call_args + assert call_kwargs.kwargs["method"] == "POST"