Waiting for both load and networkIdle

[Janpot]

Some pages will fire load event before network goes idle, some after. I'd like to wait for both events to have happened before considering a page 'navigated'. Currently looking at the watchdog code it seems like it's either one or the other. The way to do it right now is:

const timeout = 30000;
const networkIdleTimeout = 500;
await Promise.all([
  page.waitForNavigation({ timeout, waitUntil: 'load' }),
  page.waitForNavigation({ timeout, waitUntil: 'networkidle', networkIdleTimeout }),
  page.goto(url)
]);

But it seems to me that this is a functionality that deserves a more straightforward out-of-the-box solution. for instance:

page.goto(url, { 
  timeout: 3000,
  waitUntil: [ 'load', 'networkidle' ],
  networkIdleTimeout: 500
});

Or assume that people that wait for 'networkidle' also always want to wait for 'load'.

PS: I might be wrong about this, but after digging through the code, it seems to me that this function doesn't take the actual state into account. For instance it won't work if the load event has already fired or if the network is already idle.

[katp4]

Hello,

I can't help with your new feature request but based on what you want to establish, your code wont work. You state that you want to wait for the load and network idle event before you navigate to the page but Promise.all runs the promises in parallel and doesn't guarantee an order. So in your case, any one of your promises might run first.

I suggest you do a Promise.all that includes your 2 waitForNavigation promises and then, only when the both promises have resolved, go to url.

[Janpot]

I think you misinterpret Promise.all here. The promises are set up synchronously. The event handlers that are set up by waitForNavigation are all set up before the command in page.goto is sent.

the function in new Promise is a called synchronously here: https://github.com/GoogleChrome/puppeteer/blob/e5c17eecb93b995c78515e8b1b0df13c392efd8e/lib/NavigatorWatcher.js#L48
and the waitForNavigation is called synchronously here as well:
https://github.com/GoogleChrome/puppeteer/blob/e2cad568d610e00c8a4b6c348a51f161ed91c2d8/lib/Page.js#L415

It will become racy when they put an await before one of those function calls.

It's true that the promises can be resolved in any order. But the setup still happens in a defined order, in the current code.

[Janpot]

@katp4 Would also like to add that there is another problem with the pattern you propose. It lies in catching the errors;
consider

async function doSomething () {
  const promise = new Promise((resolve, reject) => setTimeout(reject, 500));
  await new Promise(resolve => setTimeout(resolve, 1000));
  await promise;
}

would result in an uncaught exception, even when:

doSomething().catch(e => console.log('caught', e));

[aslushnikov]

That's reasonable.

That's an opportunity to remove the waitUntil option: the API is a bit confusing.

I'd do:

await page.goto('https://example.com', {
  waitLoad: true, 
  waitNetworkIdle: true // defaults to false
});

[Janpot]

@aslushnikov and what about having the waitForNavigation work even after the page has navigated?
Then you'd need to keep track of the page's state and network activity all the time, even when those functions aren't called yet.

[aslushnikov]

and what about having the waitForNavigation work even after the page has navigated?

@Janpot that should work as well when we migrate onto lifecycle events. The plan is to keep lifecycle state for every frame.

[Janpot]

Ah great, I saw those new networkidle events, indeed. Is there a ticket for this already by any chance? Would love to follow up.

[aslushnikov]

@Janpot there's no ticket for lifecycle events, but I think you can follow the #728.

[aslushnikov]

and what about having the waitForNavigation work even after the page has navigated?

@Janpot lifecycle events turned out to be insufficient for this. We'll need to plumb the navigationIds/documentIds through the protocol as well.

I'll scope this bug to the initially suggested array of strings.