From db1b35bcb6e8003ba1eb51ba4033402f333476a6 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sat, 9 Nov 2019 12:19:32 +0000 Subject: [PATCH 1/7] feat(api): implement `Page.waitForNetworkIdle()` which will wait for there to be no network requests in progress during the `idleTime` before resolving. --- docs/api.md | 12 +++++++++ lib/NetworkManager.js | 7 ++++++ lib/Page.js | 58 +++++++++++++++++++++++++++++++++++++++++++ test/page.spec.js | 57 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 134 insertions(+) diff --git a/docs/api.md b/docs/api.md index 8576314fe8629..e13a6f22937b4 100644 --- a/docs/api.md +++ b/docs/api.md @@ -158,6 +158,7 @@ * [page.waitForFileChooser([options])](#pagewaitforfilechooseroptions) * [page.waitForFunction(pageFunction[, options[, ...args]])](#pagewaitforfunctionpagefunction-options-args) * [page.waitForNavigation([options])](#pagewaitfornavigationoptions) + * [page.waitForNetworkIdle([options])](#pagewaitfornetworkidleoptions) * [page.waitForRequest(urlOrPredicate[, options])](#pagewaitforrequesturlorpredicate-options) * [page.waitForResponse(urlOrPredicate[, options])](#pagewaitforresponseurlorpredicate-options) * [page.waitForSelector(selector[, options])](#pagewaitforselectorselector-options) @@ -2089,6 +2090,17 @@ const [response] = await Promise.all([ Shortcut for [page.mainFrame().waitForNavigation(options)](#framewaitfornavigationoptions). +#### page.waitForNetworkIdle([options]) +- `options` <[Object]> Optional waiting parameters + - `timeout` <[number]> Maximum wait time in milliseconds, defaults to 30 seconds, pass `0` to disable the timeout. The default value can be changed by using the [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) method. + - `idleTime` <[number]> How long to wait for no network requests in milliseconds, defaults to 500 milliseconds. +- returns: <[Promise]> Promise which resolves when network is idle. + +```js +page.evaluate(() => fetch('some-url')); +page.waitForNetworkIdle(); // The promise resolves after fetch above finishes +``` + #### page.waitForRequest(urlOrPredicate[, options]) - `urlOrPredicate` <[string]|[Function]> A URL or predicate to wait for. - `options` <[Object]> Optional waiting parameters diff --git a/lib/NetworkManager.js b/lib/NetworkManager.js index 9f333009fcdd4..5ce219fe45153 100644 --- a/lib/NetworkManager.js +++ b/lib/NetworkManager.js @@ -89,6 +89,13 @@ class NetworkManager extends EventEmitter { return Object.assign({}, this._extraHTTPHeaders); } + /** + * @return {number} + */ + numRequestsInProgress() { + return this._requestIdToRequest.size; + } + /** * @param {boolean} value */ diff --git a/lib/Page.js b/lib/Page.js index 2c5d4796dd8cc..44d7d364e0bd6 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -737,6 +737,64 @@ class Page extends EventEmitter { }, timeout, this._sessionClosePromise()); } + /** + * @param {!{timeout?: number, idleTime?: number}=} options + * @return {!Promise} + */ + async waitForNetworkIdle(options = {}) { + const { + idleTime = 500, + timeout = this._timeoutSettings.timeout(), + } = options; + + const networkManager = this._frameManager.networkManager(); + + let idleResolveCallback; + const idlePromise = new Promise(resolve => { + idleResolveCallback = resolve; + }); + + let abortRejectCallback; + const abortPromise = new Promise((_, reject) => { + abortRejectCallback = reject; + }); + + let idleTimer; + const onIdle = () => idleResolveCallback(); + + const cleanup = () => { + idleTimer && clearTimeout(idleTimer); + abortRejectCallback(new Error('abort')); + }; + + const evaluate = () => { + idleTimer && clearTimeout(idleTimer); + if (networkManager.numRequestsInProgress() === 0) + idleTimer = setTimeout(onIdle, idleTime); + + }; + + evaluate(); + + const eventHandler = () => { + evaluate(); + return false; + }; + const listenToEvent = event => helper.waitForEvent(networkManager, event, eventHandler, timeout, abortPromise); + const eventPromises = []; + eventPromises.push(listenToEvent(Events.NetworkManager.Request)); + eventPromises.push(listenToEvent(Events.NetworkManager.RequestFinished)); + eventPromises.push(listenToEvent(Events.NetworkManager.RequestFailed)); + + await Promise.race([idlePromise, ...eventPromises, this._sessionClosePromise()]).then(r => { + cleanup(); + return r; + }, e => { + cleanup(); + throw e; + }); + } + /** * @param {!{timeout?: number, waitUntil?: string|!Array}=} options * @return {!Promise} diff --git a/test/page.spec.js b/test/page.spec.js index b7120c70e2e0f..55c9572c92938 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -571,6 +571,63 @@ module.exports.addTests = function({testRunner, expect, headless, puppeteer, CHR }); }); + describe('Page.waitForNetworkIdle', function() { + it('should work', async({page, server}) => { + await page.goto(server.EMPTY_PAGE); + let res; + const [t1, t2] = await Promise.all([ + page.waitForNetworkIdle().then(r => { + res = r; + return Date.now(); + }), + page.evaluate(() => (async() => { + // TODO why does this break with local url? + const url = 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; + await Promise.all([fetch(url, {mode: 'no-cors'}), fetch(url, {mode: 'no-cors'})]); + await new Promise(resolve => setTimeout(resolve, 200)); + await fetch(url, {mode: 'no-cors'}); + await new Promise(resolve => setTimeout(resolve, 400)); + await fetch(url, {mode: 'no-cors'}); + })()).then(() => Date.now()) + ]); + expect(res).toBe(undefined); + expect(t1).toBeGreaterThan(t2); + expect(t1 - t2).toBeGreaterThanOrEqual(400); + }); + it('should respect timeout', async({page, server}) => { + let error = null; + await page.waitForNetworkIdle({timeout: 1}).catch(e => error = e); + expect(error).toBeInstanceOf(puppeteer.errors.TimeoutError); + }); + it('should respect idleTime', async({page, server}) => { + await page.goto(server.EMPTY_PAGE); + const [t1, t2] = await Promise.all([ + page.waitForNetworkIdle({idleTime: 10}).then(() => Date.now()), + page.evaluate(() => (async() => { + // TODO why does this break with local url? + const url = 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; + await Promise.all([fetch(url, {mode: 'no-cors'}), fetch(url, {mode: 'no-cors'})]); + await new Promise(resolve => setTimeout(resolve, 250)); + })()).then(() => Date.now()) + ]); + expect(t2).toBeGreaterThan(t1); + }); + it('should work with no timeout', async({page, server}) => { + await page.goto(server.EMPTY_PAGE); + const [result] = await Promise.all([ + page.waitForNetworkIdle({timeout: 0}), + page.evaluate(() => setTimeout(() => { + // TODO why does this break with local url? + const url = 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; + fetch(url, {mode: 'no-cors'}); + fetch(url, {mode: 'no-cors'}); + fetch(url, {mode: 'no-cors'}); + }, 50)) + ]); + expect(result).toBe(undefined); + }); + }); + describe('Page.exposeFunction', function() { it('should work', async({page, server}) => { await page.exposeFunction('compute', function(a, b) { From 13e3da7552a4d368977416163782e860351e6fc8 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sat, 11 Sep 2021 12:46:34 +0100 Subject: [PATCH 2/7] add file missed in merge --- src/common/Page.ts | 55 ++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/src/common/Page.ts b/src/common/Page.ts index a031bddfa6e0e..1ee4f68116b2f 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -1898,21 +1898,21 @@ export class Page extends EventEmitter { * @param options - Optional waiting parameters * @returns Promise which resolves when network is idle */ - async waitForNetworkIdle(options: { idleTime?: number, timeout?: number } = {}) { - const { - idleTime = 500, - timeout = this._timeoutSettings.timeout(), - } = options; + async waitForNetworkIdle( + options: { idleTime?: number; timeout?: number } = {} + ) { + const { idleTime = 500, timeout = this._timeoutSettings.timeout() } = + options; const networkManager = this._frameManager.networkManager(); let idleResolveCallback; - const idlePromise = new Promise(resolve => { + const idlePromise = new Promise((resolve) => { idleResolveCallback = resolve; }); let abortRejectCallback; - const abortPromise = new Promise((_, reject) => { + const abortPromise = new Promise((_, reject) => { abortRejectCallback = reject; }); @@ -1928,7 +1928,6 @@ export class Page extends EventEmitter { idleTimer && clearTimeout(idleTimer); if (networkManager.numRequestsInProgress() === 0) idleTimer = setTimeout(onIdle, idleTime); - }; evaluate(); @@ -1937,19 +1936,37 @@ export class Page extends EventEmitter { evaluate(); return false; }; - const listenToEvent = event => helper.waitForEvent(networkManager, event, eventHandler, timeout, abortPromise); + const listenToEvent = (event) => + helper.waitForEvent( + networkManager, + event, + eventHandler, + timeout, + abortPromise + ); const eventPromises = []; eventPromises.push(listenToEvent(NetworkManagerEmittedEvents.Request)); - eventPromises.push(listenToEvent(NetworkManagerEmittedEvents.RequestFinished)); - eventPromises.push(listenToEvent(NetworkManagerEmittedEvents.RequestFailed)); - - await Promise.race([idlePromise, ...eventPromises, this._sessionClosePromise()]).then(r => { - cleanup(); - return r; - }, e => { - cleanup(); - throw e; - }); + eventPromises.push( + listenToEvent(NetworkManagerEmittedEvents.RequestFinished) + ); + eventPromises.push( + listenToEvent(NetworkManagerEmittedEvents.RequestFailed) + ); + + await Promise.race([ + idlePromise, + ...eventPromises, + this._sessionClosePromise(), + ]).then( + (r) => { + cleanup(); + return r; + }, + (e) => { + cleanup(); + throw e; + } + ); } /** From 783bea3f50ae3420a39739f7bbf9535040798ac1 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sat, 11 Sep 2021 12:48:52 +0100 Subject: [PATCH 3/7] chore: lint --- src/common/NetworkManager.ts | 2 +- src/common/Page.ts | 6 +-- test/page.spec.ts | 85 ++++++++++++++++++++++-------------- 3 files changed, 57 insertions(+), 36 deletions(-) diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index 1710afb00becb..aac208355e07d 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -188,7 +188,7 @@ export class NetworkManager extends EventEmitter { return Object.assign({}, this._extraHTTPHeaders); } - numRequestsInProgress() { + numRequestsInProgress(): number { return this._requestIdToRequest.size; } diff --git a/src/common/Page.ts b/src/common/Page.ts index 1ee4f68116b2f..7c87a40e2e0a8 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -1900,7 +1900,7 @@ export class Page extends EventEmitter { */ async waitForNetworkIdle( options: { idleTime?: number; timeout?: number } = {} - ) { + ): Promise { const { idleTime = 500, timeout = this._timeoutSettings.timeout() } = options; @@ -1962,9 +1962,9 @@ export class Page extends EventEmitter { cleanup(); return r; }, - (e) => { + (error) => { cleanup(); - throw e; + throw error; } ); } diff --git a/test/page.spec.ts b/test/page.spec.ts index 1880fb7979a58..695c9fdcc00d9 100644 --- a/test/page.spec.ts +++ b/test/page.spec.ts @@ -825,62 +825,83 @@ describe('Page', function () { }); }); - describe('Page.waitForNetworkIdle', function() { - it('should work', async() => { - const { page, puppeteer, server } = getTestState(); + describe('Page.waitForNetworkIdle', function () { + it('should work', async () => { + const { page, server } = getTestState(); await page.goto(server.EMPTY_PAGE); let res; const [t1, t2] = await Promise.all([ - page.waitForNetworkIdle().then(r => { + page.waitForNetworkIdle().then((r) => { res = r; return Date.now(); }), - page.evaluate(() => (async() => { - // TODO why does this break with local url? - const url = 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; - await Promise.all([fetch(url, {mode: 'no-cors'}), fetch(url, {mode: 'no-cors'})]); - await new Promise(resolve => setTimeout(resolve, 200)); - await fetch(url, {mode: 'no-cors'}); - await new Promise(resolve => setTimeout(resolve, 400)); - await fetch(url, {mode: 'no-cors'}); - })()).then(() => Date.now()) + page + .evaluate(() => + (async () => { + // TODO why does this break with local url? + const url = + 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; + await Promise.all([ + fetch(url, { mode: 'no-cors' }), + fetch(url, { mode: 'no-cors' }), + ]); + await new Promise((resolve) => setTimeout(resolve, 200)); + await fetch(url, { mode: 'no-cors' }); + await new Promise((resolve) => setTimeout(resolve, 400)); + await fetch(url, { mode: 'no-cors' }); + })() + ) + .then(() => Date.now()), ]); expect(res).toBe(undefined); expect(t1).toBeGreaterThan(t2); expect(t1 - t2).toBeGreaterThanOrEqual(400); }); - it('should respect timeout', async() => { + it('should respect timeout', async () => { const { page, puppeteer } = getTestState(); let error = null; - await page.waitForNetworkIdle({timeout: 1}).catch(e => error = e); + await page + .waitForNetworkIdle({ timeout: 1 }) + .catch((error_) => (error = error_)); expect(error).toBeInstanceOf(puppeteer.errors.TimeoutError); }); - it('should respect idleTime', async() => { + it('should respect idleTime', async () => { const { page, server } = getTestState(); await page.goto(server.EMPTY_PAGE); const [t1, t2] = await Promise.all([ - page.waitForNetworkIdle({idleTime: 10}).then(() => Date.now()), - page.evaluate(() => (async() => { - // TODO why does this break with local url? - const url = 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; - await Promise.all([fetch(url, {mode: 'no-cors'}), fetch(url, {mode: 'no-cors'})]); - await new Promise(resolve => setTimeout(resolve, 250)); - })()).then(() => Date.now()) + page.waitForNetworkIdle({ idleTime: 10 }).then(() => Date.now()), + page + .evaluate(() => + (async () => { + // TODO why does this break with local url? + const url = + 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; + await Promise.all([ + fetch(url, { mode: 'no-cors' }), + fetch(url, { mode: 'no-cors' }), + ]); + await new Promise((resolve) => setTimeout(resolve, 250)); + })() + ) + .then(() => Date.now()), ]); expect(t2).toBeGreaterThan(t1); }); - it('should work with no timeout', async() => { + it('should work with no timeout', async () => { const { page, server } = getTestState(); await page.goto(server.EMPTY_PAGE); const [result] = await Promise.all([ - page.waitForNetworkIdle({timeout: 0}), - page.evaluate(() => setTimeout(() => { - // TODO why does this break with local url? - const url = 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; - fetch(url, {mode: 'no-cors'}); - fetch(url, {mode: 'no-cors'}); - fetch(url, {mode: 'no-cors'}); - }, 50)) + page.waitForNetworkIdle({ timeout: 0 }), + page.evaluate(() => + setTimeout(() => { + // TODO why does this break with local url? + const url = + 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; + fetch(url, { mode: 'no-cors' }); + fetch(url, { mode: 'no-cors' }); + fetch(url, { mode: 'no-cors' }); + }, 50) + ), ]); expect(result).toBe(undefined); }); From 0257868048377fdb72d16905d200fb5991f64497 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sat, 11 Sep 2021 13:00:20 +0100 Subject: [PATCH 4/7] chore: switch back to local urls --- test/page.spec.ts | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/test/page.spec.ts b/test/page.spec.ts index 695c9fdcc00d9..5c9ef3a09af1d 100644 --- a/test/page.spec.ts +++ b/test/page.spec.ts @@ -838,17 +838,14 @@ describe('Page', function () { page .evaluate(() => (async () => { - // TODO why does this break with local url? - const url = - 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; await Promise.all([ - fetch(url, { mode: 'no-cors' }), - fetch(url, { mode: 'no-cors' }), + fetch('/digits/1.png'), + fetch('/digits/2.png'), ]); await new Promise((resolve) => setTimeout(resolve, 200)); - await fetch(url, { mode: 'no-cors' }); + await fetch('/digits/3.png'); await new Promise((resolve) => setTimeout(resolve, 400)); - await fetch(url, { mode: 'no-cors' }); + await fetch('/digits/4.png'); })() ) .then(() => Date.now()), @@ -873,12 +870,9 @@ describe('Page', function () { page .evaluate(() => (async () => { - // TODO why does this break with local url? - const url = - 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; await Promise.all([ - fetch(url, { mode: 'no-cors' }), - fetch(url, { mode: 'no-cors' }), + fetch('/digits/1.png'), + fetch('/digits/2.png'), ]); await new Promise((resolve) => setTimeout(resolve, 250)); })() @@ -894,12 +888,9 @@ describe('Page', function () { page.waitForNetworkIdle({ timeout: 0 }), page.evaluate(() => setTimeout(() => { - // TODO why does this break with local url? - const url = - 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png'; - fetch(url, { mode: 'no-cors' }); - fetch(url, { mode: 'no-cors' }); - fetch(url, { mode: 'no-cors' }); + fetch('/digits/1.png'); + fetch('/digits/2.png'); + fetch('/digits/3.png'); }, 50) ), ]); From 8ae25eb482e21bc65b70dd8dd3c8111d682105ae Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sat, 11 Sep 2021 13:26:52 +0100 Subject: [PATCH 5/7] chore: handle requests not being removed from `_requestIdToRequest` when cached --- src/common/NetworkManager.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index aac208355e07d..73ae84a9d512e 100644 --- a/src/common/NetworkManager.ts +++ b/src/common/NetworkManager.ts @@ -189,7 +189,9 @@ export class NetworkManager extends EventEmitter { } numRequestsInProgress(): number { - return this._requestIdToRequest.size; + return [...this._requestIdToRequest].filter(([, request]) => { + return !request.response(); + }).length; } async setOfflineMode(value: boolean): Promise { From 706cb9ca47be1ffb147ec925996d29b82a0e7155 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sat, 11 Sep 2021 13:27:03 +0100 Subject: [PATCH 6/7] chore: just use Request/Response events --- src/common/Page.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/common/Page.ts b/src/common/Page.ts index 7c87a40e2e0a8..834e0769f0798 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -1936,6 +1936,7 @@ export class Page extends EventEmitter { evaluate(); return false; }; + const listenToEvent = (event) => helper.waitForEvent( networkManager, @@ -1944,14 +1945,10 @@ export class Page extends EventEmitter { timeout, abortPromise ); + const eventPromises = []; eventPromises.push(listenToEvent(NetworkManagerEmittedEvents.Request)); - eventPromises.push( - listenToEvent(NetworkManagerEmittedEvents.RequestFinished) - ); - eventPromises.push( - listenToEvent(NetworkManagerEmittedEvents.RequestFailed) - ); + eventPromises.push(listenToEvent(NetworkManagerEmittedEvents.Response)); await Promise.race([ idlePromise, From 591aec73373d9361423cb47c15ddba5a5e7a1ca3 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sat, 11 Sep 2021 13:35:00 +0100 Subject: [PATCH 7/7] chore: small refactor --- src/common/Page.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/common/Page.ts b/src/common/Page.ts index 834e0769f0798..b2aa354d9eb30 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -1946,9 +1946,10 @@ export class Page extends EventEmitter { abortPromise ); - const eventPromises = []; - eventPromises.push(listenToEvent(NetworkManagerEmittedEvents.Request)); - eventPromises.push(listenToEvent(NetworkManagerEmittedEvents.Response)); + const eventPromises = [ + listenToEvent(NetworkManagerEmittedEvents.Request), + listenToEvent(NetworkManagerEmittedEvents.Response), + ]; await Promise.race([ idlePromise,