From 812245f369934028de7726d3e35a58e8391750ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20Mari=C8=99?= Date: Thu, 20 Sep 2018 20:32:08 -0700 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=9A=80=20utils-debugging-protocol-c?= =?UTF-8?q?ommon=20-=20v1.0.6=20[skip=20ci]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/utils-debugging-protocol-common/CHANGELOG.md | 7 +++++++ packages/utils-debugging-protocol-common/package.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/utils-debugging-protocol-common/CHANGELOG.md b/packages/utils-debugging-protocol-common/CHANGELOG.md index 0b51f2a2e7e..03d8d0f8764 100644 --- a/packages/utils-debugging-protocol-common/CHANGELOG.md +++ b/packages/utils-debugging-protocol-common/CHANGELOG.md @@ -1,3 +1,10 @@ +# 1.0.6 (September 20, 2018) + +## Bug fixes / Improvements + +* [[`ea56a95ce4`](https://github.com/webhintio/hint/commit/ea56a95ce452c136c872dadd9c790b2cc5f9cd06)] - Fix: Several issues with the Debugging Protocol (by [`Antón Molleda`](https://github.com/molant) / see also: [`#1325`](https://github.com/webhintio/hint/issues/1325)). + + # 1.0.5 (September 20, 2018) ## Bug fixes / Improvements diff --git a/packages/utils-debugging-protocol-common/package.json b/packages/utils-debugging-protocol-common/package.json index 415033de8d0..f2ebbceabfe 100644 --- a/packages/utils-debugging-protocol-common/package.json +++ b/packages/utils-debugging-protocol-common/package.json @@ -52,5 +52,5 @@ "watch": "npm run build && npm-run-all --parallel -c watch:*", "watch:ts": "npm run build:ts -- --watch" }, - "version": "1.0.5" + "version": "1.0.6" } From 5b928e6c5a14f97ac97ee29e14bbfa5b7bb1435b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20Mari=C8=99?= Date: Thu, 20 Sep 2018 20:32:38 -0700 Subject: [PATCH 02/11] Chore: Update `utils-debugging-protocol-common` to `v1.0.6` [skip ci] --- packages/connector-chrome/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/connector-chrome/package.json b/packages/connector-chrome/package.json index 6385326ce64..9ca5346b237 100644 --- a/packages/connector-chrome/package.json +++ b/packages/connector-chrome/package.json @@ -7,7 +7,7 @@ "timeout": "1m" }, "dependencies": { - "@hint/utils-debugging-protocol-common": "^1.0.5", + "@hint/utils-debugging-protocol-common": "^1.0.6", "chrome-launcher": "^0.10.4", "is-ci": "^1.2.1", "lockfile": "^1.0.4" From 790135a4049a00fdb6d42a0be6459214adac8a3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20Mari=C8=99?= Date: Thu, 20 Sep 2018 20:43:59 -0700 Subject: [PATCH 03/11] =?UTF-8?q?=F0=9F=9A=80=20parser-html=20-=20v1.0.2?= =?UTF-8?q?=20[skip=20ci]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/parser-html/CHANGELOG.md | 7 +++++++ packages/parser-html/package.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/parser-html/CHANGELOG.md b/packages/parser-html/CHANGELOG.md index 835542a34af..05581a0123f 100644 --- a/packages/parser-html/CHANGELOG.md +++ b/packages/parser-html/CHANGELOG.md @@ -1,3 +1,10 @@ +# 1.0.2 (September 20, 2018) + +## Bug fixes / Improvements + +* [[`537bbbbd98`](https://github.com/webhintio/hint/commit/537bbbbd98c2269d95ecda08e54aa4a086468183)] - Fix: Use JSDOM locations for elements if available (by [`Tony Ross`](https://github.com/antross)). + + # 1.0.1 (September 6, 2018) ## Bug fixes / Improvements diff --git a/packages/parser-html/package.json b/packages/parser-html/package.json index 1d3873bde82..a3457aef982 100644 --- a/packages/parser-html/package.json +++ b/packages/parser-html/package.json @@ -73,5 +73,5 @@ "watch:test": "ava --watch", "watch:ts": "npm run build:ts -- --watch" }, - "version": "1.0.1" + "version": "1.0.2" } From 3d84b7aafb69ba67f789d1b5a721cffb5f559176 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20Mari=C8=99?= Date: Thu, 20 Sep 2018 20:45:20 -0700 Subject: [PATCH 04/11] Chore: Update `parser-html` to `v1.0.2` [skip ci] --- packages/configuration-development/package.json | 2 +- packages/configuration-web-recommended/package.json | 2 +- packages/connector-local/package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/configuration-development/package.json b/packages/configuration-development/package.json index fd785934b38..8dfc60a7ebb 100644 --- a/packages/configuration-development/package.json +++ b/packages/configuration-development/package.json @@ -15,7 +15,7 @@ "@hint/hint-typescript-config": "^1.1.0", "@hint/hint-webpack-config": "^1.0.0", "@hint/parser-babel-config": "^1.1.0", - "@hint/parser-html": "^1.0.1", + "@hint/parser-html": "^1.0.2", "@hint/parser-typescript-config": "^1.1.0", "@hint/parser-webpack-config": "^1.0.0" }, diff --git a/packages/configuration-web-recommended/package.json b/packages/configuration-web-recommended/package.json index 1bdfb6ea400..aa04863fef2 100644 --- a/packages/configuration-web-recommended/package.json +++ b/packages/configuration-web-recommended/package.json @@ -29,7 +29,7 @@ "@hint/hint-stylesheet-limits": "^1.0.1", "@hint/hint-validate-set-cookie-header": "^1.0.2", "@hint/hint-x-content-type-options": "^1.0.3", - "@hint/parser-html": "^1.0.1" + "@hint/parser-html": "^1.0.2" }, "description": "webhint's recommended hints configuration for live websites", "engines": { diff --git a/packages/connector-local/package.json b/packages/connector-local/package.json index 8abe8a59c8c..e3e9fa3329d 100644 --- a/packages/connector-local/package.json +++ b/packages/connector-local/package.json @@ -12,7 +12,7 @@ }, "description": "hint local connector", "devDependencies": { - "@hint/parser-html": "^1.0.1", + "@hint/parser-html": "^1.0.2", "@types/chokidar": "^1.7.5", "@types/mock-require": "^2.0.0", "ava": "^0.25.0", From 4d7706a4db34fb05afaf2e25b9b3e1cc4990b604 Mon Sep 17 00:00:00 2001 From: Tony Ross Date: Fri, 21 Sep 2018 10:39:54 -0500 Subject: [PATCH 05/11] Fix: Disable hints that are currently noisy when run locally Disable `hint-apple-touch-icons` because it is not aware if the project is a PWA or not (needs to check for a manifest). Disable `webpack-config/module-esnext-typescript` because it requires loading both the webpack and typescript configs at the same time to work (which doesn't currently happen in VS Code). These can be re-enabled once the affected hints are updated. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Close #1329 --- packages/configuration-development/index.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/configuration-development/index.json b/packages/configuration-development/index.json index 01a7eb65170..9571ff347e1 100644 --- a/packages/configuration-development/index.json +++ b/packages/configuration-development/index.json @@ -8,6 +8,7 @@ "summary" ], "hints": { + "apple-touch-icons": "off", "axe": "error", "babel-config/is-valid": "error", "disown-opener": "error", @@ -25,7 +26,7 @@ "typescript-config/target": "error", "webpack-config/is-installed": "error", "webpack-config/is-valid": "error", - "webpack-config/module-esnext-typescript": "error", + "webpack-config/module-esnext-typescript": "off", "webpack-config/modules/false-babel": "error", "webpack-config/no-devtool-in-prod": "error" }, From 870e599d49bbd663c37d9e217c1dc10be19080e6 Mon Sep 17 00:00:00 2001 From: molant Date: Fri, 21 Sep 2018 09:49:58 -0700 Subject: [PATCH 06/11] Fix: Convert to text base64 text based responses The body of some responses returns as base64 by the Debugging Protocol even though they are text based. This happens a lot with the JavaScript of ads and causes the JavaScript parser to fail. --- packages/utils-debugging-protocol-common/package.json | 1 + .../src/debugging-protocol-connector.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/utils-debugging-protocol-common/package.json b/packages/utils-debugging-protocol-common/package.json index f2ebbceabfe..935e54f4dd4 100644 --- a/packages/utils-debugging-protocol-common/package.json +++ b/packages/utils-debugging-protocol-common/package.json @@ -1,6 +1,7 @@ { "dependencies": { "@hint/utils-connector-tools": "^1.0.5", + "abab": "^2.0.0", "chrome-remote-interface": "^0.26.1", "lodash": "^4.17.11" }, diff --git a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts index 9bf1bb23f8e..1319a3656b3 100644 --- a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts +++ b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts @@ -16,6 +16,7 @@ import { promisify } from 'util'; import * as cdp from 'chrome-remote-interface'; import { compact, filter } from 'lodash'; +import { atob } from 'abab'; import { CDPAsyncHTMLDocument, AsyncHTMLElement } from './cdp-async-html'; import { getContentTypeData, getType } from 'hint/dist/src/lib/utils/content-type'; @@ -321,7 +322,7 @@ export class Connector implements IConnector { const { body, base64Encoded } = await this._client.Network.getResponseBody({ requestId: cdpResponse.requestId }); const encoding = base64Encoded ? 'base64' : 'utf8'; - content = body; + content = base64Encoded ? atob(body) : body; // There are some JS responses that are base64 encoded for some reason rawContent = Buffer.from(body, encoding); const returnValue = { From b80fbfb64cc6075f65afc642fff37a733db167ab Mon Sep 17 00:00:00 2001 From: molant Date: Fri, 21 Sep 2018 13:12:03 -0700 Subject: [PATCH 07/11] Fix: Location issues with `jsdom` Depending on the package manager used for the installation, different versions of `parse5` with different APIs could be installed. Until #1274 is merged, this solves for that case. --- packages/hint/src/lib/types/jsdom-async-html.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/hint/src/lib/types/jsdom-async-html.ts b/packages/hint/src/lib/types/jsdom-async-html.ts index 1da09e078a4..ec76cab5429 100644 --- a/packages/hint/src/lib/types/jsdom-async-html.ts +++ b/packages/hint/src/lib/types/jsdom-async-html.ts @@ -63,11 +63,16 @@ export class JSDOMAsyncHTMLElement implements IAsyncHTMLElement { /* istanbul ignore next */ public getLocation(): ProblemLocation { try { - const location = this._dom && this._dom.nodeLocation(this._htmlelement); + /* + * TODO: Depending on the install (yarn vs npm) we get a different version of `jsdom` + * that has a different version of `parse5`. This takes care of this issue but should + * be fixed once we migrate completely to jsdom 12 (https://github.com/webhintio/hint/pull/1274) + */ + const location: any = this._dom && this._dom.nodeLocation(this._htmlelement); return location && { - column: location.startTag.col, - line: location.startTag.line - 1 + column: location.startTag.col || location.startTag.startCol, + line: (location.startTag.line || location.startTag.startLine) - 1 } || null; } catch (e) { // JSDOM throws an exception if `includeNodeLocations` wasn't set. From 32f6449694cf378d6cee174319c3c1da99502e2f Mon Sep 17 00:00:00 2001 From: molant Date: Fri, 21 Sep 2018 13:49:01 -0700 Subject: [PATCH 08/11] Chore: Add types to debugging protocol --- .../package.json | 1 + .../src/cdp-async-html.ts | 10 +++--- .../src/debugging-protocol-connector.ts | 35 ++++++++++--------- yarn.lock | 4 +++ 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/packages/utils-debugging-protocol-common/package.json b/packages/utils-debugging-protocol-common/package.json index 935e54f4dd4..0b46bb786f4 100644 --- a/packages/utils-debugging-protocol-common/package.json +++ b/packages/utils-debugging-protocol-common/package.json @@ -8,6 +8,7 @@ "description": "hint debugging protocol common functionality", "devDependencies": { "@types/lodash": "^4.14.115", + "chrome-remote-debug-protocol": "^1.2.20180422", "eslint": "^5.6.0", "eslint-plugin-import": "^2.14.0", "eslint-plugin-markdown": "^1.0.0-beta.7", diff --git a/packages/utils-debugging-protocol-common/src/cdp-async-html.ts b/packages/utils-debugging-protocol-common/src/cdp-async-html.ts index eacca395515..bb35a9767b1 100644 --- a/packages/utils-debugging-protocol-common/src/cdp-async-html.ts +++ b/packages/utils-debugging-protocol-common/src/cdp-async-html.ts @@ -1,3 +1,5 @@ +import { Crdp } from 'chrome-remote-debug-protocol'; + import { IAsyncHTMLDocument, IAsyncHTMLElement } from 'hint/dist/src/lib/types/async-html'; //eslint-disable-line import { debug as d } from 'hint/dist/src/lib/utils/debug'; import { ProblemLocation } from 'hint/dist/src/lib/types'; @@ -7,7 +9,7 @@ const debug: debug.IDebugger = d(__filename); /** An implementation of AsyncHTMLDocument on top of the Chrome Debugging Protocol */ export class CDPAsyncHTMLDocument implements IAsyncHTMLDocument { /** The DOM domain of the CDP client. */ - private _DOM; + private _DOM: Crdp.DOMClient; /** The root element of the real DOM. */ private _dom; /** A map with all the nodes accessible using `nodeId`. */ @@ -25,7 +27,7 @@ export class CDPAsyncHTMLDocument implements IAsyncHTMLDocument { * initially, we store them in a Map using the `nodeId` as the key so we can access to them * later. */ - private trackNodes(root) { + private trackNodes(root: Crdp.DOM.Node) { this._nodes.set(root.nodeId, root); if (!root.children) { return; @@ -36,7 +38,7 @@ export class CDPAsyncHTMLDocument implements IAsyncHTMLDocument { }); } - private getHTMLChildren(children: Array) { + private getHTMLChildren(children: Array) { return children.find((item) => { return item.nodeType === 1 && item.nodeName === 'HTML'; }); @@ -128,7 +130,7 @@ export class AsyncHTMLElement implements IAsyncHTMLElement { private _attributesArray: Array<{ name: string; value: string; }> = []; private _attributesMap: Map = new Map(); - public constructor(htmlelement, ownerDocument, DOM) { + public constructor(htmlelement: Crdp.DOM.Node, ownerDocument: CDPAsyncHTMLDocument, DOM: Crdp.DOMClient) { if (typeof htmlelement === 'number') { throw new Error(); } diff --git a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts index 1319a3656b3..a538a868186 100644 --- a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts +++ b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts @@ -17,6 +17,7 @@ import { promisify } from 'util'; import * as cdp from 'chrome-remote-interface'; import { compact, filter } from 'lodash'; import { atob } from 'abab'; +import { Crdp } from 'chrome-remote-debug-protocol'; import { CDPAsyncHTMLDocument, AsyncHTMLElement } from './cdp-async-html'; import { getContentTypeData, getType } from 'hint/dist/src/lib/utils/content-type'; @@ -53,7 +54,7 @@ export class Connector implements IConnector { /** The instance of hint that is using this connector. */ private _server: Engine; /** The client to talk to the browser. */ - private _client; + private _client: Crdp.CrdpClient; /** A set of requests done by the connector to retrieve initial information more easily. */ private _requests: Map; /** Indicates if there has been an error loading the page (e.g.: it doesn't exists). */ @@ -192,7 +193,7 @@ export class Connector implements IConnector { } /** Event handler for when the browser is about to make a request. */ - private async onRequestWillBeSent(params) { + private async onRequestWillBeSent(params: Crdp.Network.RequestWillBeSentEvent) { const requestUrl: string = params.request.url; debug(`About to start fetching ${cutString(requestUrl)} (${params.requestId})`); @@ -246,7 +247,7 @@ export class Connector implements IConnector { } /** Event handler fired when HTTP request fails for some reason. */ - private async onLoadingFailed(params) { + private async onLoadingFailed(params: Crdp.Network.LoadingFailedEvent) { const requestInfo = this._requests.get(params.requestId); /* @@ -303,7 +304,7 @@ export class Connector implements IConnector { } } - private async getResponseBody(cdpResponse): Promise<{ content: string, rawContent: Buffer, rawResponse(): Promise }> { + private async getResponseBody(cdpResponse: Crdp.Network.ResponseReceivedEvent): Promise<{ content: string, rawContent: Buffer, rawResponse(): Promise }> { let content: string = ''; let rawContent: Buffer = null; const rawResponse = (): Promise => { @@ -329,7 +330,7 @@ export class Connector implements IConnector { content, rawContent, rawResponse(): Promise { - const self = (this as any); + const self = (this as { _rawResponse: Promise }); if (self) { const cached = self._rawResponse; @@ -408,7 +409,7 @@ export class Connector implements IConnector { } /** Returns a Response for the given request. */ - private async createResponse(cdpResponse, element: IAsyncHTMLElement): Promise { + private async createResponse(cdpResponse: Crdp.Network.ResponseReceivedEvent, element: IAsyncHTMLElement): Promise { const resourceUrl: string = cdpResponse.response.url; const hops: Array = this._redirects.calculate(resourceUrl); const resourceHeaders: object = normalizeHeaders(cdpResponse.response.headers); @@ -454,7 +455,7 @@ export class Connector implements IConnector { } /** Event handler fired when HTTP response is available and DOM loaded. */ - private async onResponseReceived(params) { + private async onResponseReceived(params: Crdp.Network.ResponseReceivedEvent) { const resourceUrl: string = params.response.url; const hops: Array = this._redirects.calculate(resourceUrl); const originalUrl: string = hops[0] || resourceUrl; @@ -572,7 +573,7 @@ export class Connector implements IConnector { } /** Wait until the browser load the first tab */ - private getClient(port, tab): Promise { + private getClient(port: number, tab: number): Promise { let retries: number = 0; const loadCDP = async () => { try { @@ -662,9 +663,10 @@ export class Connector implements IConnector { await Promise.all([ Network.clearBrowserCache(), Network.setCacheDisabled({ cacheDisabled: true }), - Network.requestWillBeSent(this.onRequestWillBeSent.bind(this)), - Network.responseReceived(this.onResponseReceived.bind(this)), - Network.loadingFailed(this.onLoadingFailed.bind(this)) + // The typings we use for CDP aren't 100% compatible with our libarary + Network['requestWillBeSent'](this.onRequestWillBeSent.bind(this)), // eslint-disable-line dot-notation + Network['responseReceived'](this.onResponseReceived.bind(this)), // eslint-disable-line dot-notation + Network['loadingFailed'](this.onLoadingFailed.bind(this)) // eslint-disable-line dot-notation ]); } @@ -672,13 +674,14 @@ export class Connector implements IConnector { private async configureAndEnableCDP() { const { Network, Page } = this._client; - this._client.on('error', this.onError); - this._client.on('disconnect', this.onDisconnect); + // The typings we use for CDP aren't 100% compatible with our libarary + (this._client as any).on('error', this.onError); + (this._client as any).on('disconnect', this.onDisconnect); await this.enableNetworkEvents(); await Promise.all([ - Network.enable(), + Network.enable({}), Page.enable() ]); } @@ -902,7 +905,7 @@ export class Connector implements IConnector { const tab = this._tabs.pop(); try { - await cdp.Close({ id: tab.id, port: this._client.port }); // eslint-disable-line new-cap + await cdp.Close({ id: tab.id, port: (this._client as any).port }); // eslint-disable-line new-cap } catch (e) { debug(`Couldn't close tab ${tab.id}`); } @@ -910,7 +913,7 @@ export class Connector implements IConnector { try { - this._client.close(); + (this._client as any).close(); /* * We need to wait until the browser is closed because diff --git a/yarn.lock b/yarn.lock index 458bcad7d09..76ab011274d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1894,6 +1894,10 @@ chrome-launcher@^0.10.4: mkdirp "0.5.1" rimraf "^2.6.1" +chrome-remote-debug-protocol@^1.2.20180422: + version "1.2.20180422" + resolved "https://registry.yarnpkg.com/chrome-remote-debug-protocol/-/chrome-remote-debug-protocol-1.2.20180422.tgz#535846acbd3d6663e68301276372a2d02eb79e20" + chrome-remote-interface@^0.26.1: version "0.26.1" resolved "https://registry.yarnpkg.com/chrome-remote-interface/-/chrome-remote-interface-0.26.1.tgz#6c7d4479742b6d236752d716a9bc2d322d7d8ad2" From f9e32dee12bfedfc11537bc5f393b4dd4499e6f9 Mon Sep 17 00:00:00 2001 From: molant Date: Fri, 21 Sep 2018 13:49:38 -0700 Subject: [PATCH 09/11] Chore: Change `utf8` to `utf-8` Node supports both. Updated for consistency. --- .../src/debugging-protocol-connector.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts index a538a868186..355df3d95e1 100644 --- a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts +++ b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts @@ -321,7 +321,7 @@ export class Connector implements IConnector { try { const { body, base64Encoded } = await this._client.Network.getResponseBody({ requestId: cdpResponse.requestId }); - const encoding = base64Encoded ? 'base64' : 'utf8'; + const encoding = base64Encoded ? 'base64' : 'utf-8'; content = base64Encoded ? atob(body) : body; // There are some JS responses that are base64 encoded for some reason rawContent = Buffer.from(body, encoding); From fda932e5306c78d4915513b1cae3d4964e2e5547 Mon Sep 17 00:00:00 2001 From: molant Date: Fri, 21 Sep 2018 13:50:52 -0700 Subject: [PATCH 10/11] Chore: Prefix all `private` properties with `_` --- .../src/debugging-protocol-connector.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts index 355df3d95e1..5843987004b 100644 --- a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts +++ b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts @@ -72,13 +72,13 @@ export class Connector implements IConnector { /** The amount of time before an event is going to be timedout. */ private _timeout: number; /** Browser PID */ - private pid: number; + private _pid: number; private _targetNetworkData: NetworkData; private launcher: ILauncher; /** Promise that gets resolved when the taget is downloaded. */ private _waitForTarget: Promise; /** Function to call when the target is downloaded. */ - private targetReceived: Function; + private _targetReceived: Function; public constructor(engine: Engine, config: object, launcher: ILauncher) { const defaultOptions = { @@ -105,7 +105,7 @@ export class Connector implements IConnector { this.launcher = launcher; this._waitForTarget = new Promise((resolve) => { - this.targetReceived = resolve; + this._targetReceived = resolve; }); } @@ -499,7 +499,7 @@ export class Connector implements IConnector { response }; - this.targetReceived(); + this._targetReceived(); } eventName = `${eventName}::${getType(response.mediaType)}`; @@ -600,7 +600,7 @@ export class Connector implements IConnector { const launcher: BrowserInfo = await this.launcher.launch(this._options.useTabUrl ? this._options.tabUrl : 'about:blank'); let client; - this.pid = launcher.pid; + this._pid = launcher.pid; /* * We want a new tab for this session. If it is a new browser, a new tab @@ -878,7 +878,7 @@ export class Connector implements IConnector { * https://nodejs.org/api/process.html#process_process_kill_pid_signal */ - process.kill(this.pid, 0); + process.kill(this._pid, 0); maxTries--; @@ -889,7 +889,7 @@ export class Connector implements IConnector { await delay(50); } } catch (e) { - debug(`Process with ${this.pid} doesn't seem to be running`); + debug(`Process with ${this._pid} doesn't seem to be running`); finish = true; } } From d5811ef19ae8f95ea59128efb4a8b02a3f32abe7 Mon Sep 17 00:00:00 2001 From: molant Date: Fri, 21 Sep 2018 13:55:16 -0700 Subject: [PATCH 11/11] Fix: Truncated body in some circumstances To make sure the body is available, `getResponseBody` has to be called after the `loadingFinished` event is received for the `requestId`. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Fix #1325 --- .../src/debugging-protocol-connector.ts | 60 ++++++++++++------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts index 5843987004b..918dd7c0daa 100644 --- a/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts +++ b/packages/utils-debugging-protocol-common/src/debugging-protocol-connector.ts @@ -65,6 +65,8 @@ export class Connector implements IConnector { private _redirects = new RedirectManager(); /** A collection of requests with their initial data. */ private _pendingResponseReceived: Array; + /** Collection of */ + private _finishedRequests: Map; /** List of all the tabs used by the connector. */ private _tabs = []; /** Tells if a favicon of a page has been downloaded from a link tag. */ @@ -101,6 +103,7 @@ export class Connector implements IConnector { this._requests = new Map(); this._pendingResponseReceived = []; + this._finishedRequests = new Map(); this.launcher = launcher; @@ -304,6 +307,18 @@ export class Connector implements IConnector { } } + /** Wait until the given `requestId` request has loaded all the content. */ + // TODO: remove `any` from return type + private waitForContentLoaded(requestId: string): Promise { + if (this._finishedRequests.has(requestId)) { + return Promise.resolve(); + } + + return new Promise((resolve, reject) => { + this._finishedRequests.set(requestId, { reject, resolve }); + }); + } + private async getResponseBody(cdpResponse: Crdp.Network.ResponseReceivedEvent): Promise<{ content: string, rawContent: Buffer, rawResponse(): Promise }> { let content: string = ''; let rawContent: Buffer = null; @@ -320,6 +335,7 @@ export class Connector implements IConnector { } try { + await this.waitForContentLoaded(cdpResponse.requestId); const { body, base64Encoded } = await this._client.Network.getResponseBody({ requestId: cdpResponse.requestId }); const encoding = base64Encoded ? 'base64' : 'utf-8'; @@ -413,25 +429,7 @@ export class Connector implements IConnector { const resourceUrl: string = cdpResponse.response.url; const hops: Array = this._redirects.calculate(resourceUrl); const resourceHeaders: object = normalizeHeaders(cdpResponse.response.headers); - let { content, rawContent, rawResponse } = await this.getResponseBody(cdpResponse); - let retry = 3; - - /* - * Sometimes, the content is empty at the beginning, but - * after few millisecons, it isn't. - */ - while (!content && (!rawContent || rawContent.length === 0) && retry > 0) { - await delay(250); - - ({ content, rawContent, rawResponse } = await this.getResponseBody(cdpResponse)); - - retry--; - } - - if (retry === 0) { - debug(`${resourceUrl} is empty`); - } - + const { content, rawContent, rawResponse } = await this.getResponseBody(cdpResponse); const response: Response = { body: { content, @@ -445,7 +443,6 @@ export class Connector implements IConnector { statusCode: cdpResponse.response.status, url: resourceUrl }; - const { charset, mediaType } = getContentTypeData(element, resourceUrl, response.headers, response.body.rawContent); response.mediaType = mediaType; @@ -525,6 +522,21 @@ export class Connector implements IConnector { this._requests.delete(params.requestId); } + /** Event handler fired when an HTTP request has finished and all the content is available */ + private onLoadingFinished(params: Crdp.Network.LoadingFinishedEvent) { + const { requestId } = params; + + if (this._finishedRequests.has(requestId)) { + const { resolve } = this._finishedRequests.get(requestId); + + // We remove the ones that have been processed already + this._finishedRequests.delete(requestId); + resolve(); + } else { + this._finishedRequests.set(requestId, { reject() { }, resolve() { } }); + } + } + /** Traverses the DOM notifying when a new element is traversed. */ private async traverseAndNotify(element) { /* @@ -666,6 +678,7 @@ export class Connector implements IConnector { // The typings we use for CDP aren't 100% compatible with our libarary Network['requestWillBeSent'](this.onRequestWillBeSent.bind(this)), // eslint-disable-line dot-notation Network['responseReceived'](this.onResponseReceived.bind(this)), // eslint-disable-line dot-notation + Network['loadingFinished'](this.onLoadingFinished.bind(this)), // eslint-disable-line dot-notation Network['loadingFailed'](this.onLoadingFailed.bind(this)) // eslint-disable-line dot-notation ]); } @@ -899,6 +912,13 @@ export class Connector implements IConnector { } public async close() { + debug(`Canceling all pending requests: ${this._finishedRequests.size}`); + + this._finishedRequests.forEach(({ reject }, requestId) => { + debug(`Cancelling request ${requestId}`); + reject(); + }); + debug(`Pending tabs: ${this._tabs.length}`); while (this._tabs.length > 0) {