fix: 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 now send outbound session cookies the same way crawl already did (via _build_request).
Impit (default client): actually honors persist_cookies_per_session (was accepted but ignored), caches clients by (proxy, cookie-jar identity), and closes cached clients on cleanup/eviction.
Httpx fingerprints: Accept / Accept-Language / User-Agent are generated from a single fingerprint profile instead of two independent generate() calls that could mix browser profiles.

Why

context.send_request() silently dropped session cookies under Httpx, breaking auth that worked for navigation. 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 — full file (82 passed)
New coverage: send/stream cookies, persist on/off for curl/httpx/impit, single-fingerprint headers, Impit cleanup cache reset

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.

+        # Ephemeral jars (persist_cookies_per_session=False) must not pollute / thrash the LRU cache.
+        cacheable = cookie_jar is None or self._persist_cookies_per_session
+        cache_key = self._make_cache_key(proxy_url, cookie_jar) if cacheable else None


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', '')

Mantisus · 2026-08-02T21:23:35Z

+        for cookie in session.cookies.jar:
+            jar.set_cookie(deepcopy(cookie))
+        return jar


SessionCookies already does exactly this, so we can use it here.

from crawlee.sessions import SessionCookies return SessionCookies(session.cookies).jar

The deepcopy isn't needed. CookieJar.set_cookie replaces the entry in its own dict rather than mutating the stored Cookie, and extract_cookies always builds new objects. A fresh jar is enough to isolate the session.

Superseded by the Cookie-header approach for persist_cookies_per_session=False — no jar copy is needed anymore.

Mantisus · 2026-08-02T21:35:13Z

+        if cache_key is not None:
+            # Close the client being evicted when the LRU is full, to avoid leaking connections.
+            if len(self._client_cache) >= self._client_cache.maxsize:
+                _evicted_key, evicted_entry = next(iter(self._client_cache.items()))


If I'm not mistaken, you need to use popitem with cachetools.LRUCache

Fixed — eviction now uses self._client_cache.popitem().

Mantisus · 2026-08-02T21:37:14Z

+            # Close the client being evicted when the LRU is full, to avoid leaking connections.
+            if len(self._client_cache) >= self._client_cache.maxsize:
+                _evicted_key, evicted_entry = next(iter(self._client_cache.items()))
+                asyncio.get_running_loop().create_task(self._close_client(evicted_entry['client']))


The task may be garbage collected before it completes.

Removed the fire-and-forget close task entirely.

Mantisus · 2026-08-02T21:44:53Z

+    async def _close_client(self, client: AsyncClient) -> None:
+        # Impit exposes cleanup via the async context manager protocol.
+        result = client.__aexit__(None, None, None)
+        if hasattr(result, '__await__'):
+            await result  # type: ignore[misc]


It's a good guard, but overall, impit should handle potential leaks well thanks to its Rust implementation

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

Mantisus · 2026-08-03T15:53:57Z

+        # UrllibRequest is only used to format Cookie headers via CookieJar; it never opens a connection.
+        request = UrllibRequest(url, headers=dict(headers) if headers else {})  # noqa: S310
+        jar.add_cookie_header(request)
+        return request.get_header('Cookie') or ''


Suggested change

return request.get_header('Cookie') or ''

return request.get_header('Cookie', '')

Mantisus · 2026-08-03T16:15:53Z

+        cookie_header = self._get_cookie_header(session.cookies.jar, url, headers)
+        if cookie_header and 'cookie' not in headers:
+            headers = headers | HttpHeaders({'Cookie': cookie_header})


_get_cookie_headercorrectly handles cases where a Cookie header is present in the headers

Suggested change

cookie_header = self._get_cookie_header(session.cookies.jar, url, headers)

if cookie_header and 'cookie' not in headers:

headers = headers | HttpHeaders({'Cookie': cookie_header})

if cookie_header := self._get_cookie_header(session.cookies.jar, url, headers):

headers = headers | HttpHeaders({'Cookie': cookie_header})

Mantisus · 2026-08-03T16:20:24Z

+        if session is None:
+            return None, headers or None


or None is unnecessary after:

if isinstance(headers, dict) or headers is None: headers = HttpHeaders(headers or {})

Mantisus · 2026-08-03T16:26:26Z

+        if cache_key not in self._client_cache and len(self._client_cache) >= self._client_cache.maxsize:
+            self._client_cache.popitem()


This is dead code now. The LRU cache automatically evicts the items.

Mantisus · 2026-08-03T16:31:42Z

+    assert {cookie['name']: cookie['value'] for cookie in session.cookies.get_cookies_as_dicts()} == {'a': '1'}
+
+
+async def test_httpx_headers_come_from_single_fingerprint() -> None:


Tests that verify the behavior of a specific client should be placed in a separate test file for that client

Mantisus · 2026-08-03T16:31:47Z

+    assert combined['user-agent'] == 'TestAgent/1.0'
+
+
+async def test_impit_cleanup_clears_client_cache(server_url: URL) -> None:


Tests that verify the behavior of a specific client should be placed in a separate test file for that client

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

Mantisus suggested changes Aug 2, 2026

View reviewed changes

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

	return request.get_header('Cookie') or ''
	return request.get_header('Cookie', '')

		if cache_key not in self._client_cache and len(self._client_cache) >= self._client_cache.maxsize:
		self._client_cache.popitem()

		assert {cookie['name']: cookie['value'] for cookie in session.cookies.get_cookies_as_dicts()} == {'a': '1'}


		async def test_httpx_headers_come_from_single_fingerprint() -> None:

		assert combined['user-agent'] == 'TestAgent/1.0'


		async def test_impit_cleanup_clears_client_cache(server_url: URL) -> None:

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!

Mantisus left a comment

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!

Ayush7614 commented Aug 3, 2026

Uh oh!

Mantisus left a comment

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

Ayush7614 commented Jul 30, 2026 •

edited

Loading