Skip to content

fix: socket conn handling #814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 29, 2025
+67 −89
Merged

fix: socket conn handling #814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5bb2b87
fix: rm deepest elem dialog filtering
RohitR311 Sep 29, 2025
0a914b0
fix: rm Run require model
RohitR311 Sep 29, 2025
6b7a1f8
fix: socket conn handling
RohitR311 Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion server/src/workflow-management/classes/Interpreter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,6 @@ export class WorkflowInterpreter {
try {
const sequelize = require('../../storage/db').default;
await sequelize.transaction(async (transaction: any) => {
const { Run } = require('../../models');
const run = await Run.findOne({
where: { runId: this.currentRunId! },
transaction
Expand Down Expand Up @@ -690,6 +689,10 @@ export class WorkflowInterpreter {
}
} finally {
this.persistenceInProgress = false;

if (this.persistenceBuffer.length > 0 && !this.persistenceTimer) {
this.scheduleBatchFlush();
}
}
};
}
33 changes: 31 additions & 2 deletions src/context/socket.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface SocketState {
queueSocket: Socket | null;
id: string;
setId: (id: string) => void;
connectToQueueSocket: (userId: string, onRunCompleted?: (data: any) => void) => void;
connectToQueueSocket: (userId: string, onRunCompleted?: (data: any) => void, onRunStarted?: (data: any) => void, onRunRecovered?: (data: any) => void, onRunScheduled?: (data: any) => void) => void;
disconnectQueueSocket: () => void;
};

Expand All @@ -29,6 +29,9 @@ export const SocketProvider = ({ children }: { children: JSX.Element }) => {
const [queueSocket, setQueueSocket] = useState<Socket | null>(socketStore.queueSocket);
const [id, setActiveId] = useState<string>(socketStore.id);
const runCompletedCallbackRef = useRef<((data: any) => void) | null>(null);
const runStartedCallbackRef = useRef<((data: any) => void) | null>(null);
const runRecoveredCallbackRef = useRef<((data: any) => void) | null>(null);
const runScheduledCallbackRef = useRef<((data: any) => void) | null>(null);

const setId = useCallback((id: string) => {
// the socket client connection is recomputed whenever id changes -> the new browser has been initialized
Expand All @@ -45,8 +48,11 @@ export const SocketProvider = ({ children }: { children: JSX.Element }) => {
setActiveId(id);
}, [setSocket]);

const connectToQueueSocket = useCallback((userId: string, onRunCompleted?: (data: any) => void) => {
const connectToQueueSocket = useCallback((userId: string, onRunCompleted?: (data: any) => void, onRunStarted?: (data: any) => void, onRunRecovered?: (data: any) => void, onRunScheduled?: (data: any) => void) => {
runCompletedCallbackRef.current = onRunCompleted || null;
runStartedCallbackRef.current = onRunStarted || null;
runRecoveredCallbackRef.current = onRunRecovered || null;
runScheduledCallbackRef.current = onRunScheduled || null;

const newQueueSocket = io(`${SERVER_ENDPOINT}/queued-run`, {
transports: ["websocket"],
Expand All @@ -69,6 +75,27 @@ export const SocketProvider = ({ children }: { children: JSX.Element }) => {
}
});

newQueueSocket.on('run-started', (startedData) => {
console.log('Run started event received:', startedData);
if (runStartedCallbackRef.current) {
runStartedCallbackRef.current(startedData);
}
});

newQueueSocket.on('run-recovered', (recoveredData) => {
console.log('Run recovered event received:', recoveredData);
if (runRecoveredCallbackRef.current) {
runRecoveredCallbackRef.current(recoveredData);
}
});

newQueueSocket.on('run-scheduled', (scheduledData) => {
console.log('Run scheduled event received:', scheduledData);
if (runScheduledCallbackRef.current) {
runScheduledCallbackRef.current(scheduledData);
}
});

setQueueSocket(currentSocket => {
if (currentSocket) {
currentSocket.disconnect();
Expand All @@ -89,6 +116,8 @@ export const SocketProvider = ({ children }: { children: JSX.Element }) => {

socketStore.queueSocket = null;
runCompletedCallbackRef.current = null;
runRecoveredCallbackRef.current = null;
runScheduledCallbackRef.current = null;
}, []);
Comment on lines 118 to 121
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Missed reset for runStarted callback on disconnect.

runStartedCallbackRef isn’t cleared, unlike the others. Reset it to avoid stale references.

   socketStore.queueSocket = null;
   runCompletedCallbackRef.current = null;
+  runStartedCallbackRef.current = null;
   runRecoveredCallbackRef.current = null;
   runScheduledCallbackRef.current = null;

You can also quickly scan for event-name consistency:

🏁 Script executed:

#!/bin/bash
rg -n "run-(completed|started|recovered|scheduled)" -C2

Length of output: 3870

🏁 Script executed:

sed -n '100,140p' src/context/socket.tsx

Length of output: 876

🏁 Script executed:

rg -n "runStartedCallbackRef" -C3 src/context/socket.tsx

Length of output: 1227

Reset runStartedCallbackRef in disconnectQueueSocket
Add runStartedCallbackRef.current = null alongside the other callback resets to avoid retaining a stale “run-started” handler.

--- a/src/context/socket.tsx
+++ b/src/context/socket.tsx
@@ const disconnectQueueSocket = useCallback(() => {…, []);  
-  socketStore.queueSocket = null;
-  runCompletedCallbackRef.current = null;
-  runRecoveredCallbackRef.current = null;
-  runScheduledCallbackRef.current = null;
+  socketStore.queueSocket = null;
+  runStartedCallbackRef.current = null;
+  runCompletedCallbackRef.current = null;
+  runRecoveredCallbackRef.current = null;
+  runScheduledCallbackRef.current = null;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
runCompletedCallbackRef.current = null;
runRecoveredCallbackRef.current = null;
runScheduledCallbackRef.current = null;
}, []);
const disconnectQueueSocket = useCallback(() => {
socketStore.queueSocket = null;
runStartedCallbackRef.current = null;
runCompletedCallbackRef.current = null;
runRecoveredCallbackRef.current = null;
runScheduledCallbackRef.current = null;
}, []);
🤖 Prompt for AI Agents 
In src/context/socket.tsx around lines 118 to 121, disconnectQueueSocket resets
several callback refs but omits runStartedCallbackRef, which can retain a stale
run-started handler; add a line setting runStartedCallbackRef.current = null
alongside runCompletedCallbackRef.current = null,
runRecoveredCallbackRef.current = null, and runScheduledCallbackRef.current =
null so all run-* callback refs are cleared on disconnect.


// Cleanup on unmount
Expand Down
84 changes: 0 additions & 84 deletions src/helpers/clientSelectorGenerator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3906,24 +3906,6 @@ class ClientSelectorGenerator {
let elements = iframeDoc.elementsFromPoint(x, y) as HTMLElement[];
if (!elements.length) return null;

const dialogElement = this.findDialogElement(elements);
if (dialogElement) {
const dialogRect = dialogElement.getBoundingClientRect();
const isClickInsideDialog = x >= dialogRect.left && x <= dialogRect.right &&
y >= dialogRect.top && y <= dialogRect.bottom;

if (isClickInsideDialog) {
const dialogElements = elements.filter(
(el) => el === dialogElement || dialogElement.contains(el)
);

const deepestInDialog = this.findDeepestInDialog(dialogElements, dialogElement);
if (deepestInDialog) {
return deepestInDialog;
}
}
}

const filteredElements = this.filterLogicalElements(elements, x, y);
const targetElements =
filteredElements.length > 0 ? filteredElements : elements;
Expand Down Expand Up @@ -4061,72 +4043,6 @@ class ClientSelectorGenerator {
return depth;
}

/**
* Find dialog element in the elements array
*/
private findDialogElement(elements: HTMLElement[]): HTMLElement | null {
let dialogElement = elements.find((el) => el.getAttribute("role") === "dialog");

if (!dialogElement) {
dialogElement = elements.find((el) => el.tagName.toLowerCase() === "dialog");
}

if (!dialogElement) {
dialogElement = elements.find((el) => {
const classList = el.classList.toString().toLowerCase();
const id = (el.id || "").toLowerCase();

return (
classList.includes("modal") ||
classList.includes("dialog") ||
classList.includes("popup") ||
classList.includes("overlay") ||
id.includes("modal") ||
id.includes("dialog") ||
id.includes("popup")
);
});
}

return dialogElement || null;
}

/**
* Find the deepest element within a dialog
*/
private findDeepestInDialog(
dialogElements: HTMLElement[],
dialogElement: HTMLElement
): HTMLElement | null {
if (!dialogElements.length) return null;
if (dialogElements.length === 1) return dialogElements[0];

let deepestElement = dialogElements[0];
let maxDepth = 0;

for (const element of dialogElements) {
let depth = 0;
let current = element;

// Calculate depth within the dialog context
while (
current &&
current.parentElement &&
current !== dialogElement.parentElement
) {
depth++;
current = current.parentElement;
}

if (depth > maxDepth) {
maxDepth = depth;
deepestElement = element;
}
}

return deepestElement;
}

/**
* Check if an element is a dialog
*/
Expand Down
34 changes: 32 additions & 2 deletions src/pages/MainPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,14 @@ export const MainPage = ({ handleEditRecording, initialContent }: MainPageProps)

useEffect(() => {
if (user?.id) {
const handleRunStarted = (startedData: any) => {
setRerenderRuns(true);
invalidateRuns();

const robotName = startedData.robotName || 'Unknown Robot';
notify('info', t('main_page.notifications.run_started', { name: robotName }));
};

const handleRunCompleted = (completionData: any) => {
setRerenderRuns(true);
invalidateRuns();
Expand All @@ -235,14 +243,36 @@ export const MainPage = ({ handleEditRecording, initialContent }: MainPageProps)
notify('error', t('main_page.notifications.interpretation_failed', { name: robotName }));
}
};

const handleRunRecovered = (recoveredData: any) => {
setRerenderRuns(true);
invalidateRuns();

if (queuedRuns.has(recoveredData.runId)) {
setQueuedRuns(prev => {
const newSet = new Set(prev);
newSet.delete(recoveredData.runId);
return newSet;
});
}

const robotName = recoveredData.robotName || 'Unknown Robot';
notify('error', t('main_page.notifications.interpretation_failed', { name: robotName }));
};

const handleRunScheduled = (scheduledData: any) => {
setRerenderRuns(true);
invalidateRuns();
};

connectToQueueSocket(user.id, handleRunCompleted);
connectToQueueSocket(user.id, handleRunCompleted, handleRunStarted, handleRunRecovered, handleRunScheduled);

return () => {
console.log('Disconnecting persistent queue socket for user:', user.id);
disconnectQueueSocket();
};
}
}, [user?.id, connectToQueueSocket, disconnectQueueSocket, t, setRerenderRuns, queuedRuns, setQueuedRuns, invalidateRuns]);
}, [user?.id, connectToQueueSocket, disconnectQueueSocket, t, setRerenderRuns, queuedRuns, setQueuedRuns]);

const DisplayContent = () => {
switch (content) {
Expand Down