mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
fix: clarify invalid selectors and dedupe diagnostics
This commit is contained in:
@@ -49,7 +49,7 @@ import { WSTransport } from "@/transport/ws-transport";
|
||||
export default defineBackground(() => {
|
||||
const diagnostics = new DiagnosticLogger();
|
||||
diagnostics.record("extension.worker.started", {
|
||||
diagnostic_build: "lumi-session-loss-v1",
|
||||
diagnostic_build: "lumi-session-loss-v2",
|
||||
extension_version: __BSK_EXT_VERSION__,
|
||||
extension_runtime_id: chrome.runtime.id,
|
||||
daemon_ws_url: __BSK_DAEMON_WS_URL__,
|
||||
|
||||
@@ -58,4 +58,28 @@ describe("DiagnosticLogger", () => {
|
||||
{ event: "session.stop.requested", fields: { session_id: "abcd" } },
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not resend an acknowledged event when send records a new event", async () => {
|
||||
const storage = new MemoryStorage();
|
||||
const batches: Array<{ payload?: { entries?: Array<{ event: string }> } }> = [];
|
||||
const logger = new DiagnosticLogger(storage);
|
||||
logger.record("dispatcher.rpc.received", { rpc_id: "one" });
|
||||
await logger.flush();
|
||||
logger.bindTransport((frame) => {
|
||||
batches.push(frame as { payload?: { entries?: Array<{ event: string }> } });
|
||||
if (batches.length === 1) {
|
||||
logger.record("dispatcher.rpc.response_sent", { rpc_id: "one" });
|
||||
}
|
||||
});
|
||||
|
||||
logger.setTransportReady(true);
|
||||
await logger.flush();
|
||||
await logger.flush();
|
||||
|
||||
expect(batches.map((batch) => batch.payload?.entries?.map((entry) => entry.event))).toEqual([
|
||||
["dispatcher.rpc.received"],
|
||||
["dispatcher.rpc.response_sent"],
|
||||
]);
|
||||
expect(storage.value.bsk_diagnostic_events_v1).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -145,13 +145,24 @@ export class DiagnosticLogger {
|
||||
return;
|
||||
}
|
||||
const sentIds = new Set(entries.map((entry) => entry.id));
|
||||
const current = await this.storage.get(STORAGE_KEY);
|
||||
const currentEntries = Array.isArray(current[STORAGE_KEY])
|
||||
? (current[STORAGE_KEY] as DiagnosticEntry[])
|
||||
: [];
|
||||
await this.storage.set({
|
||||
[STORAGE_KEY]: currentEntries.filter((entry) => !sentIds.has(entry.id)),
|
||||
});
|
||||
// Serialize acknowledgement behind every record write that may have
|
||||
// started while send() was in progress. Without this, a concurrent
|
||||
// record can read the pre-ack queue and write an already-sent entry
|
||||
// back into storage, causing the next batch to duplicate it.
|
||||
this.storageQueue = this.storageQueue
|
||||
.then(async () => {
|
||||
const current = await this.storage?.get(STORAGE_KEY);
|
||||
const currentEntries = Array.isArray(current?.[STORAGE_KEY])
|
||||
? (current[STORAGE_KEY] as DiagnosticEntry[])
|
||||
: [];
|
||||
await this.storage?.set({
|
||||
[STORAGE_KEY]: currentEntries.filter((entry) => !sentIds.has(entry.id)),
|
||||
});
|
||||
})
|
||||
.catch((error) => {
|
||||
console.warn("[bsk diagnostic] failed to acknowledge event batch", error);
|
||||
});
|
||||
await this.storageQueue;
|
||||
})().catch((error) => {
|
||||
console.warn("[bsk diagnostic] flush failed", error);
|
||||
});
|
||||
|
||||
@@ -456,6 +456,30 @@ describe("handleClick", () => {
|
||||
expect(res).toMatchObject({ code: "not_found", data: { reason: "selector_not_found" } });
|
||||
});
|
||||
|
||||
it("returns invalid_selector when Chrome rejects non-CSS selector syntax", async () => {
|
||||
const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) });
|
||||
await sm.start("aa11");
|
||||
const fake = makeFakeCdp({
|
||||
"DOM.getDocument": () => ({ root: { nodeId: 1 } }),
|
||||
"DOM.querySelector": () => {
|
||||
throw new Error('{"code":-32000,"message":"DOM Error while querying"}');
|
||||
},
|
||||
});
|
||||
const res = await handleClick(
|
||||
sm,
|
||||
{ session_id: "aa11", selector: "text=下午 12~18点" },
|
||||
{ cdp: fake.cdp, tabsApi: fake.tabsApi },
|
||||
);
|
||||
expect(res).toMatchObject({
|
||||
code: "invalid_params",
|
||||
message: "selector is not valid CSS: text=下午 12~18点",
|
||||
data: {
|
||||
reason: "invalid_selector",
|
||||
selector: "text=下午 12~18点",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("respects the AbortSignal", async () => {
|
||||
const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) });
|
||||
const ctx = await sm.start("aa11");
|
||||
|
||||
@@ -106,6 +106,14 @@ function isAbortLikeError(err: unknown): boolean {
|
||||
return err instanceof DOMException && err.name === "AbortError";
|
||||
}
|
||||
|
||||
function isInvalidCssSelectorError(err: unknown): boolean {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
return (
|
||||
message.includes("DOM Error while querying") ||
|
||||
/invalid selector|not a valid selector|querySelector.*failed/i.test(message)
|
||||
);
|
||||
}
|
||||
|
||||
async function wait(ms: number, signal?: AbortSignal): Promise<void> {
|
||||
if (signal?.aborted) throw new DOMException("aborted", "AbortError");
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
@@ -208,6 +216,14 @@ async function resolveBackendNode(
|
||||
}
|
||||
return { backendNodeId, cdpTarget: { tabId: target.tabId }, usedSelector: params.selector };
|
||||
} catch (err) {
|
||||
if (isInvalidCssSelectorError(err)) {
|
||||
return rpcError(
|
||||
"invalid_params",
|
||||
"invalid_selector",
|
||||
`selector is not valid CSS: ${params.selector}`,
|
||||
{ selector: params.selector },
|
||||
);
|
||||
}
|
||||
return {
|
||||
code: "cdp_failed",
|
||||
message: err instanceof Error ? err.message : String(err),
|
||||
|
||||
@@ -24,6 +24,7 @@ export type RpcErrorReason =
|
||||
| "agent_window_scope"
|
||||
| "element_not_visible"
|
||||
| "ref_not_found"
|
||||
| "invalid_selector"
|
||||
| "selector_not_found"
|
||||
| "target_not_fillable"
|
||||
| "target_not_select"
|
||||
|
||||
@@ -23,7 +23,7 @@ export default defineConfig({
|
||||
modules: ["@wxt-dev/module-react"],
|
||||
manifest: {
|
||||
name: "BrowserSkill (Lumi Diagnostics)",
|
||||
version_name: `${EXTENSION_VERSION}-lumi-session-loss-v1`,
|
||||
version_name: `${EXTENSION_VERSION}-lumi-session-loss-v2`,
|
||||
description:
|
||||
"Let AI agents use your logged-in browser in a separate Agent Window—without interrupting your work. Powered by the bsk CLI.",
|
||||
// Flat debugger sessions are required to address out-of-process iframes.
|
||||
|
||||
@@ -497,4 +497,21 @@ mod tests {
|
||||
.contains("CSS selector")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_selector_explains_css_contract_in_json() {
|
||||
let cli = CliError::from_rpc(RpcError {
|
||||
code: ErrorCode::InvalidParams,
|
||||
message: "selector is not valid CSS: text=submit".into(),
|
||||
data: Some(serde_json::json!({ "reason": "invalid_selector" })),
|
||||
});
|
||||
let exit = cli.exit_code();
|
||||
let hint = hint_for(&cli, render_info_for(&cli).as_ref());
|
||||
let json = json_error_string(&cli, exit, hint);
|
||||
let parsed: serde_json::Value = serde_json::from_str(&json).unwrap();
|
||||
let hint = parsed.get("hint").and_then(|v| v.as_str()).unwrap();
|
||||
assert!(hint.contains("CSS selector"));
|
||||
assert!(hint.contains("text="));
|
||||
assert!(hint.contains("@eN"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -41,6 +41,7 @@ pub mod reason {
|
||||
pub const AGENT_WINDOW_SCOPE: &str = "agent_window_scope";
|
||||
pub const ELEMENT_NOT_VISIBLE: &str = "element_not_visible";
|
||||
pub const REF_NOT_FOUND: &str = "ref_not_found";
|
||||
pub const INVALID_SELECTOR: &str = "invalid_selector";
|
||||
pub const SELECTOR_NOT_FOUND: &str = "selector_not_found";
|
||||
pub const TARGET_NOT_FILLABLE: &str = "target_not_fillable";
|
||||
pub const TARGET_NOT_SELECT: &str = "target_not_select";
|
||||
@@ -221,6 +222,13 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend
|
||||
hint: Some("verify the CSS selector or wait for the element to appear before retrying"),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::InvalidParams, reason::INVALID_SELECTOR) => RenderInfo {
|
||||
summary: "selector is not valid CSS",
|
||||
hint: Some(
|
||||
"use a standard CSS selector or a fresh @eN ref; Playwright text= selectors and XPath are not supported",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::InvalidParams, reason::TARGET_NOT_FILLABLE) => RenderInfo {
|
||||
summary: "target element is not fillable",
|
||||
hint: Some(
|
||||
|
||||
@@ -40,7 +40,7 @@ pub(crate) const DAEMONIZED_ENV: &str = "BSK_DAEMONIZED";
|
||||
/// daemon lock, IPC socket, and WS port — before taking over.
|
||||
pub(crate) const DAEMON_REPLACEMENT_WAIT_ENV: &str = "BSK_DAEMON_REPLACES_PID";
|
||||
|
||||
const LUMI_DIAGNOSTIC_BUILD: &str = "lumi-session-loss-v1";
|
||||
const LUMI_DIAGNOSTIC_BUILD: &str = "lumi-session-loss-v2";
|
||||
|
||||
/// Concrete daemon configuration resolved from CLI flags / defaults.
|
||||
#[derive(Debug, Clone)]
|
||||
|
||||
Reference in New Issue
Block a user