From c0769af181bf201693cdcfbd2b16f8f708221ff7 Mon Sep 17 00:00:00 2001 From: germondai Date: Sun, 9 Aug 2026 18:15:48 +0200 Subject: [PATCH] fix(browser): bound memory with rolling recycling --- CHANGELOG.md | 1 + README.md | 2 +- apps/api/src/config.ts | 2 + apps/docs/architecture/browser-pool.md | 4 +- apps/docs/deployment/docker-compose.md | 2 +- apps/docs/getting-started/configuration.md | 4 +- packages/browser/src/index.ts | 2 +- packages/browser/src/pool.ts | 166 +++++++++++++++--- packages/browser/tests/freshContext.test.ts | 31 +++- packages/browser/tests/pool.test.ts | 91 ++++++++-- packages/tiers/src/orchestrator.ts | 22 --- packages/tiers/src/tiers/3.ts | 19 +- packages/tiers/src/tiers/4.ts | 15 +- packages/tiers/tests/orchestrator.test.ts | 32 ---- .../tests/temporaryContextCleanup.test.ts | 45 +++++ packages/types/src/index.ts | 3 +- 16 files changed, 319 insertions(+), 122 deletions(-) delete mode 100644 packages/tiers/tests/orchestrator.test.ts create mode 100644 packages/tiers/tests/temporaryContextCleanup.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 61f963e..b7cd131 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Translate Prowlarr's serialized `headers.contentType` metadata at the FlareSolverr `/v1` compatibility boundary and discard `contentLength`, allowing form POST requests to enter the scraper pipeline (#50). +- Bound Camoufox memory growth by counting every Tier 3/4 temporary context and rolling-replacing browsers at `BROWSER_RECYCLE_AFTER_CONTEXTS`, while keeping existing capacity available during warm-up. Replacement launches are serialized, cleanup is timeout-bounded, and failed launches retain the usable browser (#52). ## [1.3.1] - 2026-08-02 diff --git a/README.md b/README.md index 0408d46..f538ebb 100644 --- a/README.md +++ b/README.md @@ -386,7 +386,7 @@ for pool and mounted-file examples. | -------------------------------- | ------------------------ | ----------------------------------------------------------------------------------- | | `BROWSER_POOL_SIZE` | `3` | Warm Camoufox Firefox instances | | `BROWSER_ACQUIRE_TIMEOUT_MS` | `15000` | How long `acquire()` polls for a free browser before HTTP 429 is returned | -| `BROWSER_RECYCLE_AFTER_CONTEXTS` | `8` | Recycle a browser after this many `blocked`/`needs-js` outcomes; set `0` to disable | +| `BROWSER_RECYCLE_AFTER_CONTEXTS` | `8` | Rolling-replace after this many Tier 3/4 contexts; set `0` to disable | | `BROWSER_CONTENT_PROCESSES` | `2` | Cap Firefox content processes per browser (`dom.ipc.processCount`); lowers RAM/CPU | | `SESSION_TTL_SECONDS` | `3600` | Redis session cache TTL (seconds) | | `REDIS_URL` | `redis://localhost:6379` | Redis connection string | diff --git a/apps/api/src/config.ts b/apps/api/src/config.ts index 4cd9b4b..1ba4a9e 100644 --- a/apps/api/src/config.ts +++ b/apps/api/src/config.ts @@ -8,6 +8,8 @@ export const POOL_SIZE = Number(process.env.BROWSER_POOL_SIZE ?? "3") // Tune lower for fast-fail feedback in dev; tune higher for very heavy upstream targets. export const ACQUIRE_TIMEOUT_MS = Number(process.env.BROWSER_ACQUIRE_TIMEOUT_MS ?? "15000") export const SESSION_TTL = Number(process.env.SESSION_TTL_SECONDS ?? "3600") +// Rolling-replace a browser after this many Tier 3/4 temporary contexts. Every +// creation counts regardless of outcome; 0 disables periodic replacement. export const RECYCLE_AFTER_TEMPORARY_CONTEXTS = Number(process.env.BROWSER_RECYCLE_AFTER_CONTEXTS ?? "8") // Caps Firefox content processes per browser. Default `2` keeps thread/RAM footprint // minimal while still allowing CF/Imperva challenges to resolve. Raise if specific diff --git a/apps/docs/architecture/browser-pool.md b/apps/docs/architecture/browser-pool.md index f8def0a..73048c7 100644 --- a/apps/docs/architecture/browser-pool.md +++ b/apps/docs/architecture/browser-pool.md @@ -61,9 +61,9 @@ When `acquireTimeoutMs` elapses, the API surfaces the rejection as **HTTP 429** Tier 3 and Tier 4 create short-lived isolated contexts for fresh challenge solves and proxy escalation. Those contexts are closed by the tier code, but long-running Firefox/Camoufox processes can still retain child content processes after repeated solves. Two complementary defenses bound this growth: 1. **`contentProcesses` (default `2`)** caps Firefox content processes per browser at launch via the `dom.ipc.processCount` Firefox pref. This is the primary defense — bounds thread/RAM growth at the source regardless of context churn. -2. **`recycleAfterTemporaryContexts` (default `8`)** is now **recycle-on-suspect**: the orchestrator only flags a browser for recycle when Tier 3/Tier 4 returns `blocked` / `needs-js`. Successful solves preserve cookies, `cf_clearance`, and warm fingerprint state. Set `BROWSER_RECYCLE_AFTER_CONTEXTS=0` to disable this recycling. +2. **`recycleAfterTemporaryContexts` (default `8`)** counts every Tier 3/Tier 4 context creation, including successful, timed-out, errored, and blocked attempts. At the threshold the pool warms a replacement while the current browser remains available, swaps it in when idle, and then closes the retired browser. Only one replacement is warmed across the pool at a time, so the brief memory peak is bounded to one additional browser. Set `BROWSER_RECYCLE_AFTER_CONTEXTS=0` to disable this recycling. -See issue #13 (original bug), #17 (recycle-on-suspect trade-off discussion), and the [configuration docs](/getting-started/configuration#browser_recycle_after_contexts) for tuning. +See issues #13, #17, and #52, plus the [configuration docs](/getting-started/configuration#browser_recycle_after_contexts) for tuning. ## Self-healing diff --git a/apps/docs/deployment/docker-compose.md b/apps/docs/deployment/docker-compose.md index b67eeed..db30efb 100644 --- a/apps/docs/deployment/docker-compose.md +++ b/apps/docs/deployment/docker-compose.md @@ -72,7 +72,7 @@ See [Standalone Containers → Older CPUs & Synology NAS](/deployment/standalone | -------------------------------- | -------------------- | ----------------------------------------------------------------------- | | `BROWSER_POOL_SIZE` | `3` | Warm browsers; supplied minimal/cached Compose files override this to `1` | | `BROWSER_ACQUIRE_TIMEOUT_MS` | `15000` | How long `acquire()` polls for a free browser before returning HTTP 429 | -| `BROWSER_RECYCLE_AFTER_CONTEXTS` | `8` | Restart after this many blocked/needs-js outcomes; set `0` to disable | +| `BROWSER_RECYCLE_AFTER_CONTEXTS` | `8` | Rolling-replace after this many Tier 3/4 contexts; `0` disables it | | `REDIS_URL` | `redis://redis:6379` | Redis connection (set automatically in compose) | | `PROXY_URL` | — | Optional Tier 3 datacenter proxy or pool | | `RESIDENTIAL_PROXY_URL` | — | Enables Tier 4 proxy escalation | diff --git a/apps/docs/getting-started/configuration.md b/apps/docs/getting-started/configuration.md index aa39d2c..d7f0f0e 100644 --- a/apps/docs/getting-started/configuration.md +++ b/apps/docs/getting-started/configuration.md @@ -67,10 +67,10 @@ When the timeout fires, both `/v1` and `/scrape` return **HTTP 429** with the Fl **Default:** `8` -How many `blocked` / `needs-js` outcomes a pooled browser can produce before TRAWL restarts the full browser process. The recycle counter only increments when Tier 3 or Tier 4 reports the upstream actively rejected the browser's profile — successful solves preserve cookies, `cf_clearance`, and warm fingerprint state. This avoids the HTTP-429 storm that occurred when the pool preemptively recycled mid-flight (issue #17). +How many Tier 3 or Tier 4 temporary contexts a pooled browser can create before TRAWL rolling-replaces the full browser process. Every context counts, regardless of whether the attempt succeeds, times out, errors, or is blocked. TRAWL warms one replacement while the existing browser remains available, installs it when the entry is idle, then closes the retired browser. This briefly raises the pool by one browser, and replacements are serialized pool-wide to bound that peak. ```ini -BROWSER_RECYCLE_AFTER_CONTEXTS=8 # default - recycle after 8 blocked/needs-js outcomes +BROWSER_RECYCLE_AFTER_CONTEXTS=8 # default - replace after 8 Tier 3/4 contexts BROWSER_RECYCLE_AFTER_CONTEXTS=0 # disable browser recycling entirely ``` diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index cd86e05..a89561e 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -1,5 +1,5 @@ export { FINGERPRINT, FINGERPRINT_POOL } from "./fingerprint" export type { BrowserHandle } from "./pool" -export { BrowserPool, newFreshContext, PoolExhaustedError } from "./pool" +export { BrowserPool, closeTemporaryContext, newFreshContext, PoolExhaustedError } from "./pool" export { type PlaywrightProxy, toPlaywrightProxy } from "./proxy" export { SessionCache } from "./session" diff --git a/packages/browser/src/pool.ts b/packages/browser/src/pool.ts index 010b6ab..c67effa 100644 --- a/packages/browser/src/pool.ts +++ b/packages/browser/src/pool.ts @@ -70,6 +70,7 @@ interface PoolEntry extends PoolBrowser { stallAt?: number restartReason?: string restarting?: boolean + replacementRequested?: string fingerprint: (typeof FINGERPRINT_POOL)[number] } @@ -90,6 +91,8 @@ export class BrowserPool { private healthInterval?: ReturnType private abandonedLaunches = 0 private maxAbandonedLaunches: number + private replacementRunning = false + private shuttingDown = false constructor({ poolSize, @@ -316,9 +319,13 @@ export class BrowserPool { fingerprint: entry.fingerprint, // Captured lease: a reclaimed request that resumes later must not attribute its // failure to the replacement browser now occupying this entry. - noteTemporaryContext: ((lease: number) => (reason: string) => { + noteTemporaryContext: ((lease: number) => () => { if (entry.lease !== lease) return - this.noteTemporaryContext(entry, reason) + this.noteTemporaryContext(entry) + })(entry.lease), + requestBrowserReplacement: ((lease: number) => (reason: string) => { + if (entry.lease !== lease) return + this.requestRollingReplacement(entry, reason) })(entry.lease), }) return true @@ -350,15 +357,79 @@ export class BrowserPool { return available[0] } - private noteTemporaryContext(entry: PoolEntry, reason: string): void { + private noteTemporaryContext(entry: PoolEntry): void { if (this.recycleAfterTemporaryContexts <= 0) return - // Skip if the entry is already being recycled — avoids incrementing the counter - // against a dead entry and racing with the in-flight restartEntry. - if (entry.restarting) return - entry.temporaryContextUses++ if (entry.temporaryContextUses >= this.recycleAfterTemporaryContexts) { - entry.restartReason = `${reason}; ${entry.temporaryContextUses} temporary contexts used` + this.requestRollingReplacement(entry, `${entry.temporaryContextUses} temporary contexts created`) + } + } + + private requestRollingReplacement(entry: PoolEntry, reason: string): void { + if (entry.restarting) return + entry.replacementRequested ??= reason + if (!entry.busy) void this.runNextRollingReplacement() + } + + // Periodic recycling is rolling: the existing browser remains usable while its + // replacement starts. A pool-wide lock bounds the temporary peak to one browser. + private async runNextRollingReplacement(): Promise { + if (this.replacementRunning || this.shuttingDown) return + const entry = this.entries.find((candidate) => candidate.replacementRequested && !candidate.restarting) + if (!entry) return + this.replacementRunning = true + const reason = entry.replacementRequested as string + entry.replacementRequested = undefined + console.warn(`[pool] browser ${entry.id} warming replacement: ${reason}`) + + let replacement: { browser: Browser; context: BrowserContext } | undefined + try { + if (this.abandonedLaunches >= this.maxAbandonedLaunches) { + throw new Error(`${this.abandonedLaunches} launches already abandoned; not starting another until one settles`) + } + replacement = await this.launchWithin(entry.fingerprint, this.launchTimeoutMs) + + // Acquires continue during the launch. Wait to swap until the current lease is + // released, without marking the entry unavailable in the meantime. + while (entry.busy && !this.shuttingDown) { + await new Promise((resolve) => setTimeout(resolve, this.pollIntervalMs)) + } + if (this.shuttingDown) return + + const retiredBrowser = entry.browser + const retiredContext = entry.context + const pendingPageCloses = entry.pendingPageCloses + entry.browser = replacement.browser + entry.context = replacement.context + replacement = undefined + entry.pendingPageCloses = undefined + entry.temporaryContextUses = 0 + // Contexts created while this replacement was warming belong to the browser + // being retired. They may have re-asserted the threshold request, but the new + // browser starts clean and must not immediately replace itself again. + entry.replacementRequested = undefined + entry.restartCount++ + entry.healthy = true + entry.lease++ + console.log(`[pool] browser ${entry.id} rolling replacement installed (total: ${entry.restartCount})`) + + await settleWithin(pendingPageCloses ? () => pendingPageCloses : undefined, this.closeTimeoutMs) + await settleWithin(() => retiredContext?.close(), this.closeTimeoutMs) + await settleWithin(() => retiredBrowser?.close(), this.closeTimeoutMs) + } catch (err) { + // A failed warm-up never disturbs the browser currently serving the entry. + entry.replacementRequested ??= reason + console.error(`[pool] browser ${entry.id} failed to warm replacement:`, err) + } finally { + if (replacement) { + await settleWithin(() => replacement?.context?.close(), this.closeTimeoutMs) + await settleWithin(() => replacement?.browser?.close(), this.closeTimeoutMs) + } + this.replacementRunning = false + // Do not spin immediately on a failed launch. Health checks and subsequent + // releases provide bounded retry opportunities. + const next = this.entries.find((candidate) => candidate.replacementRequested && candidate !== entry) + if (next) void this.runNextRollingReplacement() } } @@ -388,6 +459,8 @@ export class BrowserPool { // acquiring this entry in the window before the browser is actually torn down. // restartEntry waits on pendingPageCloses itself. void this.restartEntry(entry, reason) + } else if (entry.replacementRequested) { + void this.runNextRollingReplacement() } } @@ -421,6 +494,7 @@ export class BrowserPool { await this.restartEntry(entry, "browser disconnected") } else { entry.healthy = true + if (entry.replacementRequested) void this.runNextRollingReplacement() } } } @@ -574,6 +648,7 @@ export class BrowserPool { } async shutdown(): Promise { + this.shuttingDown = true if (this.healthInterval) { clearInterval(this.healthInterval) delete this.healthInterval @@ -588,27 +663,68 @@ export class BrowserPool { } } -// biome-ignore lint/suspicious/noExplicitAny: preserves the caller's Playwright or Patchright context type -export const newFreshContext = async (browser: any, options?: { proxy?: string }): Promise => { +// These preserve the caller's Playwright or Patchright types; the two libraries are +// structurally incompatible even though both expose the same runtime methods. +// biome-ignore lint/suspicious/noExplicitAny: see comment above +type FreshBrowser = any +// biome-ignore lint/suspicious/noExplicitAny: see comment above +type FreshContext = any +export const newFreshContext = async ( + browser: FreshBrowser, + options?: { proxy?: string; onCreated?: () => void; requestReplacement?: (reason: string) => void }, +): Promise => { const context = await browser.newContext({ viewport: null, ...(options?.proxy ? { proxy: toPlaywrightProxy(options.proxy) } : {}), }) - await context.addInitScript(() => { - window.onerror = () => true - window.addEventListener( - "unhandledrejection", - (e: PromiseRejectionEvent) => { - e.preventDefault() - }, - true, + options?.onCreated?.() + try { + await context.addInitScript(() => { + window.onerror = () => true + window.addEventListener( + "unhandledrejection", + (e: PromiseRejectionEvent) => { + e.preventDefault() + }, + true, + ) + const _orig = Element.prototype.attachShadow + Element.prototype.attachShadow = function (init: ShadowRootInit) { + const r = _orig.call(this, init) + Object.defineProperty(this, "shadowRootUnl", { configurable: true, value: r }) + return r + } + }) + return context + } catch (err) { + await closeTemporaryContext( + context, + options?.requestReplacement, + "temporary context initialization cleanup timed out", + CLOSE_TIMEOUT_MS, ) - const _orig = Element.prototype.attachShadow - Element.prototype.attachShadow = function (init: ShadowRootInit) { - const r = _orig.call(this, init) - Object.defineProperty(this, "shadowRootUnl", { configurable: true, value: r }) - return r - } + throw err + } +} + +export const closeTemporaryContext = async ( + context: { close: AsyncAction } | undefined, + requestReplacement?: (reason: string) => void, + reason = "temporary context cleanup timed out", + timeoutMs = 5_000, +): Promise => { + if (!context) return + let settled = false + let timer: ReturnType | undefined + await Promise.race([ + settle(() => context.close()).then(() => { + settled = true + }), + new Promise((resolve) => { + timer = setTimeout(resolve, timeoutMs) + }), + ]).finally(() => { + if (timer) clearTimeout(timer) }) - return context + if (!settled) requestReplacement?.(reason) } diff --git a/packages/browser/tests/freshContext.test.ts b/packages/browser/tests/freshContext.test.ts index abd49f6..dd6ba4b 100644 --- a/packages/browser/tests/freshContext.test.ts +++ b/packages/browser/tests/freshContext.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { newFreshContext } from "../src/pool" +import { closeTemporaryContext, newFreshContext } from "../src/pool" import { toPlaywrightProxy } from "../src/proxy" describe("newFreshContext", () => { @@ -36,4 +36,33 @@ describe("newFreshContext", () => { password: "p:ass", }) }) + + test("closes a partially initialized context", async () => { + let closed = false + let created = 0 + const browser = { + newContext: async () => ({ + addInitScript: async () => { + throw new Error("init failed") + }, + close: async () => { + closed = true + }, + }), + } + await expect(newFreshContext(browser, { onCreated: () => created++ })).rejects.toThrow("init failed") + expect(created).toBe(1) + expect(closed).toBe(true) + }) + + test("cleanup timeout requests a rolling replacement", async () => { + let reason = "" + await closeTemporaryContext( + { close: () => new Promise(() => {}) }, + (value) => (reason = value), + "cleanup wedged", + 10, + ) + expect(reason).toBe("cleanup wedged") + }) }) diff --git a/packages/browser/tests/pool.test.ts b/packages/browser/tests/pool.test.ts index 6600ffb..d842922 100644 --- a/packages/browser/tests/pool.test.ts +++ b/packages/browser/tests/pool.test.ts @@ -126,11 +126,7 @@ describe("BrowserPool recycling", () => { expect(browsers).toHaveLength(1) }) - test("successful acquires do NOT trigger recycle (recycle driven by orchestrator, not pool)", async () => { - // Documents the contract: the pool itself does NOT decide when to recycle based - // on temporary-context count. The orchestrator decides (by calling - // noteTemporaryContext only on blocked/needs-js outcomes). This test verifies - // that the pool, given N successful acquires, never recycles on its own. + test("counts every reported temporary context independent of outcome", async () => { const { factory, browsers } = makeFactory() const pool = createPool({ @@ -141,15 +137,76 @@ describe("BrowserPool recycling", () => { await pool.init() - // Simulate 10 "successful" Tier 3 attempts that the orchestrator does NOT flag. - // (No noteTemporaryContext calls.) Pool should never recycle. - for (let i = 0; i < 10; i++) { + for (const _outcome of ["success", "timeout"]) { const handle = await pool.acquire("example.com") + handle.noteTemporaryContext?.() pool.release(handle.id) } - expect(pool.getStats().restarts).toBe(0) - expect(browsers).toHaveLength(1) + await waitFor(() => pool.getStats().restarts === 1) + expect(browsers).toHaveLength(2) + }) + + test("pool size 1 stays acquirable while a replacement is launching", async () => { + const { factory: baseFactory } = makeFactory() + let finishLaunch: (() => void) | undefined + let launches = 0 + const factory = async () => { + launches++ + if (launches === 2) await new Promise((resolve) => (finishLaunch = resolve)) + return baseFactory() + } + const pool = createPool({ + poolSize: 1, + recycleAfterTemporaryContexts: 1, + acquireTimeoutMs: 50, + browserFactory: factory, + }) + await pool.init() + const first = await pool.acquire("example.com") + first.noteTemporaryContext?.() + pool.release(first.id, first.lease) + await waitFor(() => launches === 2) + + const duringWarmup = await pool.acquire("example.com") + expect(pool.getStats().live).toBe(1) + // This context belongs to the incumbent that is about to be retired. It must not + // schedule a second replacement after the warmed browser is installed. + duringWarmup.noteTemporaryContext?.() + finishLaunch?.() + pool.release(duringWarmup.id, duringWarmup.lease) + await waitFor(() => pool.getStats().restarts === 1) + const afterInstall = await pool.acquire("example.com") + pool.release(afterInstall.id, afterInstall.lease) + await new Promise((resolve) => setTimeout(resolve, 20)) + expect(launches).toBe(2) + }) + + test("warms only one replacement across the pool", async () => { + const { factory: baseFactory } = makeFactory() + let finishFirstReplacement: (() => void) | undefined + let launches = 0 + const factory = async () => { + launches++ + if (launches === 3) await new Promise((resolve) => (finishFirstReplacement = resolve)) + return baseFactory() + } + const pool = createPool({ poolSize: 2, recycleAfterTemporaryContexts: 1, browserFactory: factory }) + await pool.init() + + const first = await pool.acquire("one.example") + const second = await pool.acquire("two.example") + first.noteTemporaryContext?.() + second.noteTemporaryContext?.() + pool.release(first.id, first.lease) + pool.release(second.id, second.lease) + + await waitFor(() => launches === 3) + await new Promise((resolve) => setTimeout(resolve, 20)) + expect(launches).toBe(3) + finishFirstReplacement?.() + await waitFor(() => launches === 4) + await waitFor(() => pool.getStats().restarts === 2) }) test("contentProcesses option is stored without crashing", async () => { @@ -289,7 +346,7 @@ describe("BrowserPool wedge recovery", () => { expect(pool.getStats().available).toBe(1) }) - test("a launch that never resolves fails the restart instead of hanging it", async () => { + test("a timed-out rolling replacement leaves the existing browser usable", async () => { let launches = 0 const factory = async () => { launches++ @@ -330,13 +387,11 @@ describe("BrowserPool wedge recovery", () => { handle.noteTemporaryContext?.("tier4 blocked") pool.release(handle.id, handle.lease) - // Wait for the restart to actually be in flight (entry detached, no capacity) before - // asserting recovery — otherwise this passes on the pre-restart state and proves - // nothing. - await waitFor(() => pool.getStats().live === 0, 2000) - // The hung launch is then abandoned, `restarting` clears, and the next health-check - // tick retries the entry from scratch — so the pool heals rather than wedging here. - await waitFor(() => pool.getStats().live === 1, 3000) + await new Promise((resolve) => setTimeout(resolve, 80)) + expect(pool.getStats().live).toBe(1) + const stillUsable = await pool.acquire("example.com") + pool.release(stillUsable.id, stillUsable.lease) + await waitFor(() => pool.getStats().restarts === 1, 3000) await pool.shutdown() }) diff --git a/packages/tiers/src/orchestrator.ts b/packages/tiers/src/orchestrator.ts index f6829ea..117d78f 100644 --- a/packages/tiers/src/orchestrator.ts +++ b/packages/tiers/src/orchestrator.ts @@ -27,14 +27,6 @@ export class ScrapeError extends Error { } } -// True when a Tier 3/4 result indicates the browser's profile was actively rejected -// by the upstream (CF / Imperva / etc.). On these outcomes the orchestrator flags the -// pool for a future recycle; on every other outcome (success, transient error, timeout) -// the browser is kept warm so cookies + cf_clearance survive. -export function shouldFlagForRecycle(status: TierResult["status"]): boolean { - return status === "blocked" || status === "needs-js" -} - export interface OrchestratorDeps { acquireBrowser(domain: string, budgetMs?: number): Promise releaseBrowser(id: number, lease?: number): void @@ -158,14 +150,6 @@ export async function scrape(req: ScrapeRequest, deps: OrchestratorDeps): Promis const remaining3 = maxTimeout - (Date.now() - totalStart) t3 = await runTier3(req.url, handle, remaining3, proxy3, sanitizedHeaders, req.method, req.body) - // Only flag the pool for a recycle when the upstream actively rejected the - // browser's profile ("blocked"/"needs-js"). Successful solves preserve cookies, - // cf_clearance, and TLS fingerprint — recycling after success would force a - // costly cold start on the next request to the same domain. - if (shouldFlagForRecycle(t3.status)) { - handle.noteTemporaryContext?.(`tier3 ${t3.status}`) - } - const pool = deps.proxyPool if (t3.status !== "blocked" || req.proxy || !proxy3 || !pool || attempt + 1 >= MAX_PROXY_ATTEMPTS) break pool.markBad(proxy3) @@ -225,12 +209,6 @@ export async function scrape(req: ScrapeRequest, deps: OrchestratorDeps): Promis const remaining4 = maxTimeout - (Date.now() - totalStart) t4 = await runTier4Lazy(req.url, handle, remaining4, proxy4, sanitizedHeaders, req.method, req.body) - // Mirror Tier 3's recycle-on-suspect policy — only flag when the upstream - // explicitly rejected the browser's profile. - if (shouldFlagForRecycle(t4.status)) { - handle.noteTemporaryContext?.(`tier4 ${t4.status}`) - } - const pool = deps.residentialProxyPool if (t4.status !== "blocked" || req.proxy || !pool || attempt + 1 >= MAX_PROXY_ATTEMPTS) break pool.markBad(proxy4) diff --git a/packages/tiers/src/tiers/3.ts b/packages/tiers/src/tiers/3.ts index 2b95d7f..ecae817 100644 --- a/packages/tiers/src/tiers/3.ts +++ b/packages/tiers/src/tiers/3.ts @@ -1,5 +1,5 @@ import type { BrowserHandle } from "@trawl/browser" -import { FINGERPRINT, newFreshContext } from "@trawl/browser" +import { closeTemporaryContext, FINGERPRINT, newFreshContext } from "@trawl/browser" import type { Cookie, TierResult } from "@trawl/types" import { solvePageCaptchas } from "../solvers" import { waitForAkamaiResolution } from "../utils/akamaiWait" @@ -49,10 +49,15 @@ export async function runTier3( // engine state) that CF's behavioral analysis scores as suspicious — resulting in 40s // challenge evaluation. A fresh context with no prior state gets managed-mode treatment: // CF evaluates in under 1s and the challenge resolves in 3-4s total. - const freshCtx = await newFreshContext(handle.browser, { proxy: proxyUrl }) - const page = await freshCtx.newPage() + let freshCtx: Awaited> | undefined try { + freshCtx = await newFreshContext(handle.browser, { + proxy: proxyUrl, + onCreated: handle.noteTemporaryContext, + requestReplacement: handle.requestBrowserReplacement, + }) + const page = await freshCtx.newPage() if ((extraHeaders && Object.keys(extraHeaders).length > 0) || method === "POST") { await page.route(url, (route: RouteLike) => { route.continue(routeContinueOverrides(route, extraHeaders, method, body)) @@ -194,10 +199,8 @@ export async function runTier3( reason: err instanceof Error ? err.message : String(err), } } finally { - await page.close().catch(() => {}) - // Bound context.close() with a timeout — Camoufox/Firefox occasionally hangs on - // close when a content process is wedged, leaking the process until the next - // browser recycle. 5s is well above the typical <500ms close path. - await Promise.race([freshCtx.close(), new Promise((resolve) => setTimeout(resolve, 5000))]).catch(() => {}) + // Closing the context closes all of its pages. If Firefox wedges during cleanup, + // ask the pool to replace this browser as soon as the lease is released. + await closeTemporaryContext(freshCtx, handle.requestBrowserReplacement, "tier3 context cleanup timed out") } } diff --git a/packages/tiers/src/tiers/4.ts b/packages/tiers/src/tiers/4.ts index 1b52838..a9a8f0b 100644 --- a/packages/tiers/src/tiers/4.ts +++ b/packages/tiers/src/tiers/4.ts @@ -1,5 +1,5 @@ import type { BrowserHandle } from "@trawl/browser" -import { FINGERPRINT, newFreshContext } from "@trawl/browser" +import { closeTemporaryContext, FINGERPRINT, newFreshContext } from "@trawl/browser" import type { Cookie, TierResult } from "@trawl/types" import { solvePageCaptchas } from "../solvers" import { waitForAkamaiResolution } from "../utils/akamaiWait" @@ -51,7 +51,11 @@ export async function runTier4( const state: { proxyContext?: Awaited> } = {} try { - const proxyContext = await newFreshContext(handle.browser, { proxy: proxyUrl }) + const proxyContext = await newFreshContext(handle.browser, { + proxy: proxyUrl, + onCreated: handle.noteTemporaryContext, + requestReplacement: handle.requestBrowserReplacement, + }) state.proxyContext = proxyContext const page = await proxyContext.newPage() @@ -187,11 +191,6 @@ export async function runTier4( reason: err instanceof Error ? err.message : String(err), } } finally { - // Same timeout-bounded close as tier3 — see comment there. - if (state.proxyContext) { - await Promise.race([state.proxyContext.close(), new Promise((resolve) => setTimeout(resolve, 5000))]).catch( - () => {}, - ) - } + await closeTemporaryContext(state.proxyContext, handle.requestBrowserReplacement, "tier4 context cleanup timed out") } } diff --git a/packages/tiers/tests/orchestrator.test.ts b/packages/tiers/tests/orchestrator.test.ts deleted file mode 100644 index bde54c7..0000000 --- a/packages/tiers/tests/orchestrator.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { describe, expect, test } from "bun:test" -import { shouldFlagForRecycle } from "../src/orchestrator" - -// Covers the recycle-on-suspect policy: only `blocked` and `needs-js` outcomes -// should flag the pool for a browser recycle. Successful solves, transient -// errors, and timeouts must NOT trigger a recycle — otherwise warm cookies -// + cf_clearance get thrown away on every request. -describe("shouldFlagForRecycle", () => { - test("flags blocked outcomes", () => { - expect(shouldFlagForRecycle("blocked")).toBe(true) - }) - - test("flags needs-js outcomes", () => { - expect(shouldFlagForRecycle("needs-js")).toBe(true) - }) - - test("does NOT flag success outcomes", () => { - expect(shouldFlagForRecycle("success")).toBe(false) - }) - - test("does NOT flag transient errors", () => { - expect(shouldFlagForRecycle("error")).toBe(false) - }) - - test("does NOT flag timeouts", () => { - expect(shouldFlagForRecycle("timeout")).toBe(false) - }) - - test("does NOT flag skipped outcomes", () => { - expect(shouldFlagForRecycle("skipped")).toBe(false) - }) -}) diff --git a/packages/tiers/tests/temporaryContextCleanup.test.ts b/packages/tiers/tests/temporaryContextCleanup.test.ts new file mode 100644 index 0000000..78bbe79 --- /dev/null +++ b/packages/tiers/tests/temporaryContextCleanup.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from "bun:test" +import type { BrowserHandle } from "@trawl/browser" +import { runTier3 } from "../src/tiers/3" +import { runTier4 } from "../src/tiers/4" + +const makeHandle = () => { + let contexts = 0 + let closes = 0 + const context = { + addInitScript: async () => {}, + newPage: async () => { + throw new Error("page creation failed") + }, + close: async () => { + closes++ + }, + } + const handle = { + id: 0, + lease: 1, + context: {}, + browser: { newContext: async () => context }, + fingerprint: { userAgent: "test", platform: "Linux x86_64", locale: "en-US", timezone: "UTC" }, + noteTemporaryContext: () => { + contexts++ + }, + } satisfies BrowserHandle + return { handle, counts: () => ({ contexts, closes }) } +} + +describe("temporary context cleanup", () => { + test("Tier 3 counts and closes a context when page creation fails", async () => { + const { handle, counts } = makeHandle() + const result = await runTier3("https://example.com", handle, 1_000) + expect(result.status).toBe("error") + expect(counts()).toEqual({ contexts: 1, closes: 1 }) + }) + + test("Tier 4 counts and closes a context when page creation fails", async () => { + const { handle, counts } = makeHandle() + const result = await runTier4("https://example.com", handle, 1_000, "http://proxy.example:8080") + expect(result.status).toBe("error") + expect(counts()).toEqual({ contexts: 1, closes: 1 }) + }) +}) diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index a23f48d..95f938b 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -115,7 +115,8 @@ export interface BrowserHandle { // biome-ignore lint/suspicious/noExplicitAny: see comment above browser: any fingerprint: BrowserFingerprint - noteTemporaryContext?: (reason: string) => void + noteTemporaryContext?: () => void + requestBrowserReplacement?: (reason: string) => void } // Per-request proxy override as it arrives at the API. Prowlarr's Cardigann flow