Skip to content

Commit eaf4034

Browse files
amhsirakamhsirak
authored
Merge pull request #734 from getmaxun/browser-ready
feat: browser checks for run execution
2 parents 422c575 + fe4a12c commit eaf4034

File tree

4 files changed

+109
-71
lines changed

4 files changed

+109
-71
lines changed

‎server/src/browser-management/classes/BrowserPool.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,12 @@ export class BrowserPool {
221221
return undefined;
222222
}
223223

224+
// Return undefined for failed slots
225+
if (poolInfo.status === "failed") {
226+
logger.log('debug', `Browser ${id} has failed status`);
227+
return undefined;
228+
}
229+
224230
return poolInfo.browser || undefined;
225231
};
226232

@@ -607,8 +613,13 @@ export class BrowserPool {
607613
* @returns true if successful, false if slot wasn't reserved
608614
*/
609615
public upgradeBrowserSlot = (id: string, browser: RemoteBrowser): boolean => {
610-
if (!this.pool[id] || this.pool[id].status !== "reserved") {
611-
logger.log('warn', `Cannot upgrade browser ${id}: slot not reserved`);
616+
if (!this.pool[id]) {
617+
logger.log('warn', `Cannot upgrade browser ${id}: slot does not exist in pool`);
618+
return false;
619+
}
620+
621+
if (this.pool[id].status !== "reserved") {
622+
logger.log('warn', `Cannot upgrade browser ${id}: slot not in reserved state (current: ${this.pool[id].status})`);
612623
return false;
613624
}
614625

@@ -629,4 +640,17 @@ export class BrowserPool {
629640
this.deleteRemoteBrowser(id);
630641
}
631642
};
643+
644+
/**
645+
* Gets the current status of a browser slot.
646+
*
647+
* @param id browser ID to check
648+
* @returns the status or null if browser doesn't exist
649+
*/
650+
public getBrowserStatus = (id: string): "reserved" | "initializing" | "ready" | "failed" | null => {
651+
if (!this.pool[id]) {
652+
return null;
653+
}
654+
return this.pool[id].status || null;
655+
};
632656
}

‎server/src/browser-management/controller.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ const initializeBrowserAsync = async (id: string, userId: string) => {
242242
logger.log('warn', `No client connected to browser ${id} within timeout, proceeding with dummy socket`);
243243
resolve(null);
244244
}
245-
}, 10000);
245+
}, 15000);
246246
});
247247

248248
namespace.on('error', (error: any) => {
@@ -272,21 +272,25 @@ const initializeBrowserAsync = async (id: string, userId: string) => {
272272
browserSession = new RemoteBrowser(dummySocket, userId, id);
273273
}
274274

275+
logger.log('debug', `Starting browser initialization for ${id}`);
275276
await browserSession.initialize(userId);
277+
logger.log('debug', `Browser initialization completed for ${id}`);
276278

277279
const upgraded = browserPool.upgradeBrowserSlot(id, browserSession);
278280
if (!upgraded) {
279281
throw new Error('Failed to upgrade reserved browser slot');
280282
}
281283

284+
await new Promise(resolve => setTimeout(resolve, 500));
285+
282286
if (socket) {
283287
socket.emit('ready-for-run');
284288
} else {
285289
setTimeout(async () => {
286290
try {
287-
logger.log('info', `Starting execution for browser ${id} with dummy socket`);
291+
logger.log('info', `Browser ${id} with dummy socket is ready for execution`);
288292
} catch (error: any) {
289-
logger.log('error', `Error executing run for browser ${id}: ${error.message}`);
293+
logger.log('error', `Error with dummy socket browser ${id}: ${error.message}`);
290294
}
291295
}, 100);
292296
}
@@ -299,10 +303,12 @@ const initializeBrowserAsync = async (id: string, userId: string) => {
299303
if (socket) {
300304
socket.emit('error', { message: error.message });
301305
}
306+
throw error;
302307
}
303308

304309
} catch (error: any) {
305310
logger.log('error', `Error setting up browser ${id}: ${error.message}`);
306311
browserPool.failBrowserSlot(id);
312+
throw error;
307313
}
308314
};

