Skip to content

Commit ac18622

Browse files
authored
fix(static): send the request once when retries is below 1 (#420)
1 parent 63cdc99 commit ac18622

5 files changed

Lines changed: 62 additions & 2 deletions

File tree

scrapling/engines/static.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ def _make_request(self, method: SUPPORTED_HTTP_METHODS, stealth: Optional[bool]
228228
stealth = self._stealth if stealth is None else stealth
229229

230230
selector_config = self._get_param(kwargs, "selector_config", self.selector_config) or self.selector_config
231-
max_retries = self._get_param(kwargs, "retries", self._default_retries)
231+
# Always attempt the request once; `retries` below 1 (or `None`) means "send it, but don't retry"
232+
max_retries = max(1, self._get_param(kwargs, "retries", self._default_retries) or 1)
232233
retry_delay = self._get_param(kwargs, "retry_delay", self._default_retry_delay)
233234
static_proxy = kwargs.pop("proxy", None)
234235

@@ -443,7 +444,8 @@ async def _make_request(self, method: SUPPORTED_HTTP_METHODS, stealth: Optional[
443444
stealth = self._stealth if stealth is None else stealth
444445

445446
selector_config = self._get_param(kwargs, "selector_config", self.selector_config) or self.selector_config
446-
max_retries = self._get_param(kwargs, "retries", self._default_retries)
447+
# Always attempt the request once; `retries` below 1 (or `None`) means "send it, but don't retry"
448+
max_retries = max(1, self._get_param(kwargs, "retries", self._default_retries) or 1)
447449
retry_delay = self._get_param(kwargs, "retry_delay", self._default_retry_delay)
448450
static_proxy = kwargs.pop("proxy", None)
449451

tests/fetchers/async/test_requests.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,3 +160,8 @@ async def test_selector_config_overrides_configure(
160160
)
161161
assert response._storage is not None
162162
assert response.url == "from-request.test"
163+
164+
async def test_retries_below_one_still_performs_the_request(self, fetcher, urls):
165+
"""``retries`` below 1 means "send the request once", not "send nothing"."""
166+
assert (await fetcher.get(urls["status_200"], retries=0)).status == 200
167+
assert (await fetcher.get(urls["status_200"], retries=-1)).status == 200

tests/fetchers/async/test_requests_session.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,28 @@ async def test_proxy_rotates_per_retry_attempt(self):
6060

6161
proxies_used = [call.kwargs["proxy"] for call in mocked_request.call_args_list]
6262
assert proxies_used == ["http://p1:8080", "http://p2:8080"]
63+
64+
@pytest.mark.asyncio
65+
@pytest.mark.parametrize("retries", [0, -1, None])
66+
async def test_retries_below_one_still_sends_the_request(self, retries):
67+
"""A session-level retries below 1 must still send the request once instead of skipping it"""
68+
async with AsyncFetcherSession(retries=retries, retry_delay=0) as session:
69+
with (
70+
patch.object(session._async_curl_session, "request", new=AsyncMock()) as mocked_request,
71+
patch("scrapling.engines.static.ResponseFactory.from_http_request", return_value=MagicMock()),
72+
):
73+
await session.get("http://example.com")
74+
75+
assert mocked_request.call_count == 1
76+
77+
@pytest.mark.asyncio
78+
async def test_per_request_retries_below_one_still_sends_the_request(self):
79+
"""A per-request retries of 0 must override the session default without skipping the request"""
80+
async with AsyncFetcherSession(retries=3, retry_delay=0) as session:
81+
with (
82+
patch.object(session._async_curl_session, "request", new=AsyncMock()) as mocked_request,
83+
patch("scrapling.engines.static.ResponseFactory.from_http_request", return_value=MagicMock()),
84+
):
85+
await session.get("http://example.com", retries=0)
86+
87+
assert mocked_request.call_count == 1

tests/fetchers/sync/test_requests.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,3 +151,8 @@ def test_selector_config_overrides_configure(self, fetcher, _reset_fetcher_confi
151151
)
152152
assert response._storage is not None
153153
assert response.url == "from-request.test"
154+
155+
def test_retries_below_one_still_performs_the_request(self, fetcher):
156+
"""``retries`` below 1 means "send the request once", not "send nothing"."""
157+
assert fetcher.get(self.status_200, retries=0).status == 200
158+
assert fetcher.get(self.status_200, retries=-1).status == 200

tests/fetchers/sync/test_requests_session.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,26 @@ def test_proxy_rotates_per_retry_attempt(self):
8282

8383
proxies_used = [call.kwargs["proxy"] for call in mocked_request.call_args_list]
8484
assert proxies_used == ["http://p1:8080", "http://p2:8080"]
85+
86+
@pytest.mark.parametrize("retries", [0, -1, None])
87+
def test_retries_below_one_still_sends_the_request(self, retries):
88+
"""A session-level retries below 1 must still send the request once instead of skipping it"""
89+
with FetcherSession(retries=retries, retry_delay=0) as session:
90+
with (
91+
patch.object(session._curl_session, "request") as mocked_request,
92+
patch("scrapling.engines.static.ResponseFactory.from_http_request", return_value=MagicMock()),
93+
):
94+
session.get("http://example.com")
95+
96+
assert mocked_request.call_count == 1
97+
98+
def test_per_request_retries_below_one_still_sends_the_request(self):
99+
"""A per-request retries of 0 must override the session default without skipping the request"""
100+
with FetcherSession(retries=3, retry_delay=0) as session:
101+
with (
102+
patch.object(session._curl_session, "request") as mocked_request,
103+
patch("scrapling.engines.static.ResponseFactory.from_http_request", return_value=MagicMock()),
104+
):
105+
session.get("http://example.com", retries=0)
106+
107+
assert mocked_request.call_count == 1

0 commit comments

Comments
 (0)