Skip to content

feat: faster remote browser loading #860

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 5 commits into from
Nov 12, 2025
Merged

feat: faster remote browser loading #860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1f99295
feat: rm canvas logic, add in-browser loader
RohitR311 Nov 3, 2025
58aa5f5
feat: speed up snapshot emission
RohitR311 Nov 3, 2025
d7da903
fix: add recording setup route
RohitR311 Nov 4, 2025
cd53994
Merge branch 'develop' into fast-loader
RohitR311 Nov 5, 2025
fcb3a12
Merge branch 'develop' into fast-loader
RohitR311 Nov 7, 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
78 changes: 62 additions & 16 deletions server/src/browser-management/classes/RemoteBrowser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,11 @@ export class RemoteBrowser {
private networkRequestTimeout: NodeJS.Timeout | null = null;
private pendingNetworkRequests: string[] = [];
private readonly NETWORK_QUIET_PERIOD = 8000;
private readonly INITIAL_LOAD_QUIET_PERIOD = 3000;
private networkWaitStartTime: number = 0;
private progressInterval: NodeJS.Timeout | null = null;
private hasShownInitialLoader: boolean = false;
private isInitialLoadInProgress: boolean = false;

/**
* Initializes a new instances of the {@link Generator} and {@link WorkflowInterpreter} classes and
Expand Down Expand Up @@ -432,42 +437,74 @@ export class RemoteBrowser {
if (!this.currentPage) return;

this.currentPage.on("domcontentloaded", async () => {
logger.info("DOM content loaded - triggering snapshot");
await this.makeAndEmitDOMSnapshot();
if (!this.isInitialLoadInProgress) {
logger.info("DOM content loaded - triggering snapshot");
await this.makeAndEmitDOMSnapshot();
}
});

this.currentPage.on("response", async (response) => {
const url = response.url();
if (
response.request().resourceType() === "document" ||
url.includes("api/") ||
url.includes("ajax")
) {
const isDocumentRequest = response.request().resourceType() === "document";

if (!this.hasShownInitialLoader && isDocumentRequest && !url.includes("about:blank")) {
Copy link

@coderabbitai coderabbitai bot Nov 3, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fragile URL check could match unintended substrings.

Using url.includes("about:blank") could incorrectly match legitimate URLs that contain "about:blank" as a substring (e.g., https://example.com/page?redirect=about:blank).

Consider using a more precise check:

-        if (!this.hasShownInitialLoader && isDocumentRequest && !url.includes("about:blank")) {
+        if (!this.hasShownInitialLoader && isDocumentRequest && url !== "about:blank") {
📝 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
if (!this.hasShownInitialLoader && isDocumentRequest && !url.includes("about:blank")) {
if (!this.hasShownInitialLoader && isDocumentRequest && url !== "about:blank") {

this.hasShownInitialLoader = true;
this.isInitialLoadInProgress = true;
this.pendingNetworkRequests.push(url);

if (this.networkRequestTimeout) {
clearTimeout(this.networkRequestTimeout);
this.networkRequestTimeout = null;
}

if (this.progressInterval) {
clearInterval(this.progressInterval);
this.progressInterval = null;
}

this.networkWaitStartTime = Date.now();
this.progressInterval = setInterval(() => {
const elapsed = Date.now() - this.networkWaitStartTime;
const navigationProgress = Math.min((elapsed / this.INITIAL_LOAD_QUIET_PERIOD) * 40, 35);
const totalProgress = 60 + navigationProgress;
this.emitLoadingProgress(totalProgress, this.pendingNetworkRequests.length);
}, 500);
Comment on lines +466 to +471
Copy link

@coderabbitai coderabbitai bot Nov 3, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Memory leak: progressInterval not cleaned up in shutdown/cleanup methods.

The progressInterval is created here but only cleared within the setTimeout callback (lines 482-485). If switchOff(), stopDOM(), or other cleanup methods are called before the timeout fires, the interval will continue running indefinitely.

Add cleanup in the relevant methods:

// In switchOff() method around line 1738
if (this.progressInterval) {
  clearInterval(this.progressInterval);
  this.progressInterval = null;
}

// In stopDOM() method around line 1703
if (this.progressInterval) {
  clearInterval(this.progressInterval);
  this.progressInterval = null;
}
🤖 Prompt for AI Agents 
In server/src/browser-management/classes/RemoteBrowser.ts around lines 466 to
471, an interval (progressInterval) is started but only cleared inside a later
setTimeout callback, causing a potential memory leak if shutdown/cleanup methods
run first; add code in switchOff() (around line 1738) and stopDOM() (around line
1703) to check if this.progressInterval is set, call
clearInterval(this.progressInterval) and then set this.progressInterval to null
so the interval is always cleaned up during shutdown.


logger.debug(
`Network request received: ${url}. Total pending: ${this.pendingNetworkRequests.length}`
`Initial load network request received: ${url}. Using ${this.INITIAL_LOAD_QUIET_PERIOD}ms quiet period`
);

this.networkRequestTimeout = setTimeout(async () => {
logger.info(
`Network quiet period reached. Processing ${this.pendingNetworkRequests.length} requests`
`Initial load network quiet period reached (${this.INITIAL_LOAD_QUIET_PERIOD}ms)`
);

if (this.progressInterval) {
clearInterval(this.progressInterval);
this.progressInterval = null;
}

this.emitLoadingProgress(100, this.pendingNetworkRequests.length);

this.pendingNetworkRequests = [];
this.networkRequestTimeout = null;
this.isInitialLoadInProgress = false;

await this.makeAndEmitDOMSnapshot();
}, this.NETWORK_QUIET_PERIOD);
}, this.INITIAL_LOAD_QUIET_PERIOD);
}
});
}

private emitLoadingProgress(progress: number, pendingRequests: number): void {
this.socket.emit("domLoadingProgress", {
progress: Math.round(progress),
pendingRequests,
userId: this.userId,
timestamp: Date.now(),
});
}

private async setupPageEventListeners(page: Page) {
page.on('framenavigated', async (frame) => {
if (frame === page.mainFrame()) {
Expand Down Expand Up @@ -521,7 +558,13 @@ export class RemoteBrowser {
const MAX_RETRIES = 3;
let retryCount = 0;
let success = false;


this.socket.emit("dom-snapshot-loading", {
userId: this.userId,
timestamp: Date.now(),
});
this.emitLoadingProgress(0, 0);

while (!success && retryCount < MAX_RETRIES) {
try {
this.browser = <Browser>(await chromium.launch({
Expand All @@ -545,7 +588,9 @@ export class RemoteBrowser {
if (!this.browser || this.browser.isConnected() === false) {
throw new Error('Browser failed to launch or is not connected');
}


this.emitLoadingProgress(20, 0);

const proxyConfig = await getDecryptedProxyConfig(userId);
let proxyOptions: { server: string, username?: string, password?: string } = { server: '' };

Expand Down Expand Up @@ -623,6 +668,8 @@ export class RemoteBrowser {

this.currentPage = await this.context.newPage();

this.emitLoadingProgress(40, 0);

await this.setupPageEventListeners(this.currentPage);

const viewportSize = await this.currentPage.viewportSize();
Expand All @@ -645,7 +692,9 @@ export class RemoteBrowser {
// Still need to set up the CDP session even if blocker fails
this.client = await this.currentPage.context().newCDPSession(this.currentPage);
}


this.emitLoadingProgress(60, 0);

success = true;
logger.log('debug', `Browser initialized successfully for user ${userId}`);
} catch (error: any) {
Expand Down Expand Up @@ -1521,9 +1570,6 @@ export class RemoteBrowser {
this.isDOMStreamingActive = true;
logger.info("DOM streaming started successfully");

// Initial DOM snapshot
await this.makeAndEmitDOMSnapshot();

this.setupScrollEventListener();
this.setupPageChangeListeners();
} catch (error) {
Expand Down
4 changes: 2 additions & 2 deletions src/components/browser/BrowserContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
export const BrowserContent = () => {
const { socket } = useSocketStore();

const [tabs, setTabs] = useState<string[]>(["current"]);
const [tabs, setTabs] = useState<string[]>(["Loading..."]);
Copy link

@coderabbitai coderabbitai bot Nov 3, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Prevent phantom “Loading...” tab from breaking tab indices.

By keeping the placeholder when getCurrentTabs() returns an empty list you end up appending the first real tab after "Loading...". UI indices (1, 2, …) then no longer match the backend’s tab indices (0, 1, …), so actions like changeTab(1) or closeTab(1) target the wrong tab or fail outright. Please allow the empty array to overwrite the placeholder (or otherwise drop the placeholder before appending) so the frontend stays in sync with the backend.

Apply this diff to restore the previous behavior:

-        if (response && response.length > 0) {
-          setTabs(response);
-        }
+        if (Array.isArray(response)) {
+          setTabs(response);
+        }

Also applies to: 128-130

🤖 Prompt for AI Agents 
In src/components/browser/BrowserContent.tsx around lines 16 and 128-130, the
initial state uses a "Loading..." placeholder which is kept when
getCurrentTabs() returns an empty array, causing frontend tab indices to shift;
remove the placeholder by initializing tabs to an empty array ([]) and/or change
the logic that applies backend results so that when getCurrentTabs() returns []
it overwrites any placeholder instead of appending to it (i.e., replace the tabs
array wholesale with the backend array or drop the placeholder before
concatenation) so frontend indices stay aligned with backend indices.

const [tabIndex, setTabIndex] = React.useState(0);
const [showOutputData, setShowOutputData] = useState(false);
const { browserWidth } = useBrowserDimensionsStore();
Expand Down Expand Up @@ -125,7 +125,7 @@ export const BrowserContent = () => {
useEffect(() => {
getCurrentTabs()
.then((response) => {
if (response) {
if (response && response.length > 0) {
setTabs(response);
}
})
Expand Down
Loading