From ce8d721bf70ca928eee630f69ec296f0be122551 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 7 Feb 2019 14:39:09 -0800 Subject: [PATCH 1/4] core: do not cause protocol timeout for app manifest bug in LR --- lighthouse-core/gather/gather-runner.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index 86ae7a8f4be9..7eb5014a0eb5 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -423,9 +423,20 @@ class GatherRunner { * @return {Promise} */ static async getWebAppManifest(passContext) { - const response = await passContext.driver.getAppManifest(); - if (!response) return null; - return manifestParser(response.data, response.url, passContext.url); + try { + const response = await passContext.driver.getAppManifest(); + if (response) return manifestParser(response.data, response.url, passContext.url); + } catch (err) { + // LR will timeout fetching the app manifest in some cases + // for example, long polling (http://uninterested-badge.surge.sh/index4.html) + // or web workers (rarely https://exterkamp.codes/) + // see #7147 or b/124008171 + // instead of failing completely by throwing a protocol timeout error, + // just pretend there is no app manifest. + log.error('Failed fetching manifest', err); + } + + return null; } /** From 47b2a279bdecb6eb3d49450304ab2df095476407 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 7 Feb 2019 15:18:23 -0800 Subject: [PATCH 2/4] pr changes --- lighthouse-core/gather/driver.js | 11 ++++++++++- lighthouse-core/gather/gather-runner.js | 17 +++-------------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index b2e26267fe6c..2a187f807973 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -400,7 +400,16 @@ class Driver { */ async getAppManifest() { this.setNextProtocolTimeout(3000); - const response = await this.sendCommand('Page.getAppManifest'); + let response; + try { + response = await this.sendCommand('Page.getAppManifest'); + } catch (err) { + // LR will timeout fetching the app manifest in some cases, move on without one. + // https://github.com/GoogleChrome/lighthouse/issues/7147#issuecomment-461210921 + log.error('Driver', 'Failed fetching...', err); + return null; + } + let data = response.data; // We're not reading `response.errors` however it may contain critical and noncritical diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index 7eb5014a0eb5..86ae7a8f4be9 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -423,20 +423,9 @@ class GatherRunner { * @return {Promise} */ static async getWebAppManifest(passContext) { - try { - const response = await passContext.driver.getAppManifest(); - if (response) return manifestParser(response.data, response.url, passContext.url); - } catch (err) { - // LR will timeout fetching the app manifest in some cases - // for example, long polling (http://uninterested-badge.surge.sh/index4.html) - // or web workers (rarely https://exterkamp.codes/) - // see #7147 or b/124008171 - // instead of failing completely by throwing a protocol timeout error, - // just pretend there is no app manifest. - log.error('Failed fetching manifest', err); - } - - return null; + const response = await passContext.driver.getAppManifest(); + if (!response) return null; + return manifestParser(response.data, response.url, passContext.url); } /** From f703b06675dc4a7c1125e2caca61c411f50be3b4 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 7 Feb 2019 15:22:21 -0800 Subject: [PATCH 3/4] only if protocol timeout --- lighthouse-core/gather/driver.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 2a187f807973..e01f4cdabe2f 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -404,10 +404,14 @@ class Driver { try { response = await this.sendCommand('Page.getAppManifest'); } catch (err) { - // LR will timeout fetching the app manifest in some cases, move on without one. - // https://github.com/GoogleChrome/lighthouse/issues/7147#issuecomment-461210921 - log.error('Driver', 'Failed fetching...', err); - return null; + if (err.code === 'PROTOCOL_TIMEOUT') { + // LR will timeout fetching the app manifest in some cases, move on without one. + // https://github.com/GoogleChrome/lighthouse/issues/7147#issuecomment-461210921 + log.error('Driver', 'Failed fetching...', err); + return null; + } + + throw err; } let data = response.data; From f2d1c85aa8b0f2652f9062007bcc907ea54d6074 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 7 Feb 2019 15:39:42 -0800 Subject: [PATCH 4/4] ...manifest! --- lighthouse-core/gather/driver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index e01f4cdabe2f..299a8c78e211 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -407,7 +407,7 @@ class Driver { if (err.code === 'PROTOCOL_TIMEOUT') { // LR will timeout fetching the app manifest in some cases, move on without one. // https://github.com/GoogleChrome/lighthouse/issues/7147#issuecomment-461210921 - log.error('Driver', 'Failed fetching...', err); + log.error('Driver', 'Failed fetching manifest', err); return null; }