fix: treat `CookieJar` write methods as potentially throwing by Copilot · Pull Request #3426 · apify/crawlee

Copilot · 2026-02-20T13:16:47Z

Explore codebase and understand the issue
Fix base-http-client.ts: wrap getCookies() and setCookie() calls in try-catch with console.warn
Fix session.ts: wrap setCookie() in try-catch with this.log.warning
Revert try-catch from getCookies and getCookieString in session.ts
Add comment explaining why the cookie is invalid (domain mismatch) in the existing test
Add new test for setCookie using a garbled string that causes real tough-cookie parse failure
Run existing tests to verify no regressions (25 tests pass)

Original prompt

This section details on the original issue you should resolve

<issue_title>v4: tough-cookie's CookieJars throw on invalid Set-Cookie response contents</issue_title>
<issue_description>As seen in apify/actor-scraper#236, the internal CookieJar can throw on a deformed Set-Cookie response from the server.

This error is now not caught anywhere and will kill the entire script if used, e.g., from a helper function (like discoverValidSitemaps).

We should treat all CookieJar methods as potentially throwing and only print warnings on thrown errors.</issue_description>

<agent_instructions>Make minimal changes to achieve the goal. Use conventional commit specs for the commit messages and the PR title. Do not use excessive comments; only add comments where strictly necessary for code readability.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes v4: tough-cookie's CookieJars throw on invalid Set-Cookie response contents #3425

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

barjin · 2026-02-20T15:17:01Z

packages/http-client/package.json

        "access": "public"
    },
    "dependencies": {
+        "@apify/log": "^2.5.32",


The only part I'm unsure about is adding @apify/log as a @crawlee/http-client dependency... but well, we gotta log somehow.

I'm open to ideas here.

#3399 should resolve this problem, shouldn't it?

@crawlee/http-client should have no dependencies on other Crawlee packages (definitely not @crawlee/core), as it's used from @crawlee/utils - importing @crawlee/core would create a circular dependency. It's a bit of a predicament :/

nikitachapovskii-dev · 2026-02-20T16:09:17Z

packages/http-client/src/base-http-client.ts

        timeout?: number;
    } {
        const proxyUrl = options?.proxyUrl ?? options?.session?.proxyInfo?.url;
        const cookieJar = options?.cookieJar ?? options?.session?.cookieJar ?? new CookieJar();


Seems like it still creates cookie jar if not passed 🤔
then on a line 119 and 127 the jar is used in each request: this.applyCookies(currentRequest, cookieJar) or do I miss smth?

upd: took a closer look at the implementation, and the solution looks correct to me.

janbuchar · 2026-02-23T13:22:22Z

packages/core/src/session_pool/session.ts

+            const cookies = this.cookieJar.getCookiesSync(url);
+            return cookies.map((c) => toughCookieToBrowserPoolCookie(c));
+        } catch (e) {
+            this.log.warning('Could not get cookies from cookie jar.', { url, error: (e as Error).message });


For an error to be thrown here, the cookie jar would have to accept a malformed cookie string first and then somehow fail at retrieving it. Is there a realistic way this could happen? If not, I'd perhaps re-throw the error in this case.

Yeah, good point, all the errors I could find in the source are parameter-value related, so probably user's fault.

@copilot revert both getCookies and getCookieString hunks.

Done in fd55f27. Reverted both getCookies and getCookieString to their original form without try-catch.

janbuchar · 2026-02-23T13:22:46Z

packages/core/src/session_pool/session.ts

+        try {
+            return this.cookieJar.getCookieStringSync(url, {});
+        } catch (e) {
+            this.log.warning('Could not get cookie string.', { url, error: (e as Error).message });


Same as https://github.com/apify/crawlee/pull/3426/changes#r2840843658

janbuchar · 2026-02-23T13:23:45Z

packages/http-client/package.json

        "access": "public"
    },
    "dependencies": {
+        "@apify/log": "^2.5.32",


#3399 should resolve this problem, shouldn't it?

janbuchar · 2026-02-23T13:26:55Z

test/core/session_pool/session.test.ts


+    test('setCookies will log warning (not throw) on invalid cookies', () => {
+        const url = 'https://www.example.com';
+        const cookies = [{ name: 'cookie1', value: 'my-cookie', domain: 'abc.different.domain' }];


Would be nice to explain why this cookie is invalid (it's for a different domain than the one that sets it). Also, can we test calling setCookie with a malformed raw cookie string?

@copilot work on this, add a comment and a new test with a malformed cookie string.

Done in 36d82f7. Added a comment explaining that abc.different.domain doesn't match the request URL (causing tough-cookie to reject it), and added a new test that spies on setCookieSync to simulate a throw and verifies setCookie handles it gracefully without re-throwing.

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

janbuchar · 2026-02-23T14:04:39Z

test/core/session_pool/session.test.ts

+        vitest.spyOn(session.cookieJar, 'setCookieSync').mockImplementation(() => {
+            throw new Error('malformed cookie');
+        });


@copilot is the mock implementation necessary? If the cookie content is in fact malformed, the actual implementation of setCookieSync should detect it, right? The current test only seems to ensure that the method catches exceptions thrown from setCookieSync.

@barjin tell your agent to get cracking please 😁

@copilot chop chop, back to work

Good call — the mock wasn't needed. A cookie with domain=evil.com set against https://www.example.com causes the real setCookieSync to throw. Updated in b81fca2.

But it throws because the cookie is for a different domain. Try to make an actually malformed cookie string. @copilot

@copilot make a test where the cookie string is a garbled mess (not a well-formed cookie string). This shouldn't fail either.

Updated in 636757c — now uses 'garbled!!!@#$%nonsense' which causes the real tough-cookie parser to throw "Cookie failed to parse".

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

Initial plan

370cb02

Copilot AI assigned Copilot and barjin Feb 20, 2026

Copilot started work on behalf of barjin February 20, 2026 13:17 View session

Copilot AI and others added 2 commits February 20, 2026 13:23

fix: treat all CookieJar methods as potentially throwing

544376c

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

fix: treat all CookieJar methods as potentially throwing

d4cf37a

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix CookieJar to handle invalid Set-Cookie responses~~ fix: treat all CookieJar methods as potentially throwing Feb 20, 2026

Copilot AI requested a review from barjin February 20, 2026 13:24

Copilot finished work on behalf of barjin February 20, 2026 13:24

barjin added 2 commits February 20, 2026 16:03

chore: log warnings, guard more methods

6b9e246

chore: run formatter

098b7e4

barjin changed the title ~~fix: treat all CookieJar methods as potentially throwing~~ fix: treat all CookieJar methods as potentially throwing Feb 20, 2026

barjin marked this pull request as ready for review February 20, 2026 15:11

barjin assigned janbuchar and nikitachapovskii-dev and unassigned janbuchar and nikitachapovskii-dev Feb 20, 2026

barjin requested review from janbuchar and nikitachapovskii-dev February 20, 2026 15:11

barjin reviewed Feb 20, 2026

View reviewed changes

nikitachapovskii-dev approved these changes Feb 23, 2026

View reviewed changes

janbuchar reviewed Feb 23, 2026

View reviewed changes

Copilot started work on behalf of barjin February 23, 2026 13:39 View session

test: add comment and malformed cookie test for setCookie

36d82f7

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

Copilot AI requested a review from barjin February 23, 2026 13:45

Copilot finished work on behalf of barjin February 23, 2026 13:45

Copilot started work on behalf of barjin February 23, 2026 13:46 View session

fix: revert try-catch from getCookies and getCookieString in session

fd55f27

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

Copilot AI changed the title ~~fix: treat all CookieJar methods as potentially throwing~~ fix: treat CookieJar write methods as potentially throwing Feb 23, 2026

Copilot finished work on behalf of barjin February 23, 2026 13:48

janbuchar reviewed Feb 23, 2026

View reviewed changes

Copilot started work on behalf of barjin February 23, 2026 14:11 View session

test: use real setCookieSync throw instead of mock

b81fca2

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

Copilot finished work on behalf of barjin February 23, 2026 14:14

Copilot started work on behalf of barjin February 23, 2026 14:23 View session

test: use garbled string to trigger real cookie parse failure

636757c

Co-authored-by: barjin <61918049+barjin@users.noreply.github.com>

Copilot finished work on behalf of barjin February 23, 2026 14:26

janbuchar approved these changes Feb 23, 2026

View reviewed changes

barjin approved these changes Feb 23, 2026

View reviewed changes

barjin merged commit 7edc67c into v4 Feb 23, 2026
7 checks passed

barjin deleted the copilot/handle-cookiejar-errors branch February 23, 2026 14:40

Conversation

Copilot AI commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comments on the Issue (you are @copilot in this section)

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!

Copilot AI Feb 23, 2026

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!

Copilot AI Feb 23, 2026

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!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

janbuchar Feb 23, 2026 • edited by barjin 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!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Feb 20, 2026 •

edited

Loading

janbuchar Feb 23, 2026 •

edited by barjin

Loading