feat(click): auto-retry on transient occlusion before erroring
fork.7 caught the X mask-overlay race correctly but reported it to the user verbatim — every transient overlay (modal backdrop, focus ring, click-outside mask, sticky banner) became an error the user had to wrap in their own retry loop. Most of these clear within a frame or two on their own. Now `verify_click_target` retries the elementFromPoint probe a few times (default 3 × 200ms = 600ms total grace period) before failing. Real-world overlays that blink in for a render cycle clear during the first retry; persistent overlays still surface as errors with the same actionable message — just qualified with "still occluded after N retries / Mms" so the user knows we tried. Tunable: AGENT_BROWSER_OCCLUSION_RETRIES (default 3, 0 disables) AGENT_BROWSER_OCCLUSION_RETRY_DELAY_MS (default 200) DOM.resolveNode is called once outside the loop — backendNodeId is stable across renders, only the element under (x, y) changes when overlays flicker. Each probe is still capped at 500ms so a stuck Runtime.callFunctionOn can't stall a click for longer than the user expects.
This commit is contained in:
+74
-52
@@ -485,9 +485,8 @@ async fn verify_click_target(
|
|||||||
) -> Result<(), String> {
|
) -> Result<(), String> {
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
|
|
||||||
// Tight 500ms timeout — this is a defensive guard, not critical path.
|
// Resolve once. backendNodeId is stable across renders; only the
|
||||||
// If it can't run quickly, fall through and let the click proceed
|
// element under (x, y) is what changes when an overlay flickers.
|
||||||
// (we're no worse off than the unguarded code path).
|
|
||||||
let resolve_params = DomResolveNodeParams {
|
let resolve_params = DomResolveNodeParams {
|
||||||
backend_node_id: Some(backend_node_id),
|
backend_node_id: Some(backend_node_id),
|
||||||
node_id: None,
|
node_id: None,
|
||||||
@@ -512,6 +511,35 @@ async fn verify_click_target(
|
|||||||
return Ok(());
|
return Ok(());
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Auto-retry on transient occlusion. Many real-world overlays
|
||||||
|
// (modal backdrops, focus rings, click-outside masks) blink in for
|
||||||
|
// a frame or two during state transitions and clear on their own.
|
||||||
|
// Without retries the user gets an "occluded" error and has to
|
||||||
|
// wrap every click in their own retry loop. With retries the
|
||||||
|
// common case is invisible — only persistent overlays surface.
|
||||||
|
//
|
||||||
|
// AGENT_BROWSER_OCCLUSION_RETRIES (default 3, 0 disables)
|
||||||
|
// AGENT_BROWSER_OCCLUSION_RETRY_DELAY_MS (default 200)
|
||||||
|
let max_retries: u32 = std::env::var("AGENT_BROWSER_OCCLUSION_RETRIES")
|
||||||
|
.ok()
|
||||||
|
.and_then(|v| v.parse().ok())
|
||||||
|
.unwrap_or(3);
|
||||||
|
let retry_delay_ms: u64 = std::env::var("AGENT_BROWSER_OCCLUSION_RETRY_DELAY_MS")
|
||||||
|
.ok()
|
||||||
|
.and_then(|v| v.parse().ok())
|
||||||
|
.unwrap_or(200);
|
||||||
|
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
struct Occluder {
|
||||||
|
tag: Option<String>,
|
||||||
|
testid: Option<String>,
|
||||||
|
role: Option<String>,
|
||||||
|
#[serde(rename = "ariaLabel")]
|
||||||
|
aria_label: Option<String>,
|
||||||
|
text: Option<String>,
|
||||||
|
reason: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
// function(x, y) { ... } where `this` is the target element.
|
// function(x, y) { ... } where `this` is the target element.
|
||||||
// Return null → click is safe.
|
// Return null → click is safe.
|
||||||
// Return JSON → describes the occluding element.
|
// Return JSON → describes the occluding element.
|
||||||
@@ -527,51 +555,46 @@ async fn verify_click_target(
|
|||||||
text: ((at.textContent||'').trim().slice(0, 60)) \
|
text: ((at.textContent||'').trim().slice(0, 60)) \
|
||||||
}); \
|
}); \
|
||||||
}";
|
}";
|
||||||
let call_params = serde_json::json!({
|
|
||||||
"objectId": object_id,
|
|
||||||
"functionDeclaration": function_decl,
|
|
||||||
"arguments": [{"value": x}, {"value": y}],
|
|
||||||
"returnByValue": true,
|
|
||||||
});
|
|
||||||
let call_fut = client.send_command_typed::<_, serde_json::Value>(
|
|
||||||
"Runtime.callFunctionOn",
|
|
||||||
&call_params,
|
|
||||||
Some(session_id),
|
|
||||||
);
|
|
||||||
let Ok(call_resp) =
|
|
||||||
tokio::time::timeout(std::time::Duration::from_millis(500), call_fut).await
|
|
||||||
else {
|
|
||||||
return Ok(());
|
|
||||||
};
|
|
||||||
let Ok(call_result) = call_resp else {
|
|
||||||
return Ok(());
|
|
||||||
};
|
|
||||||
|
|
||||||
let value = call_result
|
let mut last_occ: Option<Occluder> = None;
|
||||||
.get("result")
|
for attempt in 0..=max_retries {
|
||||||
.and_then(|r| r.get("value"));
|
if attempt > 0 {
|
||||||
|
tokio::time::sleep(std::time::Duration::from_millis(retry_delay_ms)).await;
|
||||||
// null / undefined / missing → safe
|
}
|
||||||
let json_str = match value {
|
let call_params = serde_json::json!({
|
||||||
Some(serde_json::Value::String(s)) => s.clone(),
|
"objectId": object_id,
|
||||||
_ => return Ok(()),
|
"functionDeclaration": function_decl,
|
||||||
};
|
"arguments": [{"value": x}, {"value": y}],
|
||||||
|
"returnByValue": true,
|
||||||
#[derive(Deserialize)]
|
});
|
||||||
struct Occluder {
|
let call_fut = client.send_command_typed::<_, serde_json::Value>(
|
||||||
tag: Option<String>,
|
"Runtime.callFunctionOn",
|
||||||
testid: Option<String>,
|
&call_params,
|
||||||
role: Option<String>,
|
Some(session_id),
|
||||||
#[serde(rename = "ariaLabel")]
|
);
|
||||||
aria_label: Option<String>,
|
let Ok(call_resp) =
|
||||||
text: Option<String>,
|
tokio::time::timeout(std::time::Duration::from_millis(500), call_fut).await
|
||||||
reason: Option<String>,
|
else {
|
||||||
|
return Ok(()); // probe itself stalled — fall through to click
|
||||||
|
};
|
||||||
|
let Ok(call_result) = call_resp else {
|
||||||
|
return Ok(());
|
||||||
|
};
|
||||||
|
let value = call_result.get("result").and_then(|r| r.get("value"));
|
||||||
|
let json_str = match value {
|
||||||
|
Some(serde_json::Value::String(s)) => s.clone(),
|
||||||
|
// null / undefined → element at point IS our target. Safe.
|
||||||
|
_ => return Ok(()),
|
||||||
|
};
|
||||||
|
let occ: Occluder = match serde_json::from_str(&json_str) {
|
||||||
|
Ok(v) => v,
|
||||||
|
Err(_) => return Ok(()),
|
||||||
|
};
|
||||||
|
last_occ = Some(occ);
|
||||||
}
|
}
|
||||||
let occ: Occluder = match serde_json::from_str(&json_str) {
|
|
||||||
Ok(v) => v,
|
|
||||||
Err(_) => return Ok(()),
|
|
||||||
};
|
|
||||||
|
|
||||||
|
// All retries exhausted — overlay is sticky. Build the descriptive error.
|
||||||
|
let occ = last_occ.expect("loop ran at least once");
|
||||||
if let Some(reason) = occ.reason {
|
if let Some(reason) = occ.reason {
|
||||||
return Err(format!(
|
return Err(format!(
|
||||||
"Ref {} cannot be clicked at its computed position: {}. \
|
"Ref {} cannot be clicked at its computed position: {}. \
|
||||||
@@ -579,7 +602,6 @@ async fn verify_click_target(
|
|||||||
ref_id, reason
|
ref_id, reason
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut desc = occ.tag.unwrap_or_else(|| "unknown".to_string());
|
let mut desc = occ.tag.unwrap_or_else(|| "unknown".to_string());
|
||||||
if let Some(t) = occ.testid {
|
if let Some(t) = occ.testid {
|
||||||
desc.push_str(&format!("[testid={}]", t));
|
desc.push_str(&format!("[testid={}]", t));
|
||||||
@@ -595,13 +617,13 @@ async fn verify_click_target(
|
|||||||
desc.push_str(&format!(" text=\"{}\"", t));
|
desc.push_str(&format!(" text=\"{}\"", t));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
let waited_ms = (max_retries as u64) * retry_delay_ms;
|
||||||
Err(format!(
|
Err(format!(
|
||||||
"Ref {} is occluded by {} at the click point. \
|
"Ref {} is occluded by {} at the click point (still occluded after \
|
||||||
A transient overlay (modal backdrop, mask, sticky banner, etc.) \
|
{} retries / {}ms). A persistent overlay is in the way — \
|
||||||
appeared between snapshot and click. \
|
re-run snapshot, dismiss the overlay, or set \
|
||||||
Wait for it to clear or re-snapshot, then retry. \
|
AGENT_BROWSER_VERIFY_CLICK_TARGET=0 to bypass.",
|
||||||
Set AGENT_BROWSER_VERIFY_CLICK_TARGET=0 to bypass this guard.",
|
ref_id, desc, max_retries, waited_ms,
|
||||||
ref_id, desc
|
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user