perf(impit): optimize client and connection reuse across Sessions by Mantisus · Pull Request #2140 · apify/crawlee-python

Mantisus · 2026-08-07T14:32:47Z

Description

Cookie handling has been moved entirely from impit to ImpitHttpClient. Clients are now reused per proxy instead of being recreated for every Session, so a single client keeps its connection pool alive across all of them. The result is higher performance than that of HttpxHttpClient.
Redirects are followed one hop at a time, which is what makes per-session cookie handling possible. Header handling along the chain now follows the WHATWG Fetch algorithm.

Testing

Added tests for extracting cookies from Set-Cookie headers into the jar and for building the Cookie header.
Added tests for redirect handling in ImpitHttpClient.

Copilot

Pull request overview

This PR refactors ImpitHttpClient to reuse underlying impit.AsyncClient instances across sessions (per-proxy), while taking over redirect handling to process redirects hop-by-hop so per-session cookie jars can be applied and updated at each hop.

Changes:

Moved cookie extraction / cookie header construction into SessionCookies, and integrated it into ImpitHttpClient on each redirect hop.
Implemented WHATWG Fetch–style redirect behavior (method/body/header adjustments + cross-origin header dropping) and added a TooManyRedirectsError.
Updated dependency constraints/lockfile and added unit tests for cookies and redirect semantics.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
uv.lock	Bumps `impit` locked version to `0.13.2`.
pyproject.toml	Raises minimum `impit` version to `>=0.13.2`.
src/crawlee/sessions/_cookies.py	Adds `get_cookie_string()` and `extract_cookie_from_header()` helpers on `SessionCookies`.
src/crawlee/http_clients/_impit.py	Adds hop-by-hop redirect handling, cross-origin/body-header logic, and client reuse per proxy.
src/crawlee/errors.py	Introduces `TooManyRedirectsError` for redirect limit enforcement.
tests/unit/sessions/test_cookies.py	Adds coverage for Set-Cookie parsing and Cookie header construction.
tests/unit/server.py	Adds test endpoints for redirect loops and method/body echoing.
tests/unit/http_clients/test_impit.py	Adds unit tests for session cookie isolation, redirect behavior, and redirect limits.

Suppressed comments (1)

src/crawlee/http_clients/_impit.py:335

After switching to a counted redirect loop, add an explicit guard before following the next hop so that max_redirects truly limits the number of redirects followed (otherwise an endless chain can still be followed one hop too far).

            next_url = current_url.join(URL(location))
            if next_url.scheme not in _HTTP_SCHEMES:
                return response

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

vdusek · 2026-08-10T11:16:20Z

