Skip to content

Commit 7c02c08

Browse files
fix(rust-sdk): drop duplicate registration frames on first WS connect (#1616)
Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 8da5af9 commit 7c02c08

4 files changed

Lines changed: 717 additions & 12 deletions

File tree

sdk/packages/rust/iii/src/iii.rs

Lines changed: 241 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,12 +1309,44 @@ impl III {
13091309

13101310
queue.extend(self.collect_registrations());
13111311
Self::dedupe_registrations(&mut queue);
1312+
1313+
// Snapshot the registration keys we're about to send so
1314+
// we can drop duplicate copies still pending in `rx`.
1315+
// These are leftover from `register_*` calls made by user
1316+
// threads before the WS handshake completed: each call
1317+
// both inserts into the in-memory map (replayed via
1318+
// `collect_registrations`) AND queues into `outbound`.
1319+
let snapshot_ids: HashSet<String> =
1320+
queue.iter().filter_map(Self::registration_key).collect();
1321+
13121322
if let Err(err) = self.flush_queue(&mut ws_tx, &mut queue).await {
13131323
tracing::warn!(error = %err, "failed to flush queue");
13141324
sleep(Duration::from_secs(2)).await;
13151325
continue;
13161326
}
13171327

1328+
// Drain pre-connect leftovers from `rx`, dropping
1329+
// register duplicates and preserving everything else
1330+
// (invocations, results, channel ops, and any
1331+
// registrations added after the snapshot was taken).
1332+
let shutdown =
1333+
Self::drain_pre_connect_duplicates(&mut rx, &mut queue, &snapshot_ids);
1334+
if shutdown {
1335+
self.inner.running.store(false, Ordering::SeqCst);
1336+
return;
1337+
}
1338+
1339+
if !queue.is_empty() {
1340+
if let Err(err) = self.flush_queue(&mut ws_tx, &mut queue).await {
1341+
tracing::warn!(
1342+
error = %err,
1343+
"failed to flush post-drain queue"
1344+
);
1345+
sleep(Duration::from_secs(2)).await;
1346+
continue;
1347+
}
1348+
}
1349+
13181350
// Auto-register worker metadata on connect (like Node SDK)
13191351
self.register_worker_metadata();
13201352

@@ -1393,26 +1425,65 @@ impl III {
13931425
messages
13941426
}
13951427

1428+
/// Returns a stable identity key for a registration message, or `None`
1429+
/// for non-registration messages (invocations, ping/pong, etc.).
1430+
///
1431+
/// Used both to deduplicate within `queue` and to detect leftover
1432+
/// pre-connect register messages in `rx` whose state has already been
1433+
/// re-sent via `collect_registrations()`.
1434+
fn registration_key(message: &Message) -> Option<String> {
1435+
match message {
1436+
Message::RegisterTriggerType { id, .. } => Some(format!("trigger_type:{id}")),
1437+
Message::RegisterTrigger { id, .. } => Some(format!("trigger:{id}")),
1438+
Message::RegisterFunction { id, .. } => Some(format!("function:{id}")),
1439+
Message::RegisterService { id, .. } => Some(format!("service:{id}")),
1440+
_ => None,
1441+
}
1442+
}
1443+
1444+
/// Drain everything currently pending in the outbound `rx` channel,
1445+
/// dropping register messages whose keys are already covered by
1446+
/// `snapshot_ids` (already sent via `collect_registrations()`),
1447+
/// and pushing every other message onto `queue` for re-flushing.
1448+
///
1449+
/// Returns `true` if a `Shutdown` signal was observed during the
1450+
/// drain — the caller should then stop the connection loop.
1451+
fn drain_pre_connect_duplicates(
1452+
rx: &mut mpsc::UnboundedReceiver<Outbound>,
1453+
queue: &mut Vec<Message>,
1454+
snapshot_ids: &HashSet<String>,
1455+
) -> bool {
1456+
loop {
1457+
match rx.try_recv() {
1458+
Ok(Outbound::Message(msg)) => {
1459+
let is_dup = Self::registration_key(&msg)
1460+
.map(|k| snapshot_ids.contains(&k))
1461+
.unwrap_or(false);
1462+
if is_dup {
1463+
continue;
1464+
}
1465+
queue.push(msg);
1466+
}
1467+
Ok(Outbound::Shutdown) => return true,
1468+
Err(_) => return false,
1469+
}
1470+
}
1471+
}
1472+
13961473
fn dedupe_registrations(queue: &mut Vec<Message>) {
13971474
let mut seen = HashSet::new();
13981475
let mut deduped_rev = Vec::with_capacity(queue.len());
13991476

14001477
for message in queue.iter().rev() {
1401-
let key = match message {
1402-
Message::RegisterTriggerType { id, .. } => format!("trigger_type:{id}"),
1403-
Message::RegisterTrigger { id, .. } => format!("trigger:{id}"),
1404-
Message::RegisterFunction { id, .. } => {
1405-
format!("function:{id}")
1478+
match Self::registration_key(message) {
1479+
Some(key) => {
1480+
if seen.insert(key) {
1481+
deduped_rev.push(message.clone());
1482+
}
14061483
}
1407-
Message::RegisterService { id, .. } => format!("service:{id}"),
1408-
_ => {
1484+
None => {
14091485
deduped_rev.push(message.clone());
1410-
continue;
14111486
}
1412-
};
1413-
1414-
if seen.insert(key) {
1415-
deduped_rev.push(message.clone());
14161487
}
14171488
}
14181489

@@ -1958,4 +2029,162 @@ mod tests {
19582029

19592030
std::fs::remove_dir_all(&tmp).ok();
19602031
}
2032+
2033+
fn make_register_function(id: &str) -> Message {
2034+
Message::RegisterFunction {
2035+
id: id.to_string(),
2036+
description: None,
2037+
request_format: None,
2038+
response_format: None,
2039+
metadata: None,
2040+
invocation: None,
2041+
}
2042+
}
2043+
2044+
fn make_register_trigger(id: &str) -> Message {
2045+
Message::RegisterTrigger {
2046+
id: id.to_string(),
2047+
trigger_type: "demo".to_string(),
2048+
function_id: "fn".to_string(),
2049+
config: json!({}),
2050+
metadata: None,
2051+
}
2052+
}
2053+
2054+
fn make_register_trigger_type(id: &str) -> Message {
2055+
Message::RegisterTriggerType {
2056+
id: id.to_string(),
2057+
description: "tt".to_string(),
2058+
trigger_request_format: None,
2059+
call_request_format: None,
2060+
}
2061+
}
2062+
2063+
fn make_register_service(id: &str) -> Message {
2064+
Message::RegisterService {
2065+
id: id.to_string(),
2066+
name: "svc".to_string(),
2067+
description: None,
2068+
parent_service_id: None,
2069+
}
2070+
}
2071+
2072+
fn make_invoke(function_id: &str) -> Message {
2073+
Message::InvokeFunction {
2074+
invocation_id: None,
2075+
function_id: function_id.to_string(),
2076+
data: json!({}),
2077+
traceparent: None,
2078+
baggage: None,
2079+
action: None,
2080+
}
2081+
}
2082+
2083+
#[test]
2084+
fn registration_key_returns_typed_keys_for_register_messages() {
2085+
assert_eq!(
2086+
III::registration_key(&make_register_function("greet")),
2087+
Some("function:greet".to_string())
2088+
);
2089+
assert_eq!(
2090+
III::registration_key(&make_register_trigger("t1")),
2091+
Some("trigger:t1".to_string())
2092+
);
2093+
assert_eq!(
2094+
III::registration_key(&make_register_trigger_type("tt1")),
2095+
Some("trigger_type:tt1".to_string())
2096+
);
2097+
assert_eq!(
2098+
III::registration_key(&make_register_service("svc1")),
2099+
Some("service:svc1".to_string())
2100+
);
2101+
}
2102+
2103+
#[test]
2104+
fn registration_key_returns_none_for_non_register_messages() {
2105+
assert_eq!(III::registration_key(&make_invoke("f")), None);
2106+
assert_eq!(III::registration_key(&Message::Ping), None);
2107+
assert_eq!(III::registration_key(&Message::Pong), None);
2108+
assert_eq!(
2109+
III::registration_key(&Message::WorkerRegistered {
2110+
worker_id: "w".to_string()
2111+
}),
2112+
None
2113+
);
2114+
}
2115+
2116+
#[tokio::test]
2117+
async fn drain_pre_connect_duplicates_drops_only_known_register_ids() {
2118+
let (tx, mut rx) = mpsc::unbounded_channel::<Outbound>();
2119+
2120+
tx.send(Outbound::Message(make_register_function("dup-fn")))
2121+
.unwrap();
2122+
tx.send(Outbound::Message(make_invoke("some::fn"))).unwrap();
2123+
tx.send(Outbound::Message(make_register_function("new-fn")))
2124+
.unwrap();
2125+
tx.send(Outbound::Message(Message::Pong)).unwrap();
2126+
tx.send(Outbound::Message(make_register_trigger("dup-trig")))
2127+
.unwrap();
2128+
tx.send(Outbound::Message(make_register_trigger("new-trig")))
2129+
.unwrap();
2130+
tx.send(Outbound::Message(make_register_service("dup-svc")))
2131+
.unwrap();
2132+
2133+
let snapshot_ids: HashSet<String> = [
2134+
"function:dup-fn".to_string(),
2135+
"trigger:dup-trig".to_string(),
2136+
"service:dup-svc".to_string(),
2137+
]
2138+
.into_iter()
2139+
.collect();
2140+
2141+
let mut queue: Vec<Message> = Vec::new();
2142+
let shutdown = III::drain_pre_connect_duplicates(&mut rx, &mut queue, &snapshot_ids);
2143+
2144+
assert!(!shutdown);
2145+
let kept_keys: Vec<Option<String>> = queue.iter().map(III::registration_key).collect();
2146+
assert_eq!(
2147+
kept_keys,
2148+
vec![
2149+
None,
2150+
Some("function:new-fn".to_string()),
2151+
None,
2152+
Some("trigger:new-trig".to_string()),
2153+
],
2154+
"kept queue mismatch: {queue:#?}"
2155+
);
2156+
}
2157+
2158+
#[tokio::test]
2159+
async fn drain_pre_connect_duplicates_signals_shutdown() {
2160+
let (tx, mut rx) = mpsc::unbounded_channel::<Outbound>();
2161+
2162+
tx.send(Outbound::Message(make_register_function("a")))
2163+
.unwrap();
2164+
tx.send(Outbound::Shutdown).unwrap();
2165+
tx.send(Outbound::Message(make_register_function("b")))
2166+
.unwrap();
2167+
2168+
let snapshot_ids: HashSet<String> = ["function:a".to_string()].into_iter().collect();
2169+
let mut queue: Vec<Message> = Vec::new();
2170+
let shutdown = III::drain_pre_connect_duplicates(&mut rx, &mut queue, &snapshot_ids);
2171+
2172+
assert!(shutdown, "expected shutdown signal to be reported");
2173+
assert!(
2174+
queue.is_empty(),
2175+
"queue must be empty when shutdown short-circuits the drain: {queue:#?}"
2176+
);
2177+
}
2178+
2179+
#[tokio::test]
2180+
async fn drain_pre_connect_duplicates_returns_false_on_empty_channel() {
2181+
let (_tx, mut rx) = mpsc::unbounded_channel::<Outbound>();
2182+
let snapshot_ids: HashSet<String> = HashSet::new();
2183+
let mut queue: Vec<Message> = Vec::new();
2184+
2185+
let shutdown = III::drain_pre_connect_duplicates(&mut rx, &mut queue, &snapshot_ids);
2186+
2187+
assert!(!shutdown);
2188+
assert!(queue.is_empty());
2189+
}
19612190
}

0 commit comments

Comments
 (0)