Skip to content

feat: support timeouts in the send_request context helper - #2174

Merged
vdusek merged 3 commits into
apify:masterfrom
Battleplus:feat/send-request-timeout
Aug 25, 2026
Merged

feat: support timeouts in the send_request context helper#2174
vdusek merged 3 commits into
apify:masterfrom
Battleplus:feat/send-request-timeout

Conversation

@Battleplus

Copy link
Copy Markdown

Closes #2138

Summary

SendRequestFunction (used via context.send_request() in request handlers) only accepted url, method, payload and headers, so a handler could not bound how long an extra HTTP call may take. All four HTTP client implementations (HttpxHttpClient, CurlImpersonateHttpClient, ImpitHttpClient, PlaywrightHttpClient) already support a per-request timeout: timedelta | None — this PR just exposes it through the public contract.

Changes

  • src/crawlee/_types.py: add timeout: timedelta | None = None to the SendRequestFunction protocol + docstring.
  • src/crawlee/crawlers/_basic/_basic_crawler.py: thread timeout through the _prepare_send_request_function closure into HttpClient.send_request.
  • src/crawlee/crawlers/_playwright/_playwright_http_client.py: fixPlaywrightHttpClient passed timeout.total_seconds() to Playwright's APIRequestContext.fetch, which expects milliseconds. Without this fix, any timeout passed via context.send_request() on a Playwright crawler would be ~1000x too short (e.g. timedelta(seconds=12) → 12ms).
  • tests/unit/crawlers/_basic/test_basic_crawler.py: regression test — context.send_request('/slow?delay=2', timeout=timedelta(milliseconds=100)) raises asyncio.TimeoutError; plus a unit test asserting timeout is forwarded to HttpClient.send_request.
  • tests/unit/crawlers/_playwright/test_playwright_http_client.py: regression test asserting the seconds→milliseconds conversion.

Validation

  • pytest tests/unit/crawlers/_basic/test_basic_crawler.py → 112 passed, 1 skipped
  • New tests (forwarding, real timeout, Playwright ms conversion) all pass
  • ruff check + ruff format --check clean on all touched files

Note: no new dependency; timeout uses the existing timedelta convention across the Python HTTP clients (the JS implementation names it timeoutMillis, but the Python clients are timedelta-based).

Expose the HTTP clients' existing per-request `timeout` support through
`SendRequestFunction` so request handlers can bound how long an extra
HTTP call may take. Threads the value through `context.send_request()`
into `HttpClient.send_request`.

Also fixes `PlaywrightHttpClient` which passed `timedelta.total_seconds()`
to Playwright's `APIRequestContext.fetch`, which expects milliseconds —
previously any timeout would be ~1000x too short. Adds regression tests
for both the forwarding and the unit conversion.

Closes apify#2138

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@Battleplus

Copy link
Copy Markdown
Author

Follow-up audit found and fixed a Playwright edge case: timedelta(0) is a valid explicit Playwright timeout value (it disables Playwright's default timeout), but the truthiness check converted it to None. Commit 94c61229 now distinguishes None from zero and adds a regression test. Validation: targeted Playwright tests 2/2, Ruff lint and format checks passed.

@vdusek
vdusek removed the request for review from apify-factory August 24, 2026 07:03

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @Pijukatel please check it as well

@vdusek
vdusek requested a review from Pijukatel August 24, 2026 08:47
vdusek added a commit that referenced this pull request Aug 24, 2026
…ignoring it (#2187)

Follow-up to #2174: a `timedelta(0)` timeout is falsy, so `if timeout
else None`-style checks silently dropped it and treated it as "no
timeout" instead of "expire immediately".

- `ImpitHttpClient` and `HttpxHttpClient.stream()` only needed the `is
not None` fix — both already enforce an explicit `0` timeout correctly
on their own.
- `CurlImpersonateHttpClient` needed more: `curl_cffi` (via `libcurl`)
treats an explicit `timeout=0` as *no timeout*, so the same fix alone
doesn't change observable behavior there. Added an explicit guard in
`crawl`, `send_request`, and `stream` that raises `asyncio.TimeoutError`
immediately for a non-positive timeout, verified against a real slow
endpoint.
- `PlaywrightHttpClient` needed the same guard as
`CurlImpersonateHttpClient`: Playwright's `APIRequestContext.fetch` also
treats an explicit `timeout=0` as "disable timeout". #2174 originally
carried this fix; it's included here instead so all four HTTP clients
are covered in one place.
- Added a regression test covering all four clients.

*✍️ Drafted by Claude Code*
…imeout

# Conflicts:
#	src/crawlee/crawlers/_playwright/_playwright_http_client.py
#	tests/unit/crawlers/_playwright/test_playwright_http_client.py
@vdusek
vdusek merged commit c2013a0 into apify:master Aug 25, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support timeouts in send_request

4 participants