Repository navigation
fix(puppeteer-crawler): migrate cookie handling to browser-context API - #3572
Merged
Merged
Conversation
Puppeteer's page-level cookie API (page.cookies / page.setCookie) is deprecated; the successor is the browser-context level API. Update PuppeteerController to use page.browserContext().cookies() and browserContext().setCookie(), mirroring what the Playwright controller already does. This is a semantics change: the page-level API returned cookies scoped to the page's current URL, the context API returns every cookie in the browser context. With Crawlee's default one-context-per-session setup the effect is invisible, but sessions that share a context will see state bleed between their pages. Documented in docs/upgrading/upgrading_v4.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
barjin
approved these changes
Apr 15, 2026
barjin
left a comment
barjin
left a comment
Member
There was a problem hiding this comment.
lgtm, thank you 👍
A small nit below, but let's merge (and keep an eye out).
|
|
||
| protected async _setCookies(page: PuppeteerTypes.Page, cookies: Cookie[]): Promise<void> { | ||
| return page.setCookie(...cookies); | ||
| return page.browserContext().setCookie(...(cookies as PuppeteerTypes.CookieData[])); |
Member
There was a problem hiding this comment.
Note that page.setCookie() fills the cookie's url field if it's missing (source code) based on the page's current url. Using page.browserContext().setCookie() to set a cookie without the url field will end up with the browser rejecting it (e.g., here in Chromium source code).
iirc we append the URLs to the cookies in the crawler internals, so with regular Crawlee usage, the risk is small. Let's keep an eye out in case a report like this comes.
Member
Author
There was a problem hiding this comment.
Should be handled via 9daae96, also mentioned in the upgrading guide
…kies page.setCookie() used to auto-fill the cookie's `url` with the page's current URL when both `url` and `domain` were missing. BrowserContext.setCookie() doesn't, and Chromium rejects cookies with neither. Replicate the old behavior so callers that omit both fields keep working, and document the caveat in the upgrading guide. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
barjin
pushed a commit
that referenced
this pull request
Jul 20, 2026
#3572) Switches \`PuppeteerController._getCookies\` / \`_setCookies\` from the deprecated page-level API (\`page.cookies\` / \`page.setCookie\`) to the browser-context API (\`page.browserContext().cookies\` / \`setCookie\`). This aligns the Puppeteer controller with the Playwright controller, which has always operated at the context level. The page-level API returned cookies scoped to the page's current URL; the context API returns every cookie in the browser context. For the default Crawlee setup (one context per session — the \`useIncognitoPages\` / \`newContextPerSession\` pattern used by \`PuppeteerCrawler\`) there is no visible difference. Users that share a browser context across sessions will see cookies bleed between tabs, which matches playwright-side semantics. Documented in \`docs/upgrading/upgrading_v4.md\`. Surfaced in review of #3569 — that PR is a tooling migration (eslint/biome → oxlint/oxfmt) and originally carried this change as a drive-by "fix a deprecation warning". Splitting it out keeps the migration PR behavior-neutral and gives this behavior change a proper changelog entry. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
B4nan
added a commit
that referenced
this pull request
Aug 12, 2026
#3572) Switches \`PuppeteerController._getCookies\` / \`_setCookies\` from the deprecated page-level API (\`page.cookies\` / \`page.setCookie\`) to the browser-context API (\`page.browserContext().cookies\` / \`setCookie\`). This aligns the Puppeteer controller with the Playwright controller, which has always operated at the context level. The page-level API returned cookies scoped to the page's current URL; the context API returns every cookie in the browser context. For the default Crawlee setup (one context per session — the \`useIncognitoPages\` / \`newContextPerSession\` pattern used by \`PuppeteerCrawler\`) there is no visible difference. Users that share a browser context across sessions will see cookies bleed between tabs, which matches playwright-side semantics. Documented in \`docs/upgrading/upgrading_v4.md\`. Surfaced in review of #3569 — that PR is a tooling migration (eslint/biome → oxlint/oxfmt) and originally carried this change as a drive-by "fix a deprecation warning". Splitting it out keeps the migration PR behavior-neutral and gives this behavior change a proper changelog entry. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
B4nan
added a commit
that referenced
this pull request
Aug 18, 2026
#3572) Switches \`PuppeteerController._getCookies\` / \`_setCookies\` from the deprecated page-level API (\`page.cookies\` / \`page.setCookie\`) to the browser-context API (\`page.browserContext().cookies\` / \`setCookie\`). This aligns the Puppeteer controller with the Playwright controller, which has always operated at the context level. The page-level API returned cookies scoped to the page's current URL; the context API returns every cookie in the browser context. For the default Crawlee setup (one context per session — the \`useIncognitoPages\` / \`newContextPerSession\` pattern used by \`PuppeteerCrawler\`) there is no visible difference. Users that share a browser context across sessions will see cookies bleed between tabs, which matches playwright-side semantics. Documented in \`docs/upgrading/upgrading_v4.md\`. Surfaced in review of #3569 — that PR is a tooling migration (eslint/biome → oxlint/oxfmt) and originally carried this change as a drive-by "fix a deprecation warning". Splitting it out keeps the migration PR behavior-neutral and gives this behavior change a proper changelog entry. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Switches `PuppeteerController._getCookies` / `_setCookies` from the deprecated page-level API (`page.cookies` / `page.setCookie`) to the browser-context API (`page.browserContext().cookies` / `setCookie`). This aligns the Puppeteer controller with the Playwright controller, which has always operated at the context level.
Behavior change
The page-level API returned cookies scoped to the page's current URL; the context API returns every cookie in the browser context. For the default Crawlee setup (one context per session — the `useIncognitoPages` / `newContextPerSession` pattern used by `PuppeteerCrawler`) there is no visible difference. Users that share a browser context across sessions will see cookies bleed between tabs, which matches playwright-side semantics.
Documented in `docs/upgrading/upgrading_v4.md`.
Why split out
Surfaced in review of #3569 — that PR is a tooling migration (eslint/biome → oxlint/oxfmt) and originally carried this change as a drive-by "fix a deprecation warning". Splitting it out keeps the migration PR behavior-neutral and gives this behavior change a proper changelog entry.