I've fixed the bug in frontend/src/hooks/useLogStream.ts. Here's a summary of the issue and the fix: (#2027)
## Problem When multiple dev servers are running and the user switches between log tabs, logs from one dev server could appear in another dev server's tab. This happened because: 1. When `processId` changes (user switches tabs), the effect cleanup closes the old WebSocket 2. But WebSocket `close()` is asynchronous - the old WebSocket's event handlers can still fire 3. The old WebSocket's `onmessage` handler was still calling `setLogs()`, which would update the logs state even after switching to a different process ## Solution Added a `currentProcessIdRef` ref to track the current active `processId`, and a `capturedProcessId` variable that captures the processId when each WebSocket is opened. All event handlers now check if their captured processId matches the current one before updating state: 1. **`onopen`**: Closes the WebSocket immediately if processId has changed since opening 2. **`addLogEntry`**: Discards log entries if they're from a stale WebSocket 3. **`onerror`**: Ignores errors from stale WebSocket connections 4. **`onclose`**: Prevents retry logic from running for stale connections This ensures that each WebSocket connection only affects state when it's still the active connection for the currently selected process.
This commit is contained in:
@@ -15,17 +15,24 @@ export const useLogStream = (processId: string): UseLogStreamResult => {
|
||||
const retryCountRef = useRef<number>(0);
|
||||
const retryTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const isIntentionallyClosed = useRef<boolean>(false);
|
||||
// Track current processId to prevent stale WebSocket messages from contaminating logs
|
||||
const currentProcessIdRef = useRef<string>(processId);
|
||||
|
||||
useEffect(() => {
|
||||
if (!processId) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Update the ref to track the current processId
|
||||
currentProcessIdRef.current = processId;
|
||||
|
||||
// Clear logs when process changes
|
||||
setLogs([]);
|
||||
setError(null);
|
||||
|
||||
const open = () => {
|
||||
// Capture processId at the time of opening the WebSocket
|
||||
const capturedProcessId = processId;
|
||||
const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:';
|
||||
const host = window.location.host;
|
||||
const ws = new WebSocket(
|
||||
@@ -35,6 +42,11 @@ export const useLogStream = (processId: string): UseLogStreamResult => {
|
||||
isIntentionallyClosed.current = false;
|
||||
|
||||
ws.onopen = () => {
|
||||
// Ignore if processId has changed since WebSocket was opened
|
||||
if (currentProcessIdRef.current !== capturedProcessId) {
|
||||
ws.close();
|
||||
return;
|
||||
}
|
||||
setError(null);
|
||||
// Reset logs on new connection since server replays history
|
||||
setLogs([]);
|
||||
@@ -42,6 +54,10 @@ export const useLogStream = (processId: string): UseLogStreamResult => {
|
||||
};
|
||||
|
||||
const addLogEntry = (entry: LogEntry) => {
|
||||
// Only add log entry if this WebSocket is still for the current process
|
||||
if (currentProcessIdRef.current !== capturedProcessId) {
|
||||
return;
|
||||
}
|
||||
setLogs((prev) => [...prev, entry]);
|
||||
};
|
||||
|
||||
@@ -77,10 +93,18 @@ export const useLogStream = (processId: string): UseLogStreamResult => {
|
||||
};
|
||||
|
||||
ws.onerror = () => {
|
||||
// Ignore errors from stale WebSocket connections
|
||||
if (currentProcessIdRef.current !== capturedProcessId) {
|
||||
return;
|
||||
}
|
||||
setError('Connection failed');
|
||||
};
|
||||
|
||||
ws.onclose = (event) => {
|
||||
// Don't retry for stale WebSocket connections
|
||||
if (currentProcessIdRef.current !== capturedProcessId) {
|
||||
return;
|
||||
}
|
||||
// Only retry if the close was not intentional and not a normal closure
|
||||
if (!isIntentionallyClosed.current && event.code !== 1000) {
|
||||
const next = retryCountRef.current + 1;
|
||||
|
||||
Reference in New Issue
Block a user