fix: preserve HTTP method across retries in spider session
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user