fix(discord): self-healing CDP scanner — relaxed target match + idle-watchdog pump (#3693) (#4108)

This commit is contained in:
oxoxDev
2026-06-25 09:31:31 -07:00
committed by GitHub
parent 69b36b936c
commit a1c5355734
3 changed files with 512 additions and 30 deletions
+253 -4
View File
@@ -11,6 +11,7 @@ use std::time::Duration;
use serde_json::Value;
use tokio::sync::broadcast::error::RecvError;
use tokio::sync::{broadcast, mpsc};
use super::in_process::{EventFrame, WebviewCdpTransport};
@@ -73,10 +74,11 @@ impl CdpConn {
/// treated as a continuation signal: the pump keeps draining rather
/// than tearing down the session, so a burst that overflows
/// `EVENT_CHANNEL_CAP` drops the skipped frames without re-syncing.
/// Long-lived consumers (e.g. the Discord scanner) do not yet have an
/// idle watchdog to time out a stalled/destroyed page target and force
/// a re-attach — that self-healing path is tracked as a fast-follow in
/// #3693.
/// This plain pump has no idle watchdog, so it only returns once the
/// whole transport is dropped — a stale/destroyed page target leaves it
/// awaiting forever. Long-lived consumers that must self-heal from a
/// dead page target and never drop frames should use
/// [`pump_events_resilient`](Self::pump_events_resilient) instead.
pub async fn pump_events<F>(&mut self, session_id: &str, mut on_event: F) -> Result<(), String>
where
F: FnMut(&str, &Value),
@@ -108,8 +110,255 @@ impl CdpConn {
}
}
/// Like [`pump_events`](Self::pump_events) but adds the two robustness
/// properties a long-lived consumer (the Discord scanner) needs:
///
/// * **Idle watchdog** — returns `Ok(())` once `idle_timeout` elapses
/// with no inbound frame. A live session emits frames well within that
/// window (Discord's gateway heartbeats roughly every 41s), so a longer
/// silence means the page target is stale/destroyed (reload, renderer
/// crash, hard navigation) — the plain pump would await it forever.
/// Returning lets the caller's outer loop re-attach. `idle_timeout`
/// MUST be larger than the consumer's heartbeat cadence.
/// * **Loss-aware delivery** — a background task drains frames off the
/// fixed-capacity broadcast ring into an unbounded queue, so the slow
/// per-frame `on_event` work can't back up the ring and a burst is
/// absorbed instead of dropped. If the ring still overflows before the
/// drain can pull (extreme burst), the pump returns `Err` so the caller
/// re-attaches and restarts capture rather than feeding a partial stream.
///
/// Returns `Ok(())` when the transport closes or the idle watchdog trips,
/// and `Err` on an unrecoverable broadcast lag.
pub async fn pump_events_resilient<F>(
&mut self,
session_id: &str,
idle_timeout: Duration,
on_event: F,
) -> Result<(), String>
where
F: FnMut(&str, &Value),
{
pump_resilient_core(
self.transport.subscribe_events(),
session_id,
idle_timeout,
&self.label,
on_event,
)
.await
}
/// Diagnostic helper — webview label this connection is bound to.
pub fn label(&self) -> &str {
&self.label
}
}
/// Core of [`CdpConn::pump_events_resilient`], split out over raw primitives
/// (a [`broadcast::Receiver`] plus the policy params) so it can be unit-tested
/// without constructing a full [`WebviewCdpTransport`].
///
/// A tiny drain task forwards every session-matched frame from the fixed-size
/// broadcast ring into an unbounded queue, decoupling the slow per-frame
/// `on_event` work from the broadcast consumer. The main loop reads that queue
/// under an idle-timeout watchdog. Returns `Ok(())` when the transport closes
/// (drain task ends) or when `idle_timeout` elapses with no frame.
async fn pump_resilient_core<F>(
mut rx: broadcast::Receiver<EventFrame>,
session_id: &str,
idle_timeout: Duration,
label: &str,
mut on_event: F,
) -> Result<(), String>
where
F: FnMut(&str, &Value),
{
// Channel item is a Result so the drain can signal an unrecoverable lag to
// the consumer instead of silently swallowing it (see the Lagged arm).
let (tx, mut rx_u) = mpsc::unbounded_channel::<Result<EventFrame, String>>();
let session_owned = session_id.to_string();
let drain_label = label.to_string();
let drain = tokio::spawn(async move {
loop {
match rx.recv().await {
Ok(frame) => {
// Liveness must reflect OUR page session only. Forward strictly
// own-session frames; drop both other-session frames and
// empty-session (browser/target-level) events. The latter is
// critical: a dead page still sees empty-session transport
// chatter (target lifecycle, crash/reload churn), and if those
// reached the consumer they would reset the idle watchdog and a
// crashed/reloaded session would never trip re-attach. Discord's
// real traffic (`Network.*`, incl. ~41s gateway heartbeats) is
// always page-session-tagged, so nothing useful is dropped.
if frame.session_id != session_owned {
continue;
}
if tx.send(Ok(frame)).is_err() {
break; // consumer dropped — nothing left to drain into
}
}
Err(RecvError::Lagged(skipped)) => {
// The broadcast ring overflowed before the drain could pull —
// frames are already gone. Continuing would feed a partial
// stream downstream (missed Discord messages, no re-sync), so
// surface it and let the consumer force a re-attach, which
// restarts capture cleanly. The drain does ~zero work per
// frame, so this only trips under an extreme burst.
let msg = format!(
"[cdp][{}] drain lagged skipped={} session_id={} — forcing re-attach",
drain_label, skipped, session_owned
);
log::warn!("{msg}");
let _ = tx.send(Err(msg));
break;
}
Err(RecvError::Closed) => break,
}
}
});
let result = loop {
match tokio::time::timeout(idle_timeout, rx_u.recv()).await {
Ok(Some(Ok(frame))) => on_event(&frame.method, &frame.params),
Ok(Some(Err(e))) => break Err(e), // lag → re-attach (outer loop reconnects)
Ok(None) => break Ok(()), // transport closed / webview forgotten
Err(_elapsed) => {
log::info!(
"[cdp][{}] event pump idle for {:?}, forcing re-attach session_id={}",
label,
idle_timeout,
session_id
);
break Ok(());
}
}
};
drain.abort();
result
}
#[cfg(test)]
mod resilient_pump_tests {
use super::*;
use std::cell::RefCell;
use std::rc::Rc;
fn frame(method: &str, session: &str) -> EventFrame {
EventFrame {
method: method.to_string(),
params: serde_json::json!({}),
session_id: session.to_string(),
}
}
/// No frames within the idle window → the watchdog returns `Ok(())` so the
/// caller can re-attach, instead of awaiting a dead session forever.
#[tokio::test(start_paused = true)]
async fn returns_on_idle_timeout() {
let (tx, rx) = broadcast::channel::<EventFrame>(8);
let calls = Rc::new(RefCell::new(0usize));
let c = calls.clone();
let res = pump_resilient_core(rx, "sess", Duration::from_secs(30), "test", |_m, _p| {
*c.borrow_mut() += 1;
})
.await;
assert!(res.is_ok());
assert_eq!(*calls.borrow(), 0);
drop(tx); // sender kept alive across the run so the idle path is exercised
}
/// When the transport closes, frames already buffered are flushed to the
/// consumer before the pump returns.
#[tokio::test(start_paused = true)]
async fn flushes_buffered_then_returns_on_close() {
let (tx, rx) = broadcast::channel::<EventFrame>(64);
for i in 0..10 {
tx.send(frame(&format!("m{i}"), "sess")).unwrap();
}
drop(tx); // close after queuing
let got = Rc::new(RefCell::new(Vec::new()));
let g = got.clone();
let res = pump_resilient_core(rx, "sess", Duration::from_secs(30), "test", |m, _p| {
g.borrow_mut().push(m.to_string());
})
.await;
assert!(res.is_ok());
assert_eq!(got.borrow().len(), 10);
}
/// A burst far larger than `EVENT_CHANNEL_CAP` (here a tiny ring of 4) is
/// delivered without loss: the drain task forwards each frame into the
/// unbounded queue faster than it can overflow the ring.
#[tokio::test(start_paused = true)]
async fn is_non_lossy_beyond_broadcast_cap() {
let (tx, rx) = broadcast::channel::<EventFrame>(4);
let got = Rc::new(RefCell::new(0usize));
let g = got.clone();
let producer = tokio::spawn(async move {
for i in 0..200 {
let _ = tx.send(frame(&format!("m{i}"), "sess"));
// Yield so the drain task pulls each frame before the next send
// can overflow the 4-slot ring.
tokio::task::yield_now().await;
}
// tx dropped here → channel closes once the burst is drained
});
let res = pump_resilient_core(rx, "sess", Duration::from_secs(3600), "test", |_m, _p| {
*g.borrow_mut() += 1;
})
.await;
producer.await.unwrap();
assert!(res.is_ok());
assert_eq!(*got.borrow(), 200);
}
/// Only OWN-session frames are delivered. Other-session frames are dropped,
/// and — critically for the idle watchdog — so are empty-session
/// (browser/target-level) frames: a dead/reloaded page still emits
/// empty-session transport chatter, and delivering it would reset the idle
/// timer so a stale session would never trip re-attach. Regression guard for
/// the live renderer-crash smoke where the watchdog failed to fire.
#[tokio::test(start_paused = true)]
async fn delivers_only_own_session_drops_empty_and_other() {
let (tx, rx) = broadcast::channel::<EventFrame>(64);
tx.send(frame("a", "sess")).unwrap();
tx.send(frame("b", "other")).unwrap(); // other session → dropped
tx.send(frame("c", "")).unwrap(); // empty/browser-level → dropped (no false liveness)
tx.send(frame("d", "sess")).unwrap();
drop(tx);
let got = Rc::new(RefCell::new(Vec::new()));
let g = got.clone();
let res = pump_resilient_core(rx, "sess", Duration::from_secs(30), "test", |m, _p| {
g.borrow_mut().push(m.to_string());
})
.await;
assert!(res.is_ok());
assert_eq!(*got.borrow(), vec!["a", "d"]);
}
/// When the ring overflows before the drain can pull (burst with no yields),
/// the lag must surface as an error so the outer loop re-attaches and
/// re-syncs — never a silent partial stream. Regression guard for the
/// non-lossy contract (CodeRabbit review on #3693).
#[tokio::test(start_paused = true)]
async fn lag_surfaces_error_to_force_reattach() {
let (tx, rx) = broadcast::channel::<EventFrame>(4);
// Flood far past the 4-slot ring with no yields: the drain task hasn't
// run yet, so the oldest frames are evicted → first recv is Lagged.
for i in 0..200 {
let _ = tx.send(frame(&format!("m{i}"), "sess"));
}
let got = Rc::new(RefCell::new(0usize));
let g = got.clone();
let res = pump_resilient_core(rx, "sess", Duration::from_secs(3600), "test", |_m, _p| {
*g.borrow_mut() += 1;
})
.await;
assert!(
res.is_err(),
"lag must surface as Err so the session re-attaches"
);
drop(tx); // sender kept alive across the run
}
}
+159 -26
View File
@@ -40,6 +40,12 @@ mod dom_snapshot;
/// or the page target disappears (e.g. Discord refresh, navigation).
const RECONNECT_BACKOFF: Duration = Duration::from_secs(3);
const MAX_CHANNEL_MESSAGES: usize = 400;
/// Idle window after which the event pump assumes the attached page target is
/// stale/destroyed (reload, renderer crash, hard navigation) and returns so
/// the outer loop re-attaches. Chosen at >2x Discord's ~41s gateway heartbeat:
/// a live session always emits gateway WS frames within this window, so a
/// longer silence means the session is dead, not merely quiet.
const PUMP_IDLE_TIMEOUT: Duration = Duration::from_secs(90);
#[derive(Clone, Debug, PartialEq, Eq)]
struct DiscordPersistMessage {
@@ -291,8 +297,20 @@ pub fn spawn_scanner<R: Runtime>(
// tends to race with the renderer's own initialization and we miss
// the first few frames anyway.
sleep(Duration::from_secs(4)).await;
// Lock onto the page target once a strict fragment match succeeds, so
// re-attaches after a reload survive Discord stripping the URL hash
// (see `attach_account_target`). Persists across reconnects.
let mut pinned_target_id: Option<String> = None;
loop {
match run_mitm_session(&app, &account_id, &url_prefix, &fragment).await {
match run_mitm_session(
&app,
&account_id,
&url_prefix,
&fragment,
&mut pinned_target_id,
)
.await
{
Ok(()) => {
log::info!(
"[discord][{}] session ended cleanly, reconnecting",
@@ -316,23 +334,23 @@ pub fn spawn_scanner<R: Runtime>(
}
/// Run one CDP attach → enable → stream-events lifecycle. Returns when the
/// underlying WebSocket closes, the page target disappears, or any
/// dispatch hits an unrecoverable error. Caller loops.
/// in-process transport closes (webview torn down) or when the pump's idle
/// watchdog trips after `PUMP_IDLE_TIMEOUT` of no frames — i.e. the attached
/// page target went stale (Discord reload, renderer crash, hard navigation).
/// The caller's outer loop then re-attaches. `pinned_target_id` carries the
/// pin/strict/relaxed resolution state across reconnects (see
/// [`attach_account_target`]).
async fn run_mitm_session<R: Runtime>(
app: &AppHandle<R>,
account_id: &str,
url_prefix: &str,
url_fragment: &str,
pinned_target_id: &mut Option<String>,
) -> Result<(), String> {
let url_prefix_owned = url_prefix.to_string();
let url_fragment_owned = url_fragment.to_string();
let pred = move |t: &crate::cdp::target::CdpTarget| -> bool {
t.url.starts_with(&url_prefix_owned) && t.url.ends_with(&url_fragment_owned)
};
let (mut cdp, session_id) =
crate::cdp::target::connect_and_attach_matching_in_process::<R, _>(app, account_id, pred)
attach_account_target(app, account_id, url_prefix, url_fragment, pinned_target_id)
.await
.map_err(|e| format!("attach: {e} (prefix={url_prefix} fragment={url_fragment})"))?;
.map_err(|e| format!("attach: {e}"))?;
log::info!(
"[discord][{}] attached label={} session={}",
account_id,
@@ -350,16 +368,127 @@ async fn run_mitm_session<R: Runtime>(
session_id
);
// Drop into the event read loop until the in-process channel signals
// closure. V1 doesn't issue any in-stream calls (responses table from
// the previous TCP impl is gone — re-introduce a request/response API
// here when V1.5 backfills `Network.getResponseBody`).
// Drop into the event read loop. It returns when the in-process transport
// closes (webview gone) OR when the idle watchdog fires after
// `PUMP_IDLE_TIMEOUT` of no frames (stale/destroyed page target) — either
// way the outer loop re-attaches. The resilient pump also buffers bursts
// into an unbounded queue so a flood that overflows the broadcast ring
// isn't silently dropped. V1 doesn't issue any in-stream calls (responses
// table from the previous TCP impl is gone — re-introduce a
// request/response API here when V1.5 backfills `Network.getResponseBody`).
log::info!("[discord][{}] event pump started", account_id);
let mut ingest_state = DiscordIngestState::default();
cdp.pump_events(&session_id, |method, params| {
dispatch_event(app, account_id, method, params, &mut ingest_state);
})
.await
let pump_result = cdp
.pump_events_resilient(&session_id, PUMP_IDLE_TIMEOUT, |method, params| {
dispatch_event(app, account_id, method, params, &mut ingest_state);
})
.await;
// Detach the now-stale session before the outer loop re-attaches, so idle /
// lag-forced reconnects don't accumulate orphaned CDP sessions on the
// transport (mirrors the DOM-scan cleanup).
crate::cdp::detach_session(&mut cdp, &session_id).await;
pump_result
}
/// Pure pin → strict → relaxed target-selection core of
/// [`attach_account_target`]. Returns the chosen page target and whether it was
/// a strict fragment match (the caller pins only on `true`). Split out so the
/// resolution hierarchy is unit-testable without a live CDP transport.
fn resolve_page_target<'a>(
targets: &'a [crate::cdp::target::CdpTarget],
url_prefix: &str,
url_fragment: &str,
pinned_target_id: Option<&str>,
) -> Option<(&'a crate::cdp::target::CdpTarget, bool)> {
// 1. Pinned id (locked on a prior strict match) — survives the hash strip.
// Still require the prefix: a pinned tab can navigate off Discord while
// keeping its target id, and we must not keep scanning an off-prefix page.
if let Some(pid) = pinned_target_id {
if let Some(t) = targets
.iter()
.find(|t| t.id == pid && t.kind == "page" && t.url.starts_with(url_prefix))
{
return Some((t, false));
}
}
// 2. Strict fragment match — the only result that proves account ownership.
if let Some(t) = targets.iter().find(|t| {
t.kind == "page" && t.url.starts_with(url_prefix) && t.url.ends_with(url_fragment)
}) {
return Some((t, true));
}
// 3. Relaxed prefix-only — last resort; safe under per-account data-dir isolation.
targets
.iter()
.find(|t| t.kind == "page" && t.url.starts_with(url_prefix))
.map(|t| (t, false))
}
/// Resolve this account's page target, attach, and return the live
/// [`CdpConn`](crate::cdp::CdpConn) plus session id.
///
/// Discord's web client `replaceState`s to its canonical `/channels/...` URL
/// on boot, stripping the `#openhuman-account-<id>` fragment the webview was
/// opened with — so a strict `ends_with(fragment)` match only holds for the
/// first instant after navigation and fails forever after (the 4s settle delay
/// alone guarantees we attach *after* the strip). Mirrors the Slack scanner's
/// resolution hierarchy (`slack_scanner::scan_once`) via [`resolve_page_target`]:
///
/// 1. **Pinned target id** — once a strict match locked the id, prefer it
/// (still constrained to `url_prefix`). Survives the fragment strip and
/// keeps multi-account sessions from cross-wiring scanner A onto B's tab.
/// 2. **Strict fragment match** (`url_prefix` + `#openhuman-account-<id>`).
/// On hit, (re)pin the id into `pinned_target_id`.
/// 3. **Relaxed prefix-only match** — last resort. Per-account
/// `data_directory` isolation makes this safe for single-account setups;
/// never persisted into the pin (only a strict match proves ownership).
async fn attach_account_target<R: Runtime>(
app: &AppHandle<R>,
account_id: &str,
url_prefix: &str,
url_fragment: &str,
pinned_target_id: &mut Option<String>,
) -> Result<(crate::cdp::CdpConn, String), String> {
let mut cdp = crate::cdp::target::conn_for_account(app, account_id)?;
let targets_v = cdp.call("Target.getTargets", json!({}), None).await?;
let targets = crate::cdp::target::parse_targets(&targets_v);
let (page_target, is_strict) = resolve_page_target(
&targets,
url_prefix,
url_fragment,
pinned_target_id.as_deref(),
)
.ok_or_else(|| format!("no page target matching {url_prefix} fragment={url_fragment}"))?;
// (Re)pin on every live strict-fragment match — the one signal that proves
// this target is *this* account's. Refreshing (not just setting-once) lets a
// stale pin recover: after a renderer swap gives a new target id, the next
// strict match re-pins instead of being stuck on relaxed forever. Relaxed
// matches never feed the pin.
if is_strict && pinned_target_id.as_deref() != Some(page_target.id.as_str()) {
log::info!(
"[discord][{}] pinned to target_id={} (strict fragment match)",
account_id,
page_target.id
);
*pinned_target_id = Some(page_target.id.clone());
}
let target_id = page_target.id.clone();
let attach = cdp
.call(
"Target.attachToTarget",
json!({ "targetId": target_id, "flatten": true }),
None,
)
.await?;
let session = attach
.get("sessionId")
.and_then(|x| x.as_str())
.ok_or_else(|| "page attach missing sessionId".to_string())?
.to_string();
Ok((cdp, session))
}
// ---------- Event filter & emit ----------------------------------------------
@@ -990,8 +1119,17 @@ fn spawn_dom_poll<R: Runtime>(
let fragment = crate::cdp::target_url_fragment(&account_id);
sleep(Duration::from_secs(6)).await;
let mut last_hash: Option<u64> = None;
let mut pinned_target_id: Option<String> = None;
loop {
match dom_scan_once(&app, &account_id, &url_prefix, &fragment).await {
match dom_scan_once(
&app,
&account_id,
&url_prefix,
&fragment,
&mut pinned_target_id,
)
.await
{
Ok(scan) => {
if Some(scan.hash) != last_hash {
log::info!(
@@ -1027,15 +1165,10 @@ async fn dom_scan_once<R: Runtime>(
account_id: &str,
url_prefix: &str,
url_fragment: &str,
pinned_target_id: &mut Option<String>,
) -> Result<dom_snapshot::DomScan, String> {
let prefix = url_prefix.to_string();
let fragment = url_fragment.to_string();
let pred = move |t: &crate::cdp::target::CdpTarget| -> bool {
t.url.starts_with(&prefix) && t.url.ends_with(&fragment)
};
let (mut cdp, session) =
crate::cdp::target::connect_and_attach_matching_in_process::<R, _>(app, account_id, pred)
.await?;
attach_account_target(app, account_id, url_prefix, url_fragment, pinned_target_id).await?;
let scan = dom_snapshot::scan(&mut cdp, &session).await;
crate::cdp::detach_session(&mut cdp, &session).await;
scan
@@ -331,3 +331,103 @@ async fn registry_forget_all_is_repeatable_noop_after_drain() {
assert!(registry.started.lock().is_empty());
assert_all_cancelled(tasks).await;
}
// ---------- attach target resolution (pin → strict → relaxed) ----------------
fn page(id: &str, url: &str) -> crate::cdp::target::CdpTarget {
crate::cdp::target::CdpTarget {
id: id.to_string(),
kind: "page".to_string(),
url: url.to_string(),
title: String::new(),
}
}
const PFX: &str = "https://discord.com/";
const FRAG: &str = "#openhuman-account-acct-1";
#[test]
fn resolve_strict_fragment_match_is_pinnable() {
let targets = vec![
page("t-other", "https://discord.com/channels/@me"),
page(
"t-1",
"https://discord.com/channels/@me#openhuman-account-acct-1",
),
];
let (t, strict) = super::resolve_page_target(&targets, PFX, FRAG, None).unwrap();
assert_eq!(t.id, "t-1");
assert!(
strict,
"strict fragment match must report strict=true so caller pins"
);
}
#[test]
fn resolve_falls_back_to_relaxed_when_fragment_stripped() {
// Discord replaceState'd the hash away — only a prefix match remains.
let targets = vec![page("t-1", "https://discord.com/channels/@me/12345")];
let (t, strict) = super::resolve_page_target(&targets, PFX, FRAG, None).unwrap();
assert_eq!(t.id, "t-1");
assert!(
!strict,
"relaxed match must report strict=false so caller never pins it"
);
}
#[test]
fn resolve_prefers_pinned_id_over_strict_sibling() {
// Pin already locked to t-1 (fragment since stripped); a sibling tab still
// carries a strict fragment — the pin must win to avoid cross-wiring.
let targets = vec![
page(
"t-2",
"https://discord.com/channels/@me#openhuman-account-acct-1",
),
page("t-1", "https://discord.com/channels/@me/12345"),
];
let (t, strict) = super::resolve_page_target(&targets, PFX, FRAG, Some("t-1")).unwrap();
assert_eq!(t.id, "t-1");
assert!(!strict);
}
#[test]
fn resolve_ignores_stale_pin_and_recovers() {
// Pinned id no longer present (renderer crash → new target id). Resolution
// must skip the dead pin and fall through to strict/relaxed.
let targets = vec![page("t-new", "https://discord.com/channels/@me/9")];
let (t, strict) = super::resolve_page_target(&targets, PFX, FRAG, Some("t-gone")).unwrap();
assert_eq!(t.id, "t-new");
assert!(!strict);
}
#[test]
fn resolve_none_when_no_prefix_target() {
let targets = vec![
page("t-1", "https://slack.com/client/x"),
crate::cdp::target::CdpTarget {
id: "iframe".to_string(),
kind: "iframe".to_string(),
url: "https://discord.com/channels/@me".to_string(),
title: String::new(),
},
];
assert!(
super::resolve_page_target(&targets, PFX, FRAG, None).is_none(),
"no page-kind target under the prefix → None (non-page kinds excluded)"
);
}
#[test]
fn resolve_rejects_pinned_target_that_navigated_off_prefix() {
// A pinned tab can keep its target id while navigating away from Discord.
// The pin branch must still honor url_prefix, so an off-prefix pinned page is
// rejected and resolution falls through to the real Discord page.
let targets = vec![
page("t-1", "https://example.com/somewhere-else"), // pinned id, off-prefix
page("t-2", "https://discord.com/channels/@me/9"), // real discord page
];
let (t, strict) = super::resolve_page_target(&targets, PFX, FRAG, Some("t-1")).unwrap();
assert_eq!(t.id, "t-2");
assert!(!strict);
}