‎server/src/pgboss-worker.ts‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ async function triggerIntegrationUpdates(runId: string, robotMetaId: string): Pr
215215
* Modified processRunExecution function - only add browser reset
216216
*/
217217
async function processRunExecution(job: Job<ExecuteRunData>) {
218-
const BROWSER_INIT_TIMEOUT = 30000;
218+
const BROWSER_INIT_TIMEOUT = 60000;
219+
const BROWSER_PAGE_TIMEOUT = 45000;
219220

220221
const data = job.data;
221222
logger.log('info', `Processing run execution job for runId: ${data.runId}, browserId: ${data.browserId}`);
@@ -244,15 +245,28 @@ async function processRunExecution(job: Job<ExecuteRunData>) {
244245

245246
let browser = browserPool.getRemoteBrowser(browserId);
246247
const browserWaitStart = Date.now();
248+
let lastLogTime = 0;
247249

248250
while (!browser && (Date.now() - browserWaitStart) < BROWSER_INIT_TIMEOUT) {
249-
logger.log('debug', `Browser ${browserId} not ready yet, waiting...`);
250-
await new Promise(resolve => setTimeout(resolve, 1000));
251+
const currentTime = Date.now();
252+
253+
const browserStatus = browserPool.getBrowserStatus(browserId);
254+
if (browserStatus === null) {
255+
throw new Error(`Browser slot ${browserId} does not exist in pool`);
256+
}
257+
258+
if (currentTime - lastLogTime > 10000) {
259+
logger.log('info', `Browser ${browserId} not ready yet (status: ${browserStatus}), waiting... (${Math.round((currentTime - browserWaitStart) / 1000)}s elapsed)`);
260+
lastLogTime = currentTime;
261+
}
262+
263+
await new Promise(resolve => setTimeout(resolve, 2000));
251264
browser = browserPool.getRemoteBrowser(browserId);
252265
}
253266

254267
if (!browser) {
255-
throw new Error(`Browser ${browserId} not found in pool after timeout`);
268+
const finalStatus = browserPool.getBrowserStatus(browserId);
269+
throw new Error(`Browser ${browserId} not found in pool after ${BROWSER_INIT_TIMEOUT/1000}s timeout (final status: ${finalStatus})`);
256270
}
257271

258272
logger.log('info', `Browser ${browserId} found and ready for execution`);
@@ -273,14 +287,22 @@ async function processRunExecution(job: Job<ExecuteRunData>) {
273287
let currentPage = browser.getCurrentPage();
274288

275289
const pageWaitStart = Date.now();
276-
while (!currentPage && (Date.now() - pageWaitStart) < 30000) {
277-
logger.log('debug', `Page not ready for browser ${browserId}, waiting...`);
290+
let lastPageLogTime = 0;
291+
292+
while (!currentPage && (Date.now() - pageWaitStart) < BROWSER_PAGE_TIMEOUT) {
293+
const currentTime = Date.now();
294+
295+
if (currentTime - lastPageLogTime > 5000) {
296+
logger.log('info', `Page not ready for browser ${browserId}, waiting... (${Math.round((currentTime - pageWaitStart) / 1000)}s elapsed)`);
297+
lastPageLogTime = currentTime;
298+
}
299+
278300
await new Promise(resolve => setTimeout(resolve, 1000));
279301
currentPage = browser.getCurrentPage();
280302
}
281303

282304
if (!currentPage) {
283-
throw new Error(`No current page available for browser ${browserId} after timeout`);
305+
throw new Error(`No current page available for browser ${browserId} after ${BROWSER_PAGE_TIMEOUT/1000}s timeout`);
284306
}
285307

286308
logger.log('info', `Starting workflow execution for run ${data.runId}`);
@@ -775,6 +797,10 @@ async function registerRunExecutionWorker() {
775797
};
776798

777799
await checkForNewUserQueues();
800+
801+
setInterval(async () => {
802+
await checkForNewUserQueues();
803+
}, 10000);
778804

779805
logger.log('info', 'Run execution worker registered successfully');
780806
} catch (error: unknown) {
@@ -821,6 +847,10 @@ async function registerAbortRunWorker() {
821847
};
822848

823849
await checkForNewAbortQueues();
850+
851+
setInterval(async () => {
852+
await checkForNewAbortQueues();
853+
}, 10000);
824854

825855
logger.log('info', 'Abort run worker registration system initialized');
826856
} catch (error: unknown) {

‎src/helpers/clientSelectorGenerator.ts‎

Lines changed: 37 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2494,70 +2494,38 @@ class ClientSelectorGenerator {
24942494
};
24952495

24962496
private getAllDescendantsIncludingShadow(
2497-
parentElement: HTMLElement
2497+
parentElement: HTMLElement,
2498+
maxDepth: number = 20
24982499
): HTMLElement[] {
24992500
const allDescendants: HTMLElement[] = [];
25002501
const visited = new Set<HTMLElement>();
2501-
const shadowRootsSeen = new Set<ShadowRoot>();
2502-
2503-
const traverseShadowRoot = (shadowRoot: ShadowRoot, depth: number = 0) => {
2504-
if (depth > 10) return;
2505-
2506-
try {
2507-
const shadowElements = Array.from(
2508-
shadowRoot.querySelectorAll("*")
2509-
) as HTMLElement[];
2510-
2511-
shadowElements.forEach((shadowElement) => {
2512-
if (!visited.has(shadowElement)) {
2513-
visited.add(shadowElement);
2514-
allDescendants.push(shadowElement);
25152502

2516-
if (
2517-
shadowElement.shadowRoot &&
2518-
!shadowRootsSeen.has(shadowElement.shadowRoot)
2519-
) {
2520-
shadowRootsSeen.add(shadowElement.shadowRoot);
2521-
traverseShadowRoot(shadowElement.shadowRoot, depth + 1);
2522-
}
2523-
}
2524-
});
2503+
const traverse = (element: HTMLElement, currentDepth: number) => {
2504+
if (currentDepth >= maxDepth || visited.has(element)) {
2505+
return;
2506+
}
2507+
visited.add(element);
25252508

2526-
Array.from(shadowRoot.children).forEach((child) => {
2527-
const htmlChild = child as HTMLElement;
2528-
if (
2529-
htmlChild.shadowRoot &&
2530-
!shadowRootsSeen.has(htmlChild.shadowRoot)
2531-
) {
2532-
shadowRootsSeen.add(htmlChild.shadowRoot);
2533-
traverseShadowRoot(htmlChild.shadowRoot, depth + 1);
2534-
}
2535-
});
2536-
} catch (error) {
2537-
console.warn(`Error traversing shadow root:`, error);
2509+
if (element !== parentElement) {
2510+
allDescendants.push(element);
25382511
}
2539-
};
25402512

2541-
const regularDescendants = Array.from(
2542-
parentElement.querySelectorAll("*")
2543-
) as HTMLElement[];
2544-
regularDescendants.forEach((descendant) => {
2545-
if (!visited.has(descendant)) {
2546-
visited.add(descendant);
2547-
allDescendants.push(descendant);
2513+
// Traverse light DOM children
2514+
const children = Array.from(element.children) as HTMLElement[];
2515+
for (const child of children) {
2516+
traverse(child, currentDepth + 1);
25482517
}
2549-
});
25502518

2551-
const elementsWithShadow = [parentElement, ...regularDescendants].filter(
2552-
(el) => el.shadowRoot
2553-
);
2554-
elementsWithShadow.forEach((element) => {
2555-
if (!shadowRootsSeen.has(element.shadowRoot!)) {
2556-
shadowRootsSeen.add(element.shadowRoot!);
2557-
traverseShadowRoot(element.shadowRoot!, 0);
2519+
// Traverse shadow DOM if it exists
2520+
if (element.shadowRoot) {
2521+
const shadowChildren = Array.from(element.shadowRoot.children) as HTMLElement[];
2522+
for (const shadowChild of shadowChildren) {
2523+
traverse(shadowChild, currentDepth + 1);
2524+
}
25582525
}
2559-
});
2526+
};
25602527

2528+
traverse(parentElement, 0);
25612529
return allDescendants;
25622530
}
25632531

@@ -2577,6 +2545,8 @@ class ClientSelectorGenerator {
25772545
if (processedElements.has(descendant)) return;
25782546
processedElements.add(descendant);
25792547

2548+
if (!this.isMeaningfulElement(descendant)) return;
2549+
25802550
const absolutePath = this.buildOptimizedAbsoluteXPath(
25812551
descendant,
25822552
listSelector,
@@ -2766,16 +2736,20 @@ class ClientSelectorGenerator {
27662736
rootElement: HTMLElement,
27672737
otherListElements: HTMLElement[] = []
27682738
): string | null {
2769-
if (!this.elementContains(rootElement, targetElement) || targetElement === rootElement) {
2739+
if (
2740+
!this.elementContains(rootElement, targetElement) ||
2741+
targetElement === rootElement
2742+
) {
27702743
return null;
27712744
}
27722745

27732746
const pathParts: string[] = [];
27742747
let current: HTMLElement | null = targetElement;
2748+
let pathDepth = 0;
2749+
const MAX_PATH_DEPTH = 20;
27752750

27762751
// Build path from target up to root
2777-
while (current && current !== rootElement) {
2778-
// Calculate conflicts for each element in the path
2752+
while (current && current !== rootElement && pathDepth < MAX_PATH_DEPTH) {
27792753
const classes = this.getCommonClassesAcrossLists(
27802754
current,
27812755
otherListElements
@@ -2806,11 +2780,15 @@ class ClientSelectorGenerator {
28062780
pathParts.unshift(pathPart);
28072781
}
28082782

2809-
// Move to parent (either regular parent or shadow host)
2810-
current = current.parentElement ||
2783+
current =
2784+
current.parentElement ||
28112785
((current.getRootNode() as ShadowRoot).host as HTMLElement | null);
2786+
2787+
pathDepth++;
2788+
}
28122789

2813-
if (!current) break;
2790+
if (current !== rootElement) {
2791+
return null;
28142792
}
28152793

28162794
return pathParts.length > 0 ? "/" + pathParts.join("/") : null;

0 commit comments

Comments
 (0)