diff --git a/src/fetch/router.ts b/src/fetch/router.ts index cf1f9aea..812e426f 100644 --- a/src/fetch/router.ts +++ b/src/fetch/router.ts @@ -792,6 +792,26 @@ export class SmartRouter { screenshot, signal, stealth: stealthForBrowser(getConfig(), { antiBotEscalation: false }), + // WHY A THUNK AND NOT A `fallback`. Every OTHER escalation reaches the browser + // holding an HTTP result it already paid for, so it hands that over as `fallback` + // for free. This path starts AT the browser because the host is MARKED, so there + // is no such result — and eagerly fetching one would put a full HTTP round-trip in + // front of every domain-marked fetch on a healthy machine, which is the cost the + // mark exists to avoid. Deferring it means the lower tier is paid for only on the + // hosts that actually cannot reach the browser rung. + // + // The mark is a PREFERENCE, not a requirement: it is set by an under-threshold + // body, a `__NEXT_DATA__` blob or a high script ratio, none of which claim HTTP + // cannot serve the page. Without this, a machine with no browser engine answered + // `browser_engine_unavailable` for a page plain HTTP had returned in full one call + // earlier — and on a fresh install with no engine that is every fetch of the host, + // forever. + fallbackFetch: async () => { + if (!this.httpClient) return null; + const lower = await this.httpClientFetch(url, { headers, conditionalHeaders, signal }); + this.ensureStats(domain); + return this.toRawFetchResult(lower); + }, }); } @@ -871,11 +891,20 @@ export class SmartRouter { private async browserFetch( url: string, - options: BrowserFetchArgs & { fallback?: RawFetchResult }, + options: BrowserFetchArgs & { + fallback?: RawFetchResult; + /** + * A lower tier that has not been fetched yet, for the call sites that reach the + * browser without one in hand. Invoked ONLY when acquisition fails, so a healthy + * machine never pays for it. Resolving `null` (or throwing) means the lower tier + * had nothing to give, and the actionable error stands. + */ + fallbackFetch?: () => Promise; + }, ): Promise { if (!this.browserPool) throw new Error('SmartRouter: browserPool not configured'); - const { fallback, ...browserOptions } = options; + const { fallback, fallbackFetch, ...browserOptions } = options; const acquired = await this.browserAcquirer.ensureBrowser(); if (acquired !== 'ready') { const logger = createLogger('fetch'); @@ -897,14 +926,30 @@ export class SmartRouter { const companion = await this.companionRungFetch(url, browserOptions, acquired); if (companion) return this.guardChallengeShell(companion); - if (fallback) { + // Nothing in hand — ask the deferred lower tier whether it can serve the page. + // Tried AFTER the companion rung: a real browser is a better answer than HTTP. + let lowerTier = fallback; + if (!lowerTier && fallbackFetch) { + try { + lowerTier = (await fallbackFetch()) ?? undefined; + } catch (err) { + logger.debug('deferred lower-tier fetch failed; falling through to the actionable error', { + url, + error: err instanceof Error ? err.message : String(err), + }); + } + } + + if (lowerTier) { logger.info('browser engine not ready within budget — returning lower-tier content with note', { url }); - // The browser was the escalation target because the lower tier returned - // a challenge shell (or an anti-bot-status challenge body). If we cannot - // acquire it, we must NOT fall back to returning that shell as content — - // guard it so a challenge fallback becomes blocked_by_challenge, while - // legit lower-tier content passes through unchanged with the note. - return this.guardChallengeShell({ ...fallback, warning: BROWSER_INSTALLING_NOTE }); + // The lower tier is guarded whichever way it arrived. When the browser was the + // ESCALATION target, the thing that triggered the escalation may well be a + // challenge shell (or an anti-bot-status challenge body), and returning that as + // content would pass an interstitial off as the page. When it arrived from the + // deferred fetch above it is a fresh response that has never been classified at + // all. Both become blocked_by_challenge if they are challenges; legit lower-tier + // content passes through unchanged with the note. + return this.guardChallengeShell({ ...lowerTier, warning: BROWSER_INSTALLING_NOTE }); } logger.info('browser engine not ready within budget and no lower-tier content — failing with actionable error', { url }); return { diff --git a/tests/integration/px2-rc/rc-exit-gate.test.ts b/tests/integration/px2-rc/rc-exit-gate.test.ts index 1441b1b4..c25fc0fe 100644 --- a/tests/integration/px2-rc/rc-exit-gate.test.ts +++ b/tests/integration/px2-rc/rc-exit-gate.test.ts @@ -555,6 +555,23 @@ describe.skipIf(RC_GATE_DISABLED)('PX2 RC exit gate — fresh install, registrat `

Changelog

${CHANGELOG_V2}

`, ); + // WHICH OF THE TWO IT WAS (#521). Not the fixture, and not the + // warmup/acquirer disagreement the issue suspected either — measured on a + // fresh packed install with a live network, `wigolo warmup --browser` + // reports `Browser: ok` and the NEXT process's acquirer reports the driver + // and the binary both present. The disagreement was never real; under this + // suite's egress fence the driver simply cannot be acquired at all, which is + // a correct answer for a machine with no network. + // + // The red was a PRODUCT defect one layer up. `/changelog`'s body is under the + // 200-character visible-text threshold, so the seeding fetch tripped SPA-shell + // detection and MARKED the fixture host `preferPlaywright`. That first fetch + // still succeeded — the escalation had the HTTP result in hand and degraded to + // it. This re-read then took the domain-marked path, which started AT the + // browser tier with nothing in hand and hard-failed `browser_engine_unavailable` + // for a page HTTP had just served in full. `router.ts` now defers a lower-tier + // fetch onto that branch, so the arm passes through HTTP and never depends on + // whether an engine could be acquired. const refreshed = await session.call('fetch', { url: `${site.url}/changelog`, force_refresh: true, diff --git a/tests/unit/fetch/router-browser-acquire.test.ts b/tests/unit/fetch/router-browser-acquire.test.ts index 2cf757ce..b1e35c6e 100644 --- a/tests/unit/fetch/router-browser-acquire.test.ts +++ b/tests/unit/fetch/router-browser-acquire.test.ts @@ -286,5 +286,66 @@ describe('SmartRouter — lazy browser acquisition threading (D3)', () => { expect('error' in result).toBe(true); expect((result as { hint?: string; error_reason: string }).hint ?? '').toMatch(/wigolo warmup --browser/); }); + + /** + * THE MARK IS A PREFERENCE, NOT A REQUIREMENT — and this is the one branch that + * used to read it as a requirement. + * + * `preferPlaywright` is set by heuristics that never claim HTTP cannot serve the + * page: a body under the 200-character visible-text threshold, a `__NEXT_DATA__` + * blob, a high script ratio. The escalation that SETS the mark hands `browserFetch` + * the HTTP result as `fallback`, so a machine with no browser engine degrades to + * that content with the actionable note. The very next fetch of the same host takes + * `browserOrHttpForBinary` instead, which had no fallback to hand over — so the same + * host, one call later, answered `browser_engine_unavailable` for a page plain HTTP + * had just returned in full. + * + * That is the shape PX2's RC exit gate met: fetch a short fixture page (marks the + * host), then re-read it with `force_refresh` (domain-marked path) and get a hard + * error on a fresh install that has no engine and — behind the gate's egress fence — + * can never acquire one. + * + * The two fetches share ONE router because the mark is in-memory per router + * (`ensureStats`); splitting them would drop the precondition the case is about. + */ + it('the domain-marked path degrades to lower-tier content instead of hard-failing', async () => { + vi.mocked(httpClient.fetch).mockResolvedValue(makeHttpResult(SPA_SHELL_HTML)); + const { acquirer } = makeAcquirer('unavailable'); + const router = new SmartRouter({ httpClient, browserPool, pdfProbe: async () => false, browserAcquirer: acquirer, systemBrowserFetch: noInstalledBrowser }); + + // First fetch: SPA-shell detection marks the host and escalates. Already covered + // above; asserted here only to prove the precondition actually landed. + const first = await router.fetch('https://marked.example/page') as RawFetchResult; + expect(first.method).toBe('http'); + + // Second fetch: same host, now domain-marked, so it starts AT the browser tier. + const second = await router.fetch('https://marked.example/other') as RawFetchResult; + + expect(browserPool.fetchWithBrowser).not.toHaveBeenCalled(); + expect('error' in second, `domain-marked re-read hard-failed: ${JSON.stringify(second)}`).toBe(false); + expect(second.method).toBe('http'); + expect(second.html).toBe(SPA_SHELL_HTML); + expect(second.warning).toMatch(/browser engine installing/); + expect(second.warning).toMatch(/wigolo warmup --browser/); + }); + + it('the domain-marked path still hard-fails when the lower tier has nothing to give', async () => { + const { acquirer } = makeAcquirer('unavailable'); + const router = new SmartRouter({ httpClient, browserPool, pdfProbe: async () => false, browserAcquirer: acquirer, systemBrowserFetch: noInstalledBrowser }); + + vi.mocked(httpClient.fetch).mockResolvedValue(makeHttpResult(SPA_SHELL_HTML)); + await router.fetch('https://marked2.example/page'); + + // The host is marked; now HTTP itself is down, so the degradation has no content + // to return and the actionable error is the correct answer. Without this the fix + // above could have swallowed a real failure into a silent empty success. + vi.mocked(httpClient.fetch).mockRejectedValue(new Error('refused')); + const result = await router.fetch('https://marked2.example/other'); + + expect('error' in result).toBe(true); + const err = result as { error: string; error_reason: string; hint?: string }; + expect(err.error).toBe('browser_engine_unavailable'); + expect(err.hint ?? err.error_reason).toMatch(/wigolo warmup --browser/); + }); }); });