Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 54 additions & 9 deletions src/fetch/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
});
}

Expand Down Expand Up @@ -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<RawFetchResult | null>;
},
): Promise<RawFetchResult | StageError> {
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');
Expand All @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions tests/integration/px2-rc/rc-exit-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,23 @@ describe.skipIf(RC_GATE_DISABLED)('PX2 RC exit gate — fresh install, registrat
`<main><h1>Changelog</h1><p>${CHANGELOG_V2}</p></main></body></html>`,
);

// 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,
Expand Down
61 changes: 61 additions & 0 deletions tests/unit/fetch/router-browser-acquire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
});
});
});
Loading