sync/race problems + dropped frames on exit problems
This commit is contained in:
6 files changed
+176
-23
No files matched your search
+52
-10
@@ -113,30 +113,72 @@ export function App() {
|
|||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const wsUrl = `${window.location.protocol === "https:" ? "wss" : "ws"}://${window.location.host}/api/v1/control/alerts/ws`;
|
let socket: WebSocket | null = null;
|
||||||
const ws = new WebSocket(wsUrl);
|
let disposed = false;
|
||||||
|
let attempt = 0;
|
||||||
|
let openedAt = 0;
|
||||||
|
let wsGeneration = 0;
|
||||||
|
let reconnectTimer: number | undefined;
|
||||||
|
|
||||||
ws.onmessage = (evt) => {
|
const resync = () => {
|
||||||
|
const generation = wsGeneration;
|
||||||
|
void Promise.all([fetchAlerts(), fetchAlertHistory(20)])
|
||||||
|
.then(([nextAlerts, nextHistory]) => {
|
||||||
|
// A WS snapshot delivered while this request was in flight is newer.
|
||||||
|
if (generation !== wsGeneration) return;
|
||||||
|
setAlerts(nextAlerts);
|
||||||
|
setHistory(nextHistory);
|
||||||
|
})
|
||||||
|
.catch(() => undefined);
|
||||||
|
};
|
||||||
|
|
||||||
|
const connect = () => {
|
||||||
|
if (disposed) return;
|
||||||
|
const wsUrl = `${window.location.protocol === "https:" ? "wss" : "ws"}://${window.location.host}/api/v1/control/alerts/ws`;
|
||||||
|
socket = new WebSocket(wsUrl);
|
||||||
|
|
||||||
|
socket.onopen = () => {
|
||||||
|
openedAt = Date.now();
|
||||||
|
// Pull anything missed while the stream was down.
|
||||||
|
resync();
|
||||||
|
};
|
||||||
|
|
||||||
|
socket.onmessage = (evt) => {
|
||||||
|
wsGeneration += 1;
|
||||||
try {
|
try {
|
||||||
const payload = JSON.parse(String(evt.data)) as {
|
const payload = JSON.parse(String(evt.data)) as {
|
||||||
alerts?: Alert[];
|
alerts?: Alert[];
|
||||||
recent_transitions?: AlertTransition[];
|
recent_transitions?: AlertTransition[];
|
||||||
};
|
};
|
||||||
if (payload.alerts) setAlerts(payload.alerts);
|
if (payload.alerts) setAlerts(payload.alerts);
|
||||||
if (payload.recent_transitions) setHistory(payload.recent_transitions);
|
if (payload.recent_transitions)
|
||||||
|
setHistory(payload.recent_transitions);
|
||||||
} catch {
|
} catch {
|
||||||
// Ignore malformed stream payloads and keep current UI state.
|
// Ignore malformed stream payloads and keep current UI state.
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
ws.onerror = () => {
|
socket.onclose = () => {
|
||||||
const id = window.setInterval(() => {
|
socket = null;
|
||||||
void refreshAlertsAndHistory().catch(() => undefined);
|
if (disposed) return;
|
||||||
}, 8000);
|
// Only a connection that stayed up proves the stream is healthy; an
|
||||||
ws.onclose = () => window.clearInterval(id);
|
// accept-then-close server must not pin us to the first backoff step.
|
||||||
|
if (openedAt && Date.now() - openedAt > 10_000) attempt = 0;
|
||||||
|
// Browsers never reconnect a WebSocket on their own.
|
||||||
|
const delay =
|
||||||
|
Math.min(30_000, 1000 * 2 ** attempt) + Math.random() * 1000;
|
||||||
|
attempt += 1;
|
||||||
|
reconnectTimer = window.setTimeout(connect, delay);
|
||||||
|
};
|
||||||
};
|
};
|
||||||
|
|
||||||
return () => ws.close();
|
connect();
|
||||||
|
|
||||||
|
return () => {
|
||||||
|
disposed = true;
|
||||||
|
if (reconnectTimer !== undefined) window.clearTimeout(reconnectTimer);
|
||||||
|
socket?.close();
|
||||||
|
};
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { useEffect, useMemo, useState } from "react";
|
import { useEffect, useMemo, useRef, useState } from "react";
|
||||||
import { Inbox, MoreHorizontal, RefreshCw, Search, Zap } from "lucide-react";
|
import { Inbox, MoreHorizontal, RefreshCw, Search, Zap } from "lucide-react";
|
||||||
import { toast } from "sonner";
|
import { toast } from "sonner";
|
||||||
|
|
||||||
@@ -69,8 +69,12 @@ export function DevicesPage({ agents, onAfterWake, onRefresh }: Props) {
|
|||||||
const [refreshing, setRefreshing] = useState(false);
|
const [refreshing, setRefreshing] = useState(false);
|
||||||
const [wakeBusyKey, setWakeBusyKey] = useState("");
|
const [wakeBusyKey, setWakeBusyKey] = useState("");
|
||||||
const [details, setDetails] = useState<FleetDevice | null>(null);
|
const [details, setDetails] = useState<FleetDevice | null>(null);
|
||||||
|
const loadSeqRef = useRef(0);
|
||||||
|
|
||||||
async function loadFleet() {
|
async function loadFleet() {
|
||||||
|
// Latest request wins: filter changes and debounced searches overlap, and
|
||||||
|
// an older response must never overwrite fresher state.
|
||||||
|
const seq = ++loadSeqRef.current;
|
||||||
setLoading(true);
|
setLoading(true);
|
||||||
try {
|
try {
|
||||||
const [nextDevices, nextKnownDevices] = await Promise.all([
|
const [nextDevices, nextKnownDevices] = await Promise.all([
|
||||||
@@ -84,12 +88,14 @@ export function DevicesPage({ agents, onAfterWake, onRefresh }: Props) {
|
|||||||
}),
|
}),
|
||||||
fetchKnownDevices(),
|
fetchKnownDevices(),
|
||||||
]);
|
]);
|
||||||
|
if (seq !== loadSeqRef.current) return;
|
||||||
setDevices(nextDevices);
|
setDevices(nextDevices);
|
||||||
setKnownDevices(nextKnownDevices);
|
setKnownDevices(nextKnownDevices);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
if (seq !== loadSeqRef.current) return;
|
||||||
toast.error("Failed to load fleet", { description: String(err) });
|
toast.error("Failed to load fleet", { description: String(err) });
|
||||||
} finally {
|
} finally {
|
||||||
setLoading(false);
|
if (seq === loadSeqRef.current) setLoading(false);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -152,7 +158,13 @@ export function DevicesPage({ agents, onAfterWake, onRefresh }: Props) {
|
|||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [presence, known, agentId]);
|
}, [presence, known, agentId]);
|
||||||
|
|
||||||
|
// The filter effect above already loads on mount; only react to query edits.
|
||||||
|
const mountedRef = useRef(false);
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
if (!mountedRef.current) {
|
||||||
|
mountedRef.current = true;
|
||||||
|
return;
|
||||||
|
}
|
||||||
const timer = window.setTimeout(() => void loadFleet(), 220);
|
const timer = window.setTimeout(() => void loadFleet(), 220);
|
||||||
return () => window.clearTimeout(timer);
|
return () => window.clearTimeout(timer);
|
||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
|
|||||||
@@ -105,6 +105,7 @@ export function TerminalPage({
|
|||||||
const modifiersRef = useRef<TerminalModifiers>(NO_TERMINAL_MODIFIERS);
|
const modifiersRef = useRef<TerminalModifiers>(NO_TERMINAL_MODIFIERS);
|
||||||
const terminalTouchRef = useRef<TerminalTouch | null>(null);
|
const terminalTouchRef = useRef<TerminalTouch | null>(null);
|
||||||
const terminalInputFocusedBeforeAccessoryRef = useRef(false);
|
const terminalInputFocusedBeforeAccessoryRef = useRef(false);
|
||||||
|
const refreshSessionsRef = useRef<() => void>(() => {});
|
||||||
const [sessions, setSessions] = useState<TerminalSession[]>([]);
|
const [sessions, setSessions] = useState<TerminalSession[]>([]);
|
||||||
const [session, setSession] = useState<TerminalSession | null>(null);
|
const [session, setSession] = useState<TerminalSession | null>(null);
|
||||||
const [connection, setConnection] = useState<TerminalConnectionState>("idle");
|
const [connection, setConnection] = useState<TerminalConnectionState>("idle");
|
||||||
@@ -348,6 +349,7 @@ export function TerminalPage({
|
|||||||
};
|
};
|
||||||
if (control.type === "ready") {
|
if (control.type === "ready") {
|
||||||
setConnection("ready");
|
setConnection("ready");
|
||||||
|
refreshSessionsRef.current();
|
||||||
window.requestAnimationFrame(() => {
|
window.requestAnimationFrame(() => {
|
||||||
fitRef.current?.fit();
|
fitRef.current?.fit();
|
||||||
lastTerminalSizeRef.current = { rows: 0, cols: 0 };
|
lastTerminalSizeRef.current = { rows: 0, cols: 0 };
|
||||||
@@ -387,6 +389,8 @@ export function TerminalPage({
|
|||||||
if (current === "exited") return current;
|
if (current === "exited") return current;
|
||||||
return "disconnected";
|
return "disconnected";
|
||||||
});
|
});
|
||||||
|
// The agent may have reaped or reported sessions while we were away.
|
||||||
|
refreshSessionsRef.current();
|
||||||
};
|
};
|
||||||
},
|
},
|
||||||
[detachTransport, operatorId, sendResize],
|
[detachTransport, operatorId, sendResize],
|
||||||
@@ -572,7 +576,7 @@ export function TerminalPage({
|
|||||||
});
|
});
|
||||||
resizeObserver.observe(hostRef.current);
|
resizeObserver.observe(hostRef.current);
|
||||||
void restoreTerminalSession();
|
void restoreTerminalSession();
|
||||||
const sessionRefresh = window.setInterval(() => {
|
const refreshSessions = () => {
|
||||||
void listTerminals()
|
void listTerminals()
|
||||||
.then((listed) => {
|
.then((listed) => {
|
||||||
if (!cancelled) {
|
if (!cancelled) {
|
||||||
@@ -585,15 +589,20 @@ export function TerminalPage({
|
|||||||
// The attached terminal transport remains authoritative while a
|
// The attached terminal transport remains authoritative while a
|
||||||
// background list refresh is temporarily unavailable.
|
// background list refresh is temporarily unavailable.
|
||||||
});
|
});
|
||||||
}, 5000);
|
};
|
||||||
|
refreshSessionsRef.current = refreshSessions;
|
||||||
|
// Session lifecycle only changes at connect/disconnect moments or when the
|
||||||
|
// operator returns to the tab; refresh on those instead of polling.
|
||||||
|
window.addEventListener("focus", refreshSessions);
|
||||||
|
|
||||||
return () => {
|
return () => {
|
||||||
cancelled = true;
|
cancelled = true;
|
||||||
input.dispose();
|
input.dispose();
|
||||||
titleChange.dispose();
|
titleChange.dispose();
|
||||||
resizeObserver.disconnect();
|
resizeObserver.disconnect();
|
||||||
|
window.removeEventListener("focus", refreshSessions);
|
||||||
|
refreshSessionsRef.current = () => {};
|
||||||
window.cancelAnimationFrame(resizeFrame);
|
window.cancelAnimationFrame(resizeFrame);
|
||||||
window.clearInterval(sessionRefresh);
|
|
||||||
detachTransport();
|
detachTransport();
|
||||||
terminal.dispose();
|
terminal.dispose();
|
||||||
terminalRef.current = null;
|
terminalRef.current = null;
|
||||||
|
|||||||
@@ -150,10 +150,14 @@ export function TerminalSessionTabs({
|
|||||||
<button
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
className="terminal-tab-close"
|
className="terminal-tab-close"
|
||||||
disabled={locked}
|
disabled={locked || connection === "connecting"}
|
||||||
aria-label={`Close ${agentLabel} terminal session`}
|
aria-label={`Close ${agentLabel} terminal session`}
|
||||||
title={
|
title={
|
||||||
locked ? "Attached in another browser" : "Close session"
|
locked
|
||||||
|
? "Attached in another browser"
|
||||||
|
: connection === "connecting"
|
||||||
|
? "Connecting — wait before closing"
|
||||||
|
: "Close session"
|
||||||
}
|
}
|
||||||
onClick={() => onClose(item)}
|
onClick={() => onClose(item)}
|
||||||
>
|
>
|
||||||
|
|||||||
@@ -109,6 +109,10 @@ impl RelayOutputBuffer {
|
|||||||
self.snapshot_pending = false;
|
self.snapshot_pending = false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn live_sender(&self) -> Option<mpsc::Sender<Message>> {
|
||||||
|
self.sender.clone()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl TerminalState {
|
impl TerminalState {
|
||||||
@@ -465,12 +469,12 @@ async fn run_terminal(
|
|||||||
None => terminate_process_group(&mut child, process_group).await?,
|
None => terminate_process_group(&mut child, process_group).await?,
|
||||||
};
|
};
|
||||||
if relay_output.is_connected() {
|
if relay_output.is_connected() {
|
||||||
let control = TerminalControl::Exited {
|
let exit_frame = serde_json::to_string(&TerminalControl::Exited {
|
||||||
exit_code: status.code(),
|
exit_code: status.code(),
|
||||||
};
|
})
|
||||||
if let Ok(text) = serde_json::to_string(&control) {
|
.ok()
|
||||||
relay_output.queue_frame(Message::Text(text.into()));
|
.map(|text| Message::Text(text.into()));
|
||||||
}
|
flush_relay_output(&mut relay_output, exit_frame, PTY_EXIT_DRAIN_TIMEOUT).await;
|
||||||
}
|
}
|
||||||
info!(terminal_id = %terminal_id, exit_code = ?status.code(), requested_close, ttl_expired, "terminal worker exited");
|
info!(terminal_id = %terminal_id, exit_code = ?status.code(), requested_close, ttl_expired, "terminal worker exited");
|
||||||
Ok(())
|
Ok(())
|
||||||
@@ -600,6 +604,43 @@ async fn reserve_relay_output(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Flushes frames the select loop left pending, then sends the exit lifecycle
|
||||||
|
/// frame behind them, bounded by `deadline`: a relay that stays saturated past
|
||||||
|
/// it drops the backlog rather than holding worker shutdown open. Queuing the
|
||||||
|
/// exit frame without this would drop it together with the pending buffer when
|
||||||
|
/// the worker returns.
|
||||||
|
async fn flush_relay_output(
|
||||||
|
relay_output: &mut RelayOutputBuffer,
|
||||||
|
exit_frame: Option<Message>,
|
||||||
|
deadline: Duration,
|
||||||
|
) {
|
||||||
|
let _ = tokio::time::timeout(deadline, async {
|
||||||
|
while relay_output.has_pending() {
|
||||||
|
let sender = relay_output.pending_sender();
|
||||||
|
match reserve_relay_output(sender).await {
|
||||||
|
Ok(permit) => relay_output.send_reserved(permit),
|
||||||
|
Err(()) => {
|
||||||
|
relay_output.disconnect();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let Some(frame) = exit_frame else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let sender = relay_output
|
||||||
|
.pending_sender()
|
||||||
|
.or_else(|| relay_output.live_sender());
|
||||||
|
match reserve_relay_output(sender).await {
|
||||||
|
Ok(permit) => {
|
||||||
|
permit.send(frame);
|
||||||
|
}
|
||||||
|
Err(()) => relay_output.disconnect(),
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(not(unix))]
|
#[cfg(not(unix))]
|
||||||
async fn run_terminal(
|
async fn run_terminal(
|
||||||
_config: &AgentConfig,
|
_config: &AgentConfig,
|
||||||
@@ -786,6 +827,50 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn exit_frame_is_delivered_after_saturated_relay_drains() {
|
||||||
|
let (tx, mut rx) = mpsc::channel(RELAY_OUTPUT_QUEUE);
|
||||||
|
let mut relay_output = RelayOutputBuffer::default();
|
||||||
|
relay_output.connect(tx);
|
||||||
|
|
||||||
|
let total = RELAY_OUTPUT_QUEUE * 2;
|
||||||
|
for i in 0..total {
|
||||||
|
relay_output.queue_frame(Message::Text(format!("out-{i}").into()));
|
||||||
|
}
|
||||||
|
assert!(relay_output.has_pending());
|
||||||
|
|
||||||
|
let collected = tokio::spawn(async move {
|
||||||
|
let mut frames = Vec::new();
|
||||||
|
while let Some(frame) = rx.recv().await {
|
||||||
|
frames.push(frame);
|
||||||
|
}
|
||||||
|
frames
|
||||||
|
});
|
||||||
|
|
||||||
|
let exit_frame = serde_json::to_string(&TerminalControl::Exited { exit_code: Some(0) })
|
||||||
|
.expect("exit frame encodes");
|
||||||
|
flush_relay_output(
|
||||||
|
&mut relay_output,
|
||||||
|
Some(Message::Text(exit_frame.clone().into())),
|
||||||
|
PTY_EXIT_DRAIN_TIMEOUT,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
relay_output.disconnect();
|
||||||
|
|
||||||
|
let mut frames = collected.await.expect("relay reader completes");
|
||||||
|
assert_eq!(frames.len(), total + 1);
|
||||||
|
let Some(Message::Text(last)) = frames.pop() else {
|
||||||
|
panic!("last frame should be the exit control frame");
|
||||||
|
};
|
||||||
|
assert_eq!(last.as_str(), exit_frame);
|
||||||
|
for (i, frame) in frames.iter().enumerate() {
|
||||||
|
let Message::Text(payload) = frame else {
|
||||||
|
panic!("expected text frame at position {i}, got {frame:?}");
|
||||||
|
};
|
||||||
|
assert_eq!(payload.as_str(), format!("out-{i}"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn open_rejects_invalid_initial_size_before_spawning() {
|
fn open_rejects_invalid_initial_size_before_spawning() {
|
||||||
let mut config = crate::config::DEFAULT_CONFIG.clone();
|
let mut config = crate::config::DEFAULT_CONFIG.clone();
|
||||||
|
|||||||
@@ -191,7 +191,8 @@ async fn alerts_stream_socket(state: AppState, mut socket: WebSocket) {
|
|||||||
let mut rx = state.alerts.subscribe();
|
let mut rx = state.alerts.subscribe();
|
||||||
|
|
||||||
if let Some(encoded) = state.alerts.latest_payload().await
|
if let Some(encoded) = state.alerts.latest_payload().await
|
||||||
&& socket.send(Message::Text(encoded.into())).await.is_err() {
|
&& socket.send(Message::Text(encoded.into())).await.is_err()
|
||||||
|
{
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user