+            The final response of the redirect chain.
+        """
+        client = self._get_client(proxy_info.url if proxy_info else None)
+        current_url = URL(url)


Important: wrapping the URL in yarl.URL and sending str(current_url) on line 317 re-encodes it -- yarl decodes percent-escapes it considers safe, so X-Amz-Credential=AKIA%2F20240101%2F... goes on the wire as AKIA/20240101/... and presigned links start failing signature checks. This hits every request through the default client, not only redirect chains; master passed the string through verbatim.

Suggest keeping url as a str for the wire and building a yarl.URL only inside the redirect branch (for join() and _is_cross_origin).

vdusek · 2026-08-10T11:16:20Z

        self._async_client_kwargs = async_client_kwargs

-        self._client_by_proxy_url = LRUCache[str | None, _ClientCacheEntry](maxsize=10)
+        self._client_by_proxy_url = dict[str | None, AsyncClient]()


Just noting, this is the same as in #2104 (comment).

I would also appreciate a short docstring here explaining its purpose and why dict is being used here (cache without an upper bound).

And we should also unify the naming of this attribute across HTTP clients.

vdusek · 2026-08-10T11:16:20Z

+            if session and (cookie_string := session.cookies.get_cookie_string(str(current_url))):
+                headers['cookie'] = cookie_string


Note: headers['cookie'] is assigned but never cleared, so on a hop where the jar matches nothing the previous hop's value is sent again -- a cookie stored with path=/redirect reaches /cookies after the 302. Cross-origin hops are safe (_CROSS_ORIGIN_HEADERS strips it), so this violates cookie path scoping within one origin rather than leaking across sites.

Computing per-hop headers instead of mutating the shared dict fixes it while keeping the caller-supplied Cookie header that the new tests pin.

vdusek · 2026-08-10T11:16:20Z

+                url=str(current_url),
+                content=content,
+                headers=headers or None,
+                timeout=timeout.total_seconds() if timeout else None,


Note: the full timeout is now applied per hop, so a chain can take up to (max_redirects + 1) x timeout. crawl() is bounded by SharedTimeout and in-handler send_request by request_handler_timeout, so the practical blast radius is small -- but it is a change from the single impit call. Worth a deliberate call on whether this should be a deadline for the whole chain.

vdusek · 2026-08-10T11:16:20Z



+@docs_group('Errors')
+class TooManyRedirectsError(Exception):


Only ImpitHttpClient raises this -- HttpxHttpClient surfaces httpx.TooManyRedirects and curl-impersonate its own -- and it appears in no public Raises: block, while crawl/send_request/stream all document ProxyError. Worth reconsideration.

vdusek · 2026-08-10T11:16:21Z

+        self._jar.add_cookie_header(url_request)
+        return url_request.get_header('Cookie', '')
+
+    def extract_cookie_from_header(self, url: str, set_cookie_headers: list[str]) -> None:


Nit: this takes a list of headers and extracts many cookies, so the plural reads more accurately. Cheap to rename now, breaking once released (the only call site is _impit.py:325).

Suggested change

def extract_cookie_from_header(self, url: str, set_cookie_headers: list[str]) -> None:

def extract_cookies_from_headers(self, url: str, set_cookie_headers: list[str]) -> None:

vdusek · 2026-08-10T11:16:21Z

    await send_json_response(send, headers)


+async def echo_method(scope: dict[str, Any], receive: Receive, send: Send) -> None:


Nit: this body-reading loop is a verbatim copy of post_echo's, and both spin forever if receive() yields http.disconnect instead of http.request. A small shared read_body(receive) helper would cover both.

vdusek · 2026-08-10T11:16:21Z

+    assert exc_info.value.max_redirects == 2
+
+
+async def test_stream_follows_redirects(http_client: ImpitHttpClient, server_url: URL) -> None:


Note: the suite exercises send_request and stream, but nothing covers crawl() with a redirect chain plus session cookies -- the path the crawler actually takes. A test pinning the on-the-wire target for a URL with %2F/%3A in the query would also lock down the encoding regression flagged in _impit.py.

vdusek · 2026-08-10T11:16:21Z

+    assert (await read_json(response))['cookies'] == {'manual': 'value'}
+
+
+async def test_session_cookies_replace_explicit_cookie_header(http_client: ImpitHttpClient, server_url: URL) -> None:


Question: this inverts the precedence master had -- an explicit Cookie header used to beat the impit cookie jar. Sessions are on by default, so a header hand-set on a Request is now silently dropped as soon as the jar holds anything for that domain. The new behavior looks like the right call; could it be spelled out in the PR description so it reaches the changelog?

vdusek · 2026-08-10T11:38:23Z

    "cachetools>=5.5.0",
    "colorama>=0.4.0",
-    "impit>=0.8.0",
+    "impit>=0.13.2",


Do we really need to bump the constraint to exactly this version?

optimize client and connection reuse across Sessions

0cde6cd

Mantisus self-assigned this Aug 7, 2026

Mantisus requested review from vdusek and a lite review from Copilot August 7, 2026 14:33

Copilot started reviewing on behalf of Mantisus August 7, 2026 14:34 View session

Copilot AI reviewed Aug 7, 2026

View reviewed changes

Comment thread src/crawlee/http_clients/_impit.py Outdated

Comment thread src/crawlee/http_clients/_impit.py

Comment thread src/crawlee/http_clients/_impit.py

Potential fix for pull request finding

36b96dd

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Mantisus requested a review from Pijukatel August 10, 2026 10:13

vdusek reviewed Aug 10, 2026

View reviewed changes

Navigation Menu

Uh oh!

perf(impit): optimize client and connection reuse across Sessions - #2140

perf(impit): optimize client and connection reuse across Sessions#2140
Mantisus wants to merge 2 commits into
apify:masterfrom
Mantisus:impit-manucal-cookies

Mantisus commented Aug 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

vdusek Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		if session and (cookie_string := session.cookies.get_cookie_string(str(current_url))):
		headers['cookie'] = cookie_string



		@docs_group('Errors')
		class TooManyRedirectsError(Exception):

	def extract_cookie_from_header(self, url: str, set_cookie_headers: list[str]) -> None:
	def extract_cookies_from_headers(self, url: str, set_cookie_headers: list[str]) -> None:

		await send_json_response(send, headers)


		async def echo_method(scope: dict[str, Any], receive: Receive, send: Send) -> None:

		assert exc_info.value.max_redirects == 2


		async def test_stream_follows_redirects(http_client: ImpitHttpClient, server_url: URL) -> None:

		assert (await read_json(response))['cookies'] == {'manual': 'value'}


		async def test_session_cookies_replace_explicit_cookie_header(http_client: ImpitHttpClient, server_url: URL) -> None:

Uh oh!

Conversation

Mantisus commented Aug 7, 2026

Description

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants