fix(httpx): honor session cookies across HTTP client request paths by Ayush7614 · Pull Request #2104 · apify/crawlee-python

Ayush7614 · 2026-07-30T14:20:15Z

Summary

Httpx: send_request / stream / crawl share _build_request for outbound session cookies. Clients stay shared per proxy (not per session); cookies are re-applied in _HttpxTransport on every hop so they survive redirects without pinning a client to each jar.
Impit (default client): honors persist_cookies_per_session. When persistence is enabled, the session jar is attached to the client; when disabled, existing cookies are sent via a Cookie header so the shared client stays cached. Clients are cached by (proxy, CookieJar) identity.
Httpx fingerprints: Accept / Accept-Language / User-Agent come from a single fingerprint profile. HeaderGenerator.get_common_headers / get_random_user_agent_header are deprecated in favor of get_specific_headers.

Why

context.send_request() silently dropped session cookies under Httpx, breaking auth that worked for navigation (including after redirects). Impit's documented persist_cookies_per_session=False never took effect because the session jar was always attached and mutated in place.

Test plan

tests/unit/http_clients/test_http_clients.py — shared cookie send/persist/redirect coverage across clients
tests/unit/http_clients/test_httpx.py — single-generate() fingerprint headers; proxy-only client cache across sessions
tests/unit/http_clients/test_impit.py — Cookie-header path when persist is disabled

Httpx send_request/stream now send outbound session cookies like crawl. Impit respects persist_cookies_per_session, keys the client cache by jar identity, and closes cached clients on cleanup. Httpx fingerprint headers are generated from a single profile so Accept and User-Agent stay consistent.

Copilot

Pull request overview

This PR fixes inconsistent cookie handling across HTTP client request paths so that session cookies are reliably sent (and optionally persisted) whether requests go through crawler navigation (crawl) or handler-level calls (send_request/stream). It also makes Httpx’s fingerprint-derived headers internally consistent by sourcing Accept, Accept-Language, and User-Agent from a single generated fingerprint profile.

Changes:

Httpx: send_request/stream now attach outbound session cookies via the shared request-building path (matching crawl behavior).
Impit: implements persist_cookies_per_session semantics, caches clients by (proxy, cookie-jar identity), and adds client closing on cleanup/eviction.
Tests: adds coverage for cookie sending/persistence toggles, single-fingerprint headers, and Impit cleanup cache reset.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tests/unit/http_clients/test_http_clients.py	Adds unit coverage for session cookie sending in `send_request`/`stream`, persistence on/off behavior, single-fingerprint headers, and Impit cache cleanup.
src/crawlee/http_clients/_impit.py	Honors cookie persistence flag by using a resolved jar (shared vs copy), introduces client caching keyed by proxy + cookie jar identity, and closes cached clients on cleanup/eviction.
src/crawlee/http_clients/_httpx.py	Ensures handler-level requests attach session cookies and derives `Accept`/`Accept-Language`/`User-Agent` from one fingerprint profile.

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

Mantisus

Thank you for your contributions!

When persist_cookies_per_session is False, _resolve_cookie_jar builds a fresh jar for every request, and since the cache key includes the jar identity, that means a new AsyncClient per request. Consider building the Cookie header directly instead of passing a CookieJar to AsyncClient, then the client can stay cached and shared.

For example:

import urllib.request
from http.cookiejar import CookieJar


def get_cookie_header(jar: CookieJar, url: str, headers: HttpHeaders | None = None) -> str:
    request = urllib.request.Request(url, headers=dict(headers) if headers else {})
    jar.add_cookie_header(request)
    return request.get_header('Cookie', '')

Send cookies via Cookie header when persist_cookies_per_session is False so clients stay cached, and use LRUCache.popitem for eviction without fire-and-forget close tasks.

Ayush7614 · 2026-08-03T13:35:25Z

Thanks for the review @Mantisus!

Addressed in dccf1e7:

When persist_cookies_per_session=False, cookies are now sent via a Cookie header (using CookieJar.add_cookie_header) instead of attaching a fresh jar — so the Impit client stays cached/shared by proxy.
LRU eviction now uses cachetools.LRUCache.popitem().
Removed the fire-and-forget create_task(close...) path (and the __aexit__ close helper) since Impit’s Rust side already handles resource cleanup well.

Mantisus

Thanks for the update, just a few more points

Simplify cookie header merge, drop redundant LRU popitem, and move client-specific Httpx/Impit tests into dedicated test modules.

Ayush7614 · 2026-08-03T19:54:18Z

Thanks @Mantisus — addressed in 086b115:

get_header('Cookie', '')
simplified Cookie-header merge via walrus + _get_cookie_header
dropped redundant or None and manual popitem
moved Httpx/Impit-specific tests into test_httpx.py / test_impit.py

Mantisus

Please update the PR description to match the current state, otherwise LGTM

Ayush7614 · 2026-08-03T20:07:37Z

Updated the PR description to match the current implementation. Thanks @Mantisus!

Key httpx clients by session jar so cookies survive redirects, reuse _build_request from crawl, simplify Impit LRU keying by jar identity, deprecate unused HeaderGenerator helpers, and cover redirect plus persist=False cookie-header paths in tests.

Mocking get_specific_headers could not catch a regression that mixes Accept and User-Agent from separate fingerprint generations.

Ayush7614 · 2026-08-04T11:07:40Z

Thanks @vdusek

Addressed in d4f5fae / 6aaadc4:

Httpx: attach session cookie jar on the client so cookies survive redirects; crawl reuses _build_request; shared redirect cookie coverage in test_session_cookies_survive_redirect.
Impit: cache keyed by (proxy_url, CookieJar) identity; Cookie-header path when persist_cookies_per_session=False, covered by test_persist_false_still_sends_session_cookies.
Deprecated HeaderGenerator.get_common_headers / get_random_user_agent_header.
Fingerprint test now mocks _generator.generate and asserts a single call (so a dual-generate() regression would fail).

Per-session client caching pinned jars and grew without bound. Re-apply cookies in the transport on each hop (including redirects) instead so one AsyncClient per proxy stays reusable across sessions.

`async with HttpxHttpClient() as client` widens to HttpClient, so ty rejected access to `_client_by_proxy_url`.

Ayush7614 · 2026-08-04T13:02:09Z

Type-check failures on the previous run came from test_client_cache_is_shared_across_sessions: async with HttpxHttpClient() as client widens to HttpClient, so accessing _client_by_proxy_url failed under ty.

Fixed in bf9a8d0 by binding the HttpxHttpClient first (same pattern as the Impit cache test). Local ty check is clean.

Ayush7614 · 2026-08-04T13:03:02Z

cc: @vdusek

Mantisus · 2026-08-04T14:13:43Z

            headers=dict(headers) if headers else None,
            content=payload,
-            extensions={'crawlee_session': session if self._persist_cookies_per_session else None},
+            cookies=session.cookies.jar if session else None,


With this new approach that uses transport, cookies=session.cookies.jar doesn't make sense

Also, the session is already being passed to the transport. Use the session's jar no need to pass it separately.

vdusek · 2026-08-12T12:35:23Z

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


Why rename it? All HTTP clients name this attribute _client_by_proxy_url, no?

vdusek · 2026-08-12T12:54:55Z

+            session.cookies.set(f'k{i}', f'v{i}', domain=host, path='/')
+            await client.send_request(str(server_url / 'cookies'), session=session)
+
+        assert len(client._client_by_proxy_url) == 1


Note: the load-bearing new invariant is that one client per proxy is shared across sessions, so session A's cookies must never reach session B. This asserts the sharing but not the isolation. Worth adding: crawl set_cookies with session A, then request /cookies with session B and with no session, asserting {} for both.

vdusek · 2026-08-12T12:54:55Z

+
+
+async def test_client_cache_is_shared_across_sessions(server_url: URL) -> None:
+    """Distinct sessions must reuse one AsyncClient per proxy, not one client per cookie jar."""


Suggestion: _client_by_proxy_url was already a per-proxy dict on master, so this test passes there too - the docstring implies it guards this change. Also the per-iteration cookies (k{i}=v{i}) are set but never asserted.

vdusek · 2026-08-12T12:54:55Z

+        if cookie_header:
+            request.headers['cookie'] = cookie_header
+        else:
+            request.headers.pop('cookie', None)


Nit: the else branch is unreachable. When a Cookie header is present, add_cookie_header short-circuits on has_header("Cookie") and get_header returns it unchanged, so the if always wins; otherwise pop is a no-op.

Suggested change

if cookie_header:

request.headers['cookie'] = cookie_header

else:

request.headers.pop('cookie', None)

if cookie_header:

request.headers['cookie'] = cookie_header

vdusek · 2026-08-12T12:54:55Z

+    # Avoid HeaderGenerator.__init__ loading browserforge; only exercise get_specific_headers.
+    header_generator = HeaderGenerator.__new__(HeaderGenerator)
+    header_generator._generator = Mock()
+    header_generator._generator.generate = Mock(return_value=fingerprint)


The comment is not accurate - _httpx.py:124 (_DEFAULT_HEADER_GENERATOR = HeaderGenerator()) evaluates at class-body time, so importing HttpxHttpClient in this module already loads browserforge. A plain instance works and drops the __new__ bypass.

Suggested change

# Avoid HeaderGenerator.__init__ loading browserforge; only exercise get_specific_headers.

header_generator = HeaderGenerator.__new__(HeaderGenerator)

header_generator._generator = Mock()

header_generator._generator.generate = Mock(return_value=fingerprint)

header_generator = HeaderGenerator()

header_generator._generator = Mock(generate=Mock(return_value=fingerprint))

vdusek · 2026-08-12T12:54:55Z

+        """Retrieve or create an HTTP client for the given proxy URL and cookie jar.

-        If a client for the specified proxy URL does not exist, create and store a new one.
+        Clients are cached by `(proxy_url, cookie_jar)` — CookieJar hashes by identity so sessions with different


Not em dashes please - src/ is ASCII-only.

Suggested change

Clients are cached by `(proxy_url, cookie_jar)` — CookieJar hashes by identity so sessions with different

Clients are cached by `(proxy_url, cookie_jar)`: `CookieJar` hashes by identity so sessions with different

vdusek · 2026-08-12T12:54:55Z

    assert response_headers['accept-language'] == COMMON_ACCEPT_LANGUAGE

-    # By default, HTTPX uses its own User-Agent, which should be replaced by the one from the header generator.
    assert 'user-agent' in response_headers


Why deleting this comment? It was the only explanation of why the assertion below checks for python-httpx. Also the deletion is unrelated to the fix.

vdusek · 2026-08-12T13:36:51Z

+            extensions={
+                # Used by the transport to re-apply cookies on every hop (httpx strips Cookie on redirect).
+                'crawlee_cookie_jar': session.cookies.jar if session else None,
+                'crawlee_session': session if self._persist_cookies_per_session else None,
+            },


Note: building on Mantisus' open thread here -- the transport can't just read the jar off crawlee_session, because that key is None whenever persist_cookies_per_session is False, so it doubles as both the session and the persist flag. That conflation is the only reason crawlee_cookie_jar has to exist.

Pass persist_cookies_per_session into _HttpxTransport.__init__ (the transport is already per-client, L162), send extensions={'crawlee_session': session} unconditionally, and derive the jar inside the transport -- that drops both cookies= and crawlee_cookie_jar, leaving one channel with one meaning.

vdusek · 2026-08-12T13:36:51Z

+    @staticmethod
+    def _apply_cookie_header(request: httpx.Request, jar: CookieJar) -> None:
+        """Set the Cookie header from a jar for the current request URL."""
+        urllib_request = UrllibRequest(str(request.url), headers=dict(request.headers))  # noqa: S310
+        jar.add_cookie_header(urllib_request)
+        cookie_header = urllib_request.get_header('Cookie')
+        if cookie_header:
+            request.headers['cookie'] = cookie_header
+        else:
+            request.headers.pop('cookie', None)


Suggestion: this UrllibRequest + add_cookie_header + get_header('Cookie') idiom is now hand-rolled twice -- here and in ImpitHttpClient._get_cookie_header (_impit.py:142) -- and the two copies already diverge: this one pops the header when the jar yields nothing, impit returns '' and lets the caller skip.

add_cookie_header appeared nowhere in src/ before this PR. SessionCookies is what owns cookie state, so a get_cookie_header(url, headers) there would give both clients (and curl, the third latent site) one implementation instead of three.

vdusek · 2026-08-12T13:36:51Z

+        if session := cast('Session | None', request.extensions.get('crawlee_session')):
            session.cookies.store_cookies(list(response.cookies.jar))

        if 'Set-Cookie' in response.headers:


Note: httpx is the only client that strips this -- impit (_impit.py:50-51) and curl (_curl_impersonate.py:101-102) pass response headers through untouched. So the public HttpResponse.headers carries set-cookie under two clients and never under the third.

Pre-existing, but this PR promotes the deletion from implementation detail to stated mechanism in the new docstring, and it interacts with the semantics just given to persist_cookies_per_session=False: under httpx with persistence off, a Set-Cookie is now observable nowhere -- not in the session, not in the response -- while impit and curl users can still read it. The new cross-client cookie tests are the natural place to pin that divergence, or one line on HttpResponse.headers to document it.

vdusek requested review from Mantisus and Copilot July 31, 2026 09:03

Copilot started reviewing on behalf of vdusek July 31, 2026 09:04 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

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

Mantisus suggested changes Aug 2, 2026

View reviewed changes

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

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

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

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

fix: address Impit cookie review feedback

dccf1e7

Send cookies via Cookie header when persist_cookies_per_session is False so clients stay cached, and use LRUCache.popitem for eviction without fire-and-forget close tasks.

Mantisus suggested changes Aug 3, 2026

View reviewed changes

fix: address remaining Impit review nits

086b115

Simplify cookie header merge, drop redundant LRU popitem, and move client-specific Httpx/Impit tests into dedicated test modules.

Mantisus approved these changes Aug 3, 2026

View reviewed changes

Mantisus requested a review from vdusek August 3, 2026 20:07

vdusek requested changes Aug 4, 2026

View reviewed changes

vdusek changed the title ~~fix: honor session cookies across HTTP client request paths~~ fix(httpx): honor session cookies across HTTP client request paths Aug 4, 2026

Ayush7614 added 2 commits August 4, 2026 16:32

test: assert httpx headers come from a single generate() call

6aaadc4

Mocking get_specific_headers could not catch a regression that mixes Accept and User-Agent from separate fingerprint generations.

vdusek requested changes Aug 4, 2026

View reviewed changes

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

Comment thread tests/unit/http_clients/test_http_clients.py

Ayush7614 added 2 commits August 4, 2026 18:07

fix(httpx): keep clients shared per proxy for session cookies

6b2c8c8

Per-session client caching pinned jars and grew without bound. Re-apply cookies in the transport on each hop (including redirects) instead so one AsyncClient per proxy stays reusable across sessions.

test: keep HttpxHttpClient type for client-cache assertion

bf9a8d0

`async with HttpxHttpClient() as client` widens to HttpClient, so ty rejected access to `_client_by_proxy_url`.

Mantisus suggested changes Aug 4, 2026

View reviewed changes

vdusek mentioned this pull request Aug 10, 2026

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

Open

vdusek reviewed Aug 12, 2026

View reviewed changes

		self._client_by_proxy_url = LRUCache[str \| None, _ClientCacheEntry](maxsize=10)
		self._client_cache = LRUCache[tuple[str \| None, CookieJar \| None], AsyncClient](maxsize=10)



		async def test_client_cache_is_shared_across_sessions(server_url: URL) -> None:
		"""Distinct sessions must reuse one AsyncClient per proxy, not one client per cookie jar."""

	Clients are cached by `(proxy_url, cookie_jar)` — CookieJar hashes by identity so sessions with different
	Clients are cached by `(proxy_url, cookie_jar)`: `CookieJar` hashes by identity so sessions with different

Uh oh!

Conversation

Ayush7614 commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Mantisus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ayush7614 commented Aug 3, 2026

Uh oh!

Mantisus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ayush7614 commented Aug 3, 2026

Uh oh!

Mantisus left a comment

Choose a reason for hiding this comment

Uh oh!

Ayush7614 commented Aug 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ayush7614 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ayush7614 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ayush7614 commented Aug 4, 2026

Uh oh!

Mantisus Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

Ayush7614 commented Jul 30, 2026 •

edited

Loading

Ayush7614 commented Aug 4, 2026 •

edited

Loading

Ayush7614 commented Aug 4, 2026 •

edited

Loading

Mantisus Aug 4, 2026 •

edited

Loading