mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-09 11:17:44 +02:00
[eric] browser: evalInPage runs via CDP Runtime.evaluate, not webContents.executeJavaScript
The executeJavaScript path is SUSPENDED by Electron until a page stops loading, so on a page whose trackers never let it settle (Reddit, LinkedIn) every in-page command queues into the 15s backend timeout = the mount-poll wedge that has blocked every agent-loop run. CDP Runtime.evaluate runs in the browser process, out of the renderer's load-blocked event loop, so it does not suspend. This reuses the already-in-production CDP bridge (sendCdp -> send-cdp-command -> wc.debugger.sendCommand, keyed by getWebContentsId, with attach + per-wc serialization + timeout already built) that the AX/click commands use; only the eval path was left on executeJavaScript. No-regression by construction: on any CDP INFRA failure (debugger can't attach because DevTools or a remote-debugging port holds the webContents, or the bridge errors) it falls back to the original executeJavaScript path, and a real page-side exception still surfaces as a throw; window.__OSW_CDP_EVAL__ = false forces the old path. Unwrap logic extracted to cdpEval.ts + 6 unit tests. NOTE not live-demonstrable on a --remote-debugging-port bench: that port globally blocks wc.debugger.attach, so the whole app CDP path (this AND the existing list_interactives) times out there; production and normal dev do not use the port, so attach works and the existing CDP commands prove it
This commit is contained in:
@@ -7,6 +7,7 @@ import { dashboardWs } from './ws/WebSocketManager';
|
||||
import { resolveInput } from './resolveUrl';
|
||||
import { rankAndCapInteractives, type RankItem } from './interactiveRanking';
|
||||
import { shouldStopWaiting, SETTLE_POLL_MS, settleProbeJs } from './browserSettle';
|
||||
import { unwrapCdpEval } from './cdpEval';
|
||||
|
||||
let initialized = false;
|
||||
|
||||
@@ -195,7 +196,40 @@ async function countSafeRoutes(wv: BrowserWebview): Promise<number> {
|
||||
const STUCK_EVAL_GRACE_MS = 2500;
|
||||
const STUCK_EVAL_LIMIT_MS = 9000;
|
||||
|
||||
// Run `code` in the guest page. In Electron we prefer CDP Runtime.evaluate: it runs in the
|
||||
// browser process, so it is NOT suspended while the page is still loading, the way
|
||||
// webContents.executeJavaScript is (that suspend, on a page whose trackers never let it "stop
|
||||
// loading", is the 15s command wedge). When the CDP bridge isn't there (dev Chrome, or a forced
|
||||
// A/B via window.__OSW_CDP_EVAL__ = false) we fall back to the executeJavaScript path unchanged,
|
||||
// so behavior never regresses where CDP can't run. Both paths keep the same contract: return the
|
||||
// value, throw on a page-side error, mark dom-ready on success.
|
||||
async function evalInPage(wv: BrowserWebview, code: string): Promise<any> {
|
||||
const cdpBridge = (window as any).openswarm?.sendCdpCommand;
|
||||
if (cdpBridge && (window as any).__OSW_CDP_EVAL__ !== false) {
|
||||
let cdp: any;
|
||||
try {
|
||||
cdp = await sendCdp(wv, 'Runtime.evaluate',
|
||||
{ expression: code, returnByValue: true, awaitPromise: true });
|
||||
} catch {
|
||||
// CDP INFRA failure (the debugger can't attach because DevTools or a remote-debugging
|
||||
// port already holds this webContents, or the bridge errored). Never worse than today:
|
||||
// fall through to the executeJavaScript path. A real PAGE exception is NOT an infra
|
||||
// failure, it rides exceptionDetails below, so it still surfaces as a throw.
|
||||
cdp = undefined;
|
||||
}
|
||||
if (cdp !== undefined) {
|
||||
const value = unwrapCdpEval(cdp); // throws on a real page-side exception
|
||||
markDomReady(wv);
|
||||
return value;
|
||||
}
|
||||
}
|
||||
return await evalViaExecuteJs(wv, code);
|
||||
}
|
||||
|
||||
// The original webContents.executeJavaScript path, kept as the dev-Chrome / bridge-absent
|
||||
// fallback: it suspends until the page stops loading, so a grace/limit race cancels stragglers
|
||||
// with wv.stop() once the document is ready and flushes the queue.
|
||||
async function evalViaExecuteJs(wv: BrowserWebview, code: string): Promise<any> {
|
||||
const run = wv.executeJavaScript(code).then((v) => {
|
||||
markDomReady(wv);
|
||||
return { done: true as const, value: v };
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
// Run: node --test frontend/src/shared/cdpEval.test.ts
|
||||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { unwrapCdpEval } from './cdpEval.ts';
|
||||
|
||||
test('returns the serialized value on success', () => {
|
||||
assert.equal(unwrapCdpEval({ result: { value: 'hello', type: 'string' } }), 'hello');
|
||||
});
|
||||
|
||||
test('returns an object value untouched (returnByValue serialized)', () => {
|
||||
const v = unwrapCdpEval({ result: { value: { found: true, filled: false } } }) as any;
|
||||
assert.equal(v.found, true);
|
||||
assert.equal(v.filled, false);
|
||||
});
|
||||
|
||||
test('undefined result value comes back as undefined, not a throw', () => {
|
||||
assert.equal(unwrapCdpEval({ result: { type: 'undefined' } }), undefined);
|
||||
});
|
||||
|
||||
test('a page-side throw surfaces as an Error with the exception description', () => {
|
||||
assert.throws(
|
||||
() => unwrapCdpEval({ exceptionDetails: { exception: { description: 'ReferenceError: x is not defined' } } }),
|
||||
/x is not defined/,
|
||||
);
|
||||
});
|
||||
|
||||
test('falls back to exceptionDetails.text when no exception description', () => {
|
||||
assert.throws(
|
||||
() => unwrapCdpEval({ exceptionDetails: { text: 'Uncaught' } }),
|
||||
/Uncaught/,
|
||||
);
|
||||
});
|
||||
|
||||
test('exceptionDetails wins even if a result is also present', () => {
|
||||
assert.throws(
|
||||
() => unwrapCdpEval({ result: { value: 'partial' }, exceptionDetails: { text: 'boom' } }),
|
||||
/boom/,
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,21 @@
|
||||
// Turn a CDP `Runtime.evaluate` result into the value, the way webContents.executeJavaScript
|
||||
// hands it back: return the serialized value, and throw when the page code itself threw (that
|
||||
// arrives as `exceptionDetails`, not as an infra error). Kept pure + separate so it's unit
|
||||
// testable without the whole browser-command module and its Electron globals.
|
||||
|
||||
export interface CdpEvalResult {
|
||||
result?: { value?: unknown; type?: string };
|
||||
exceptionDetails?: {
|
||||
text?: string;
|
||||
exception?: { description?: string; value?: unknown };
|
||||
};
|
||||
}
|
||||
|
||||
export function unwrapCdpEval(cdp: CdpEvalResult): unknown {
|
||||
if (cdp && cdp.exceptionDetails) {
|
||||
const ex = cdp.exceptionDetails;
|
||||
const msg = (ex.exception && (ex.exception.description || ex.exception.value)) || ex.text || 'eval error in page';
|
||||
throw new Error(String(msg));
|
||||
}
|
||||
return cdp && cdp.result ? cdp.result.value : undefined;
|
||||
}
|
||||
Reference in New Issue
Block a user