mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix: browser-pool stub type errors, silent-fallback regression, grokClearance signature
Pre-merge fixes for the CloakBrowser/Playwright extraction into the optional @omniroute/browser-pool workspace package: - Add the @omniroute/browser-pool path mapping to the root tsconfig.json (tsconfig.typecheck-core.json extends the root, not open-sse/tsconfig.json, so typecheck:core was failing with 3x TS2307 in browserPool.ts). - tryBackedChat() in browserBackedChat.ts silently returned the stale httpResult (the unsolved challenge/403 body) when the optional browser-pool package was absent, instead of surfacing an error. Now throws a descriptive error so callers (claude-web, duckduckgo-web) and upstream fallback handling can react correctly instead of treating an unsolved challenge as a definitive response. Restored resolveBrowserContextProxy (dropped during the browserPool.ts merge conflict resolution against origin/release/v3.8.49) which tests/unit/browserPool-proxy.test.ts already depends on. - Fix acquireFreshGrokClearance stub signature in grokClearance.ts to match the real package impl and the grok-web.ts caller: (signal?: AbortSignal | null) => Promise<string | null>, not (prompt: string) => Promise<PooledContext | null>. - Add tests/unit/browserBackedChat-optional-package-absent.test.ts covering the previously-silent "package absent + challenge response" regression, plus a __setBrowserPoolModOverrideForTesting() hook to simulate that path deterministically. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
@@ -48,7 +48,26 @@ export interface BrowserPoolModule {
|
||||
setProxyResolver(fn: ProxyResolver): void;
|
||||
}
|
||||
let modPromise: Promise<BrowserPoolModule | null> | null = null;
|
||||
// Test-only escape hatch: lets tests simulate the "optional @omniroute/browser-pool
|
||||
// package is not installed" path deterministically, without depending on a real
|
||||
// failing dynamic import (which can otherwise trip unrelated module-resolution
|
||||
// network fallbacks and hang the test process).
|
||||
let modOverride: BrowserPoolModule | null | undefined = undefined;
|
||||
|
||||
export function __setBrowserPoolModOverrideForTesting(
|
||||
value: BrowserPoolModule | null | undefined
|
||||
): void {
|
||||
modOverride = value;
|
||||
modPromise = null;
|
||||
}
|
||||
|
||||
export function __resetBrowserPoolModOverrideForTesting(): void {
|
||||
modOverride = undefined;
|
||||
modPromise = null;
|
||||
}
|
||||
|
||||
function getMod(): Promise<BrowserPoolModule | null> {
|
||||
if (modOverride !== undefined) return Promise.resolve(modOverride);
|
||||
if (!modPromise) {
|
||||
modPromise = import("@omniroute/browser-pool").catch(() => null);
|
||||
}
|
||||
@@ -276,8 +295,23 @@ export async function tryBackedChat(
|
||||
// Need browser-backed path — await the module
|
||||
const loaded = await mod;
|
||||
|
||||
// The optional @omniroute/browser-pool package is not installed. Unlike
|
||||
// the pre-refactor inline implementation (which always had the browser
|
||||
// fallback available), there is no way to solve the challenge here —
|
||||
// silently returning the stale httpResult (the 403/challenge body) would
|
||||
// make callers (claude-web, duckduckgo-web) treat an unsolved challenge
|
||||
// as a definitive upstream response. Throw instead so upstream fallback
|
||||
// handling (combo routing / executor error paths) can react correctly.
|
||||
if (!loaded) {
|
||||
throw new Error(
|
||||
"tryBackedChat: upstream returned a challenge response " +
|
||||
`(status ${httpResult.status}) and the optional @omniroute/browser-pool ` +
|
||||
"package is not installed — cannot solve the challenge. Install " +
|
||||
"@omniroute/browser-pool to enable the browser-backed fallback."
|
||||
);
|
||||
}
|
||||
|
||||
// Get fresh cookies via browser
|
||||
if (loaded) {
|
||||
try {
|
||||
const fresh = await loaded.getFreshCookiesWithWarmup(req);
|
||||
if (fresh) {
|
||||
@@ -317,9 +351,6 @@ export async function tryBackedChat(
|
||||
}
|
||||
throw inner;
|
||||
}
|
||||
}
|
||||
|
||||
return httpResult;
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof DOMException && err.name === "AbortError") {
|
||||
return {
|
||||
|
||||
@@ -146,6 +146,21 @@ export async function resolvePlaywrightProxy(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the proxy for a browser-pool context key. Scoped context keys
|
||||
* (e.g. "claude-web:account-scope") carry a stable `proxyProviderKey` so the
|
||||
* proxy lookup always targets the underlying provider, not the scoped key.
|
||||
* Kept inline alongside resolvePlaywrightProxy — trivial wrapper, no
|
||||
* playwright/browser-pool dependency.
|
||||
*/
|
||||
export async function resolveBrowserContextProxy(
|
||||
contextKey: string,
|
||||
options: Pick<BrowserPoolContextOptions, "proxyProviderKey">,
|
||||
deps?: ResolvePlaywrightProxyDeps
|
||||
): Promise<import("playwright").LaunchOptions["proxy"] | undefined> {
|
||||
return resolvePlaywrightProxy(options.proxyProviderKey ?? contextKey, deps);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// getBrowserPoolStatus — INLINE (returns disabled status)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
import type { PooledContext } from "@omniroute/browser-pool";
|
||||
|
||||
export function shouldUseGrokBrowserBacked(): boolean {
|
||||
const flag = process.env.WEB_COOKIE_USE_BROWSER;
|
||||
if (flag === "1" || flag === "true" || flag === "on") return true;
|
||||
@@ -7,16 +5,20 @@ export function shouldUseGrokBrowserBacked(): boolean {
|
||||
return poolFlag === "on" || poolFlag === "1" || poolFlag === "true";
|
||||
}
|
||||
|
||||
let grokClearanceAcquireOverride: ((prompt: string) => Promise<PooledContext | null>) | null = null;
|
||||
let grokClearanceAcquireOverride:
|
||||
((signal?: AbortSignal | null) => Promise<string | null>)
|
||||
| null = null;
|
||||
|
||||
export function __setGrokClearanceAcquireOverrideForTesting(
|
||||
fn: ((prompt: string) => Promise<PooledContext | null>) | null,
|
||||
fn: ((signal?: AbortSignal | null) => Promise<string | null>) | null,
|
||||
): void {
|
||||
grokClearanceAcquireOverride = fn;
|
||||
}
|
||||
|
||||
export async function acquireFreshGrokClearance(prompt: string): Promise<PooledContext | null> {
|
||||
if (grokClearanceAcquireOverride) return grokClearanceAcquireOverride(prompt);
|
||||
export async function acquireFreshGrokClearance(
|
||||
signal?: AbortSignal | null,
|
||||
): Promise<string | null> {
|
||||
if (grokClearanceAcquireOverride) return grokClearanceAcquireOverride(signal);
|
||||
const mod = await import("@omniroute/browser-pool");
|
||||
return mod.acquireFreshGrokClearance(prompt);
|
||||
return mod.acquireFreshGrokClearance(signal);
|
||||
}
|
||||
|
||||
92
tests/unit/browserBackedChat-optional-package-absent.test.ts
Normal file
92
tests/unit/browserBackedChat-optional-package-absent.test.ts
Normal file
@@ -0,0 +1,92 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { describe, it } from "node:test";
|
||||
|
||||
import {
|
||||
__resetBrowserPoolModOverrideForTesting,
|
||||
__resetHttpBackedChatOverrideForTesting,
|
||||
__setBrowserPoolModOverrideForTesting,
|
||||
__setHttpBackedChatOverrideForTesting,
|
||||
tryBackedChat,
|
||||
} from "../../open-sse/services/browserBackedChat.ts";
|
||||
|
||||
// Regression test for the "optional @omniroute/browser-pool package absent"
|
||||
// path in tryBackedChat(). Before the fix, when the upstream returned a
|
||||
// challenge response (e.g. 403) and the optional browser-pool package was
|
||||
// not installed (getMod() resolves to null — the same shape a real failed
|
||||
// `import("@omniroute/browser-pool")` produces), tryBackedChat() silently
|
||||
// returned the stale challenge result instead of surfacing a clear error —
|
||||
// callers (claude-web, duckduckgo-web) would then treat the unsolved
|
||||
// challenge as a definitive upstream response.
|
||||
//
|
||||
// __setBrowserPoolModOverrideForTesting(null) simulates the module-absent
|
||||
// case deterministically (same as getMod() catching a failed dynamic
|
||||
// import), without depending on a real failing dynamic import of a
|
||||
// nonexistent package.
|
||||
describe("tryBackedChat — optional @omniroute/browser-pool package absent", () => {
|
||||
it("throws a descriptive error instead of silently returning the stale challenge response", async () => {
|
||||
__setBrowserPoolModOverrideForTesting(null);
|
||||
__setHttpBackedChatOverrideForTesting(async () => ({
|
||||
status: 403,
|
||||
contentType: "text/html",
|
||||
body: Buffer.from("<html>Just a moment...</html>"),
|
||||
isStealth: true,
|
||||
timing: {
|
||||
acquireContextMs: 0,
|
||||
navigateMs: 0,
|
||||
submitMs: 0,
|
||||
captureResponseMs: 0,
|
||||
totalMs: 0,
|
||||
},
|
||||
}));
|
||||
|
||||
try {
|
||||
await assert.rejects(
|
||||
() =>
|
||||
tryBackedChat({
|
||||
poolKey: "duckduckgo-web",
|
||||
chatUrl: "https://duck.ai/duckchat/v1/chat",
|
||||
userMessage: "hello",
|
||||
}),
|
||||
(err: unknown) => {
|
||||
assert.ok(err instanceof Error);
|
||||
// Must NOT resolve with the stale challenge body — must throw with
|
||||
// enough context to diagnose (challenge status + missing package).
|
||||
assert.match(err.message, /challenge/i);
|
||||
assert.match(err.message, /@omniroute\/browser-pool/);
|
||||
assert.match(err.message, /403/);
|
||||
return true;
|
||||
}
|
||||
);
|
||||
} finally {
|
||||
__resetHttpBackedChatOverrideForTesting();
|
||||
__resetBrowserPoolModOverrideForTesting();
|
||||
}
|
||||
});
|
||||
|
||||
it("returns the httpResult directly when it is not a challenge response (2xx path unaffected)", async () => {
|
||||
__setHttpBackedChatOverrideForTesting(async () => ({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: Buffer.from(JSON.stringify({ ok: true })),
|
||||
isStealth: true,
|
||||
timing: {
|
||||
acquireContextMs: 0,
|
||||
navigateMs: 0,
|
||||
submitMs: 0,
|
||||
captureResponseMs: 0,
|
||||
totalMs: 0,
|
||||
},
|
||||
}));
|
||||
|
||||
try {
|
||||
const result = await tryBackedChat({
|
||||
poolKey: "duckduckgo-web",
|
||||
chatUrl: "https://duck.ai/duckchat/v1/chat",
|
||||
userMessage: "hello",
|
||||
});
|
||||
assert.equal(result.status, 200);
|
||||
} finally {
|
||||
__resetHttpBackedChatOverrideForTesting();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -19,7 +19,8 @@
|
||||
"paths": {
|
||||
"@/*": ["./src/*"],
|
||||
"@omniroute/open-sse": ["./open-sse"],
|
||||
"@omniroute/open-sse/*": ["./open-sse/*"]
|
||||
"@omniroute/open-sse/*": ["./open-sse/*"],
|
||||
"@omniroute/browser-pool": ["./packages/browser-pool/src"]
|
||||
},
|
||||
"plugins": [
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user