Skip to content

Commit c0f3b98

Browse files
fix: waitForDomNetworkQuiet() can hang forever (#1284)
# why - this PR fixes an issue where `waitForDomNetworkQuiet()` does not resolve - this issue is caused by 2-3 underlying issues: - the first issue is that `frame.waitForLoadState()` was not listening for the correct lifecycle events from CDP. instead of listening for `DOMContentLoaded`, it was listening for `domcontentloaded` - the second issue is that `frame.waitForLoadState()` did not accept a timeout parameter - the third issue is that `waitForDomNetworkQuiet()` did not include the call to `frame.waitForLoadState()` as part of the total time that it waits for # what changed - added string normalization of incoming lifecycle events from CDP in `frame.waitForLoadState()` (fixes the first issue) - added a timeout parameter to `frame.waitForLoadState()` - within `waitForDomNetworkQuiet()`, I included the call to `frame.waitForLoadState()` as part of the total time that it waits for # test plan - existing unit tests & regression evals should be ok
1 parent fa18cfd commit c0f3b98

File tree

3 files changed

+54
-8
lines changed

3 files changed

+54
-8
lines changed

.changeset/nice-socks-bow.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@browserbasehq/stagehand": patch
3+
---
4+
5+
fix: waitForDomNetworkQuiet() causing `act()` to hang indefinitely

packages/core/lib/v3/handlers/handlerUtils/actHandlerUtils.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -528,8 +528,12 @@ export async function waitForDomNetworkQuiet(
528528
frame: Frame,
529529
timeoutMs?: number,
530530
): Promise<void> {
531-
const timeout = typeof timeoutMs === "number" ? timeoutMs : 5_000;
531+
const overallTimeout =
532+
typeof timeoutMs === "number" && Number.isFinite(timeoutMs)
533+
? Math.max(0, timeoutMs)
534+
: 5_000;
532535
const client = frame.session;
536+
const settleStart = Date.now();
533537

534538
// Ensure a document exists; if not, wait for DOMContentLoaded on this frame.
535539
let hasDoc: boolean;
@@ -539,8 +543,16 @@ export async function waitForDomNetworkQuiet(
539543
} catch {
540544
hasDoc = false;
541545
}
542-
if (!hasDoc) {
543-
await frame.waitForLoadState("domcontentloaded").catch(() => {});
546+
if (!hasDoc && overallTimeout > 0) {
547+
await frame
548+
.waitForLoadState("domcontentloaded", overallTimeout)
549+
.catch(() => {});
550+
}
551+
552+
const elapsed = Date.now() - settleStart;
553+
const remainingBudget = Math.max(0, overallTimeout - elapsed);
554+
if (remainingBudget === 0) {
555+
return;
544556
}
545557

546558
await client.send("Network.enable").catch(() => {});
@@ -653,7 +665,7 @@ export async function waitForDomNetworkQuiet(
653665
});
654666
}
655667
resolveDone();
656-
}, timeout);
668+
}, remainingBudget);
657669

658670
const resolveDone = () => {
659671
client.off("Network.requestWillBeSent", onRequest);

packages/core/lib/v3/understudy/frame.ts

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,16 +243,45 @@ export class Frame implements FrameManager {
243243
/** Wait for a lifecycle state (load/domcontentloaded/networkidle) */
244244
async waitForLoadState(
245245
state: "load" | "domcontentloaded" | "networkidle" = "load",
246+
timeoutMs: number = 15_000,
246247
): Promise<void> {
247248
await this.session.send("Page.enable");
248-
await new Promise<void>((resolve) => {
249+
const targetState = state.toLowerCase();
250+
const timeout = Math.max(0, timeoutMs);
251+
await new Promise<void>((resolve, reject) => {
252+
let done = false;
253+
let timer: ReturnType<typeof setTimeout> | null = null;
254+
const finish = () => {
255+
if (done) return;
256+
done = true;
257+
this.session.off("Page.lifecycleEvent", handler);
258+
if (timer) {
259+
clearTimeout(timer);
260+
timer = null;
261+
}
262+
resolve();
263+
};
249264
const handler = (evt: Protocol.Page.LifecycleEventEvent) => {
250-
if (evt.frameId === this.frameId && evt.name === state) {
251-
this.session.off("Page.lifecycleEvent", handler);
252-
resolve();
265+
const sameFrame = evt.frameId === this.frameId;
266+
// need to normalize here because CDP lifecycle names look like 'DOMContentLoaded'
267+
// but we accept 'domcontentloaded'
268+
const lifecycleName = String(evt.name ?? "").toLowerCase();
269+
if (sameFrame && lifecycleName === targetState) {
270+
finish();
253271
}
254272
};
255273
this.session.on("Page.lifecycleEvent", handler);
274+
275+
timer = setTimeout(() => {
276+
if (done) return;
277+
done = true;
278+
this.session.off("Page.lifecycleEvent", handler);
279+
reject(
280+
new Error(
281+
`waitForLoadState(${state}) timed out after ${timeout}ms for frame ${this.frameId}`,
282+
),
283+
);
284+
}, timeout);
256285
});
257286
}
258287

0 commit comments

Comments
 (0)