From 3a81d2d11e4d2aed5735d8cfe6b308e25865426b Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Thu, 23 Nov 2023 21:47:04 +0530 Subject: [PATCH 01/12] feat: added defaultmap + locking mechanism for intercepts queue --- packages/core/src/network.js | 47 ++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index dd223c04c..e08e931ef 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -8,6 +8,32 @@ const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308]; const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other']; const ABORTED_MESSAGE = 'Request was aborted by browser'; +class DefaultMap extends Map { + constructor(getDefaultValue, ...mapConstructorArgs) { + super(...mapConstructorArgs); + + if (typeof getDefaultValue !== 'function') { + throw new Error('getDefaultValue must be a function'); + } + + this.getDefaultValue = getDefaultValue; + } + + get = (key) => { + if (!this.has(key)) { + this.set(key, this.getDefaultValue(key)); + } + + return super.get(key); + }; +}; + +class RequestLifeCycle { + constructor() { + this.requestWillBeSentKey = null; + this.checkRequestSent = new Promise((resolve) => (this.requestWillBeSentKey = resolve)); + } +} // The Interceptor class creates common handlers for dealing with intercepting asset requests // for a given page using various devtools protocol events and commands. export class Network { @@ -15,9 +41,10 @@ export class Network { log = logger('core:discovery'); + #requestsLifeCycle = new DefaultMap(() => new RequestLifeCycle()); #pending = new Map(); #requests = new Map(); - #intercepts = new Map(); + // #intercepts = new Map(); #authentications = new Set(); #aborted = new Set(); @@ -125,7 +152,7 @@ export class Network { if (!keepPending) { this.#pending.delete(requestId); - this.#intercepts.delete(requestId); + // this.#intercepts.delete(requestId); } } @@ -153,6 +180,9 @@ export class Network { // aborted. If the request is already pending, handle it; otherwise set it to be intercepted. _handleRequestPaused = async (session, event) => { let { networkId: requestId, requestId: interceptId, resourceType } = event; + + // wait for requestWillBeSent event + await this.#requestsLifeCycle.get(requestId).checkRequestSent; let pending = this.#pending.get(requestId); this.#pending.delete(requestId); @@ -160,9 +190,6 @@ export class Network { if (pending?.request.url === event.request.url && pending.request.method === event.request.method) { await this._handleRequest(session, { ...pending, resourceType, interceptId }); - } else { - // track the session that intercepted the request - this.#intercepts.set(requestId, { ...event, session }); } } @@ -175,15 +202,9 @@ export class Network { if (request.url.startsWith('data:')) return; if (this.intercept) { - let intercept = this.#intercepts.get(requestId); this.#pending.set(requestId, event); - - if (intercept) { - // handle the request with the session that intercepted it - let { session, requestId: interceptId, resourceType } = intercept; - await this._handleRequest(session, { ...event, resourceType, interceptId }); - this.#intercepts.delete(requestId); - } + // release lock + this.#requestsLifeCycle.get(requestId).requestWillBeSentKey(); } } From 2a12528c7bc318442a46d7374aad4b4d62e4c06b Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Thu, 23 Nov 2023 22:35:06 +0530 Subject: [PATCH 02/12] feat: added checkResponseSent lock --- packages/core/src/network.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index e08e931ef..60757ca9e 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -30,8 +30,10 @@ class DefaultMap extends Map { class RequestLifeCycle { constructor() { - this.requestWillBeSentKey = null; - this.checkRequestSent = new Promise((resolve) => (this.requestWillBeSentKey = resolve)); + this.releaseRequest = null; + this.releaseResponse = null; + this.checkRequestSent = new Promise((resolve) => (this.releaseRequest = resolve)); + this.checkResponseSent = new Promise((resolve) => (this.releaseResponse = resolve)); } } // The Interceptor class creates common handlers for dealing with intercepting asset requests @@ -204,7 +206,7 @@ export class Network { if (this.intercept) { this.#pending.set(requestId, event); // release lock - this.#requestsLifeCycle.get(requestId).requestWillBeSentKey(); + this.#requestsLifeCycle.get(requestId).releaseRequest(); } } @@ -234,8 +236,10 @@ export class Network { // Called when a response has been received for a specific request. Associates the response with // the request data and adds a buffer method to fetch the response body when needed. - _handleResponseReceived = (session, event) => { + _handleResponseReceived = async (session, event) => { let { requestId, response } = event; + // wait for upto 2 seconds to check if request has been processed + await Promise.any([this.#requestsLifeCycle.get(requestId).checkRequestSent, new Promise((resolve) => setTimeout(resolve, 2000))]); let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; @@ -245,6 +249,8 @@ export class Network { let result = await this.send(session, 'Network.getResponseBody', { requestId }); return Buffer.from(result.body, result.base64Encoded ? 'base64' : 'utf-8'); }; + // release response + this.#requestsLifeCycle.get(requestId).releaseResponse(); } // Called when a request streams events. These types of requests break asset discovery because @@ -258,7 +264,10 @@ export class Network { // Called when a request has finished loading which triggers the this.onrequestfinished // callback. The request should have an associated response and be finished with any redirects. _handleLoadingFinished = async event => { - let request = this.#requests.get(event.requestId); + let { requestId } = event; + // wait for upto 2 seconds for response + await Promise.any([this.#requestsLifeCycle.get(requestId).checkResponseSent, new Promise((resolve) => setTimeout(resolve, 2000))]); + let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; From 90b60850f9e437d55a7c0ed020aef81af1a956fc Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Fri, 24 Nov 2023 09:01:56 +0530 Subject: [PATCH 03/12] chore: add comments + rename variables --- packages/core/src/network.js | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 60757ca9e..6c2ec8331 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -8,6 +8,8 @@ const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308]; const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other']; const ABORTED_MESSAGE = 'Request was aborted by browser'; +// DefaultMap, which returns a default value for an uninitialized key +// Similar to defaultDict in python class DefaultMap extends Map { constructor(getDefaultValue, ...mapConstructorArgs) { super(...mapConstructorArgs); @@ -28,7 +30,10 @@ class DefaultMap extends Map { }; }; -class RequestLifeCycle { +// RequestLifeCycleHandler handles life cycle of a requestId +// Ideal flow: requestWillBeSent -> requestPaused -> responseReceived -> loadingFinished / loadingFailed +// ServiceWorker flow: requestWillBeSent -> responseReceived -> loadingFinished / loadingFailed +class RequestLifeCycleHandler { constructor() { this.releaseRequest = null; this.releaseResponse = null; @@ -43,7 +48,7 @@ export class Network { log = logger('core:discovery'); - #requestsLifeCycle = new DefaultMap(() => new RequestLifeCycle()); + #requestsLifeCycleHandler = new DefaultMap(() => new RequestLifeCycleHandler()); #pending = new Map(); #requests = new Map(); // #intercepts = new Map(); @@ -183,8 +188,8 @@ export class Network { _handleRequestPaused = async (session, event) => { let { networkId: requestId, requestId: interceptId, resourceType } = event; - // wait for requestWillBeSent event - await this.#requestsLifeCycle.get(requestId).checkRequestSent; + // wait for request to be sent + await this.#requestsLifeCycleHandler.get(requestId).checkRequestSent; let pending = this.#pending.get(requestId); this.#pending.delete(requestId); @@ -205,8 +210,8 @@ export class Network { if (this.intercept) { this.#pending.set(requestId, event); - // release lock - this.#requestsLifeCycle.get(requestId).releaseRequest(); + // release request + this.#requestsLifeCycleHandler.get(requestId).releaseRequest(); } } @@ -238,8 +243,9 @@ export class Network { // the request data and adds a buffer method to fetch the response body when needed. _handleResponseReceived = async (session, event) => { let { requestId, response } = event; - // wait for upto 2 seconds to check if request has been processed - await Promise.any([this.#requestsLifeCycle.get(requestId).checkRequestSent, new Promise((resolve) => setTimeout(resolve, 2000))]); + // wait for upto 2 seconds or check if request has been sent + // no explicitly wait on requestWillBePaused as we implictly wait on it, since it manipulates the lifeCycle of request using Fetch module + await Promise.any([this.#requestsLifeCycleHandler.get(requestId).checkRequestSent, new Promise((resolve) => setTimeout(resolve, 2000))]); let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; @@ -250,7 +256,7 @@ export class Network { return Buffer.from(result.body, result.base64Encoded ? 'base64' : 'utf-8'); }; // release response - this.#requestsLifeCycle.get(requestId).releaseResponse(); + this.#requestsLifeCycleHandler.get(requestId).releaseResponse(); } // Called when a request streams events. These types of requests break asset discovery because @@ -265,8 +271,8 @@ export class Network { // callback. The request should have an associated response and be finished with any redirects. _handleLoadingFinished = async event => { let { requestId } = event; - // wait for upto 2 seconds for response - await Promise.any([this.#requestsLifeCycle.get(requestId).checkResponseSent, new Promise((resolve) => setTimeout(resolve, 2000))]); + // wait for upto 2 seconds or check if response has been sent + await Promise.any([this.#requestsLifeCycleHandler.get(requestId).checkResponseSent, new Promise((resolve) => setTimeout(resolve, 2000))]); let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; From 1cda91cfe30fb50bc4466d3e17d829b65ec2593b Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Fri, 24 Nov 2023 13:44:02 +0530 Subject: [PATCH 04/12] refactor: move DefaultMap to core/utils --- packages/core/src/network.js | 24 +----------------------- packages/core/src/utils.js | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 6c2ec8331..73a5e8e94 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -1,35 +1,13 @@ import { request as makeRequest } from '@percy/client/utils'; import logger from '@percy/logger'; import mime from 'mime-types'; -import { createResource, hostnameMatches, normalizeURL, waitFor } from './utils.js'; +import { DefaultMap, createResource, hostnameMatches, normalizeURL, waitFor } from './utils.js'; const MAX_RESOURCE_SIZE = 25 * (1024 ** 2); // 25MB const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308]; const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other']; const ABORTED_MESSAGE = 'Request was aborted by browser'; -// DefaultMap, which returns a default value for an uninitialized key -// Similar to defaultDict in python -class DefaultMap extends Map { - constructor(getDefaultValue, ...mapConstructorArgs) { - super(...mapConstructorArgs); - - if (typeof getDefaultValue !== 'function') { - throw new Error('getDefaultValue must be a function'); - } - - this.getDefaultValue = getDefaultValue; - } - - get = (key) => { - if (!this.has(key)) { - this.set(key, this.getDefaultValue(key)); - } - - return super.get(key); - }; -}; - // RequestLifeCycleHandler handles life cycle of a requestId // Ideal flow: requestWillBeSent -> requestPaused -> responseReceived -> loadingFinished / loadingFailed // ServiceWorker flow: requestWillBeSent -> responseReceived -> loadingFinished / loadingFailed diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index ea210323e..cdb190243 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -320,3 +320,25 @@ export function serializeFunction(fn) { return fnbody; } + +// DefaultMap, which returns a default value for an uninitialized key +// Similar to defaultDict in python +export class DefaultMap extends Map { + constructor(getDefaultValue, ...mapConstructorArgs) { + super(...mapConstructorArgs); + + if (typeof getDefaultValue !== 'function') { + throw new Error('getDefaultValue must be a function'); + } + + this.getDefaultValue = getDefaultValue; + } + + get = (key) => { + if (!this.has(key)) { + this.set(key, this.getDefaultValue(key)); + } + + return super.get(key); + }; +}; From 63ec621bc4d85ca9a6460ccaa99c8efea2547dca Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Fri, 24 Nov 2023 14:00:15 +0530 Subject: [PATCH 05/12] feat: track lifecycle for ALL requests, does not matter if nterwork is intercepted or not --- packages/core/src/network.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 73a5e8e94..08730e524 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -188,9 +188,11 @@ export class Network { if (this.intercept) { this.#pending.set(requestId, event); - // release request - this.#requestsLifeCycleHandler.get(requestId).releaseRequest(); } + // release request + // note: we are releasing this, even if intercept is not set for network.js + // since, we want to process all-requests in-order doesn't matter if it should be intercepted or not + this.#requestsLifeCycleHandler.get(requestId).releaseRequest(); } // Called when a pending request is paused. Handles associating redirected requests with @@ -221,9 +223,9 @@ export class Network { // the request data and adds a buffer method to fetch the response body when needed. _handleResponseReceived = async (session, event) => { let { requestId, response } = event; - // wait for upto 2 seconds or check if request has been sent + // await on requestWillBeSent // no explicitly wait on requestWillBePaused as we implictly wait on it, since it manipulates the lifeCycle of request using Fetch module - await Promise.any([this.#requestsLifeCycleHandler.get(requestId).checkRequestSent, new Promise((resolve) => setTimeout(resolve, 2000))]); + await this.#requestsLifeCycleHandler.get(requestId).checkRequestSent; let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; @@ -250,7 +252,7 @@ export class Network { _handleLoadingFinished = async event => { let { requestId } = event; // wait for upto 2 seconds or check if response has been sent - await Promise.any([this.#requestsLifeCycleHandler.get(requestId).checkResponseSent, new Promise((resolve) => setTimeout(resolve, 2000))]); + await this.#requestsLifeCycleHandler.get(requestId).checkResponseSent; let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; From 8fb51fc5bea258788c5076636d647732cb2bd310 Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Fri, 24 Nov 2023 15:17:01 +0530 Subject: [PATCH 06/12] feat: add locking mechanism for Network.loadingFailed and Network.eventSourceMessageReceived --- packages/core/src/network.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 08730e524..daa406702 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -241,8 +241,11 @@ export class Network { // Called when a request streams events. These types of requests break asset discovery because // they never finish loading, so we untrack them to signal idle after the first event. - _handleEventSourceMessageReceived = event => { - let request = this.#requests.get(event.requestId); + _handleEventSourceMessageReceived = async event => { + let { requestId } = event; + // wait for request to be sent + await this.#requestsLifeCycleHandler.get(requestId).checkRequestSent; + let request = this.#requests.get(requestId); /* istanbul ignore else: race condition paranioa */ if (request) this._forgetRequest(request); } @@ -262,7 +265,13 @@ export class Network { } // Called when a request has failed loading and triggers the this.onrequestfailed callback. - _handleLoadingFailed = event => { + _handleLoadingFailed = async event => { + let { requestId } = event; + // wait for request to be sent + // note: we are waiting on checkRequestSent and NOT checkResponseSent + // since, requests can be cancelled in-flight without Network.responseReceived having been triggered + // and in any case, order of processing for responseReceived and loadingFailed does not matter, as response capturing is done in loadingFinished + await this.#requestsLifeCycleHandler.get(requestId).checkRequestSent let request = this.#requests.get(event.requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; From f70b9eadaa784fe9f56748996ba4866d73b41c1f Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Fri, 24 Nov 2023 15:17:38 +0530 Subject: [PATCH 07/12] refactor: lock names in requestLifeCycle --- packages/core/src/network.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index daa406702..98833aa30 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -13,10 +13,10 @@ const ABORTED_MESSAGE = 'Request was aborted by browser'; // ServiceWorker flow: requestWillBeSent -> responseReceived -> loadingFinished / loadingFailed class RequestLifeCycleHandler { constructor() { - this.releaseRequest = null; - this.releaseResponse = null; - this.checkRequestSent = new Promise((resolve) => (this.releaseRequest = resolve)); - this.checkResponseSent = new Promise((resolve) => (this.releaseResponse = resolve)); + this.resolveRequestWillBeSent = null; + this.resolveResponseReceived = null; + this.requestWillBeSent = new Promise((resolve) => (this.resolveRequestWillBeSent = resolve)); + this.responseReceived = new Promise((resolve) => (this.resolveResponseReceived = resolve)); } } // The Interceptor class creates common handlers for dealing with intercepting asset requests @@ -167,7 +167,7 @@ export class Network { let { networkId: requestId, requestId: interceptId, resourceType } = event; // wait for request to be sent - await this.#requestsLifeCycleHandler.get(requestId).checkRequestSent; + await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; let pending = this.#pending.get(requestId); this.#pending.delete(requestId); @@ -192,7 +192,7 @@ export class Network { // release request // note: we are releasing this, even if intercept is not set for network.js // since, we want to process all-requests in-order doesn't matter if it should be intercepted or not - this.#requestsLifeCycleHandler.get(requestId).releaseRequest(); + this.#requestsLifeCycleHandler.get(requestId).resolveRequestWillBeSent(); } // Called when a pending request is paused. Handles associating redirected requests with @@ -225,7 +225,7 @@ export class Network { let { requestId, response } = event; // await on requestWillBeSent // no explicitly wait on requestWillBePaused as we implictly wait on it, since it manipulates the lifeCycle of request using Fetch module - await this.#requestsLifeCycleHandler.get(requestId).checkRequestSent; + await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; @@ -236,7 +236,7 @@ export class Network { return Buffer.from(result.body, result.base64Encoded ? 'base64' : 'utf-8'); }; // release response - this.#requestsLifeCycleHandler.get(requestId).releaseResponse(); + this.#requestsLifeCycleHandler.get(requestId).resolveResponseReceived(); } // Called when a request streams events. These types of requests break asset discovery because @@ -244,7 +244,7 @@ export class Network { _handleEventSourceMessageReceived = async event => { let { requestId } = event; // wait for request to be sent - await this.#requestsLifeCycleHandler.get(requestId).checkRequestSent; + await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; let request = this.#requests.get(requestId); /* istanbul ignore else: race condition paranioa */ if (request) this._forgetRequest(request); @@ -255,7 +255,7 @@ export class Network { _handleLoadingFinished = async event => { let { requestId } = event; // wait for upto 2 seconds or check if response has been sent - await this.#requestsLifeCycleHandler.get(requestId).checkResponseSent; + await this.#requestsLifeCycleHandler.get(requestId).responseReceived; let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; @@ -268,10 +268,10 @@ export class Network { _handleLoadingFailed = async event => { let { requestId } = event; // wait for request to be sent - // note: we are waiting on checkRequestSent and NOT checkResponseSent + // note: we are waiting on requestWillBeSent and NOT responseReceived // since, requests can be cancelled in-flight without Network.responseReceived having been triggered // and in any case, order of processing for responseReceived and loadingFailed does not matter, as response capturing is done in loadingFinished - await this.#requestsLifeCycleHandler.get(requestId).checkRequestSent + await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent let request = this.#requests.get(event.requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; From c1ec3d6059fa9281ca3e8634afbd8d9c4e8f71a3 Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Fri, 24 Nov 2023 15:18:17 +0530 Subject: [PATCH 08/12] chore: lint fix --- packages/core/src/network.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 98833aa30..99820e405 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -271,7 +271,7 @@ export class Network { // note: we are waiting on requestWillBeSent and NOT responseReceived // since, requests can be cancelled in-flight without Network.responseReceived having been triggered // and in any case, order of processing for responseReceived and loadingFailed does not matter, as response capturing is done in loadingFinished - await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent + await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; let request = this.#requests.get(event.requestId); /* istanbul ignore if: race condition paranioa */ if (!request) return; From 98ff139471ecccae35158168ac4faaaec1642ab5 Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Fri, 24 Nov 2023 22:12:37 +0530 Subject: [PATCH 09/12] fix: coverage issue with single if blocks --- packages/core/src/network.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 99820e405..c95713b54 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -172,10 +172,9 @@ export class Network { this.#pending.delete(requestId); // guard against redirects with the same requestId - if (pending?.request.url === event.request.url && - pending.request.method === event.request.method) { - await this._handleRequest(session, { ...pending, resourceType, interceptId }); - } + pending?.request.url === event.request.url && + pending.request.method === event.request.method && + await this._handleRequest(session, { ...pending, resourceType, interceptId }); } // Called when a request will be sent. If the request has already been intercepted, handle it; From f3c6511593b065f3029e057f2abd42ecc3743303 Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Sun, 26 Nov 2023 15:14:19 +0530 Subject: [PATCH 10/12] test: added defaultMap specs --- packages/core/test/unit/utils.test.js | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/core/test/unit/utils.test.js b/packages/core/test/unit/utils.test.js index 354dc1bb7..f5db2fa06 100644 --- a/packages/core/test/unit/utils.test.js +++ b/packages/core/test/unit/utils.test.js @@ -2,7 +2,8 @@ import { generatePromise, AbortController, yieldTo, - yieldAll + yieldAll, + DefaultMap } from '../../src/utils.js'; describe('Unit / Utils', () => { @@ -165,4 +166,21 @@ describe('Unit / Utils', () => { { done: true, value: [2, 4, null, 3, 6] }); }); }); + + describe('DefaultMap', () => { + it('should throw an error if getDefaultValue is not a function', () => { + expect(() => new DefaultMap('not a function')).toThrow(new Error('getDefaultValue must be a function')); + }); + + it('should return the default value for a key that has not been set', () => { + const map = new DefaultMap((key) => `default value for ${key}`); + expect(map.get('testKey')).toEqual('default value for testKey'); + }); + + it('should return the correct value for a key that has been set', () => { + const map = new DefaultMap((key) => `default value for ${key}`); + map.set('testKey', 'testValue'); + expect(map.get('testKey')).toEqual('testValue'); + }); + }); }); From ebd0542f2d8c1a20f1456414a0c3511954280888 Mon Sep 17 00:00:00 2001 From: nilshah98 Date: Mon, 27 Nov 2023 13:06:18 +0530 Subject: [PATCH 11/12] chore: remove unused intercepts map --- packages/core/src/network.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index c95713b54..a6f9ad00a 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -29,7 +29,6 @@ export class Network { #requestsLifeCycleHandler = new DefaultMap(() => new RequestLifeCycleHandler()); #pending = new Map(); #requests = new Map(); - // #intercepts = new Map(); #authentications = new Set(); #aborted = new Set(); @@ -137,7 +136,6 @@ export class Network { if (!keepPending) { this.#pending.delete(requestId); - // this.#intercepts.delete(requestId); } } From 3582916f066e009ccb1aa066c490b6fe9e7936fd Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 28 Nov 2023 17:02:39 +0530 Subject: [PATCH 12/12] Capture service worker intercepted requests (#1443) * feat: added new config discovery.captureServiceWorker * feat: use captureServiceWorker config to capture original requests * chore: lint fix * test: added spec for captureServiceWorker * chore: rename captureServiceWorker -> captureMockedServiceWorker --- packages/core/src/config.js | 5 +++ packages/core/src/discovery.js | 2 + packages/core/src/network.js | 14 ++++-- packages/core/src/snapshot.js | 1 + packages/core/test/discovery.test.js | 64 ++++++++++++++++++++++++++++ packages/core/types/index.d.ts | 1 + 6 files changed, 83 insertions(+), 4 deletions(-) diff --git a/packages/core/src/config.js b/packages/core/src/config.js index ce1d5bef0..39a8f68ab 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -167,6 +167,10 @@ export const configSchema = { disableCache: { type: 'boolean' }, + captureMockedServiceWorker: { + type: 'boolean', + default: false + }, requestHeaders: { type: 'object', normalize: false, @@ -249,6 +253,7 @@ export const snapshotSchema = { requestHeaders: { $ref: '/config/discovery#/properties/requestHeaders' }, authorization: { $ref: '/config/discovery#/properties/authorization' }, disableCache: { $ref: '/config/discovery#/properties/disableCache' }, + captureMockedServiceWorker: { $ref: '/config/discovery#/properties/captureMockedServiceWorker' }, userAgent: { $ref: '/config/discovery#/properties/userAgent' }, devicePixelRatio: { $ref: '/config/discovery#/properties/devicePixelRatio' } } diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index b0e4ca885..a3a4fb6cf 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -52,6 +52,7 @@ function debugSnapshotOptions(snapshot) { debugProp(snapshot, 'discovery.requestHeaders', JSON.stringify); debugProp(snapshot, 'discovery.authorization', JSON.stringify); debugProp(snapshot, 'discovery.disableCache'); + debugProp(snapshot, 'discovery.captureMockedServiceWorker'); debugProp(snapshot, 'discovery.userAgent'); debugProp(snapshot, 'clientInfo'); debugProp(snapshot, 'environmentInfo'); @@ -288,6 +289,7 @@ export function createDiscoveryQueue(percy) { requestHeaders: snapshot.discovery.requestHeaders, authorization: snapshot.discovery.authorization, userAgent: snapshot.discovery.userAgent, + captureMockedServiceWorker: snapshot.discovery.captureMockedServiceWorker, meta: snapshot.meta, // enable network inteception diff --git a/packages/core/src/network.js b/packages/core/src/network.js index a6f9ad00a..cff14b163 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -37,6 +37,7 @@ export class Network { this.timeout = options.networkIdleTimeout ?? 100; this.authorization = options.authorization; this.requestHeaders = options.requestHeaders ?? {}; + this.captureMockedServiceWorker = options.captureMockedServiceWorker ?? false; this.userAgent = options.userAgent ?? // by default, emulate a non-headless browser page.session.browser.version.userAgent.replace('Headless', ''); @@ -54,7 +55,7 @@ export class Network { let commands = [ session.send('Network.enable'), - session.send('Network.setBypassServiceWorker', { bypass: true }), + session.send('Network.setBypassServiceWorker', { bypass: !this.captureMockedServiceWorker }), session.send('Network.setCacheDisabled', { cacheDisabled: true }), session.send('Network.setUserAgentOverride', { userAgent: this.userAgent }), session.send('Network.setExtraHTTPHeaders', { headers: this.requestHeaders }) @@ -178,13 +179,16 @@ export class Network { // Called when a request will be sent. If the request has already been intercepted, handle it; // otherwise set it to be pending until it is paused. _handleRequestWillBeSent = async event => { - let { requestId, request } = event; + let { requestId, request, type } = event; // do not handle data urls if (request.url.startsWith('data:')) return; if (this.intercept) { this.#pending.set(requestId, event); + if (this.captureMockedServiceWorker) { + await this._handleRequest(undefined, { ...event, resourceType: type, interceptId: requestId }, true); + } } // release request // note: we are releasing this, even if intercept is not set for network.js @@ -195,7 +199,7 @@ export class Network { // Called when a pending request is paused. Handles associating redirected requests with // responses and calls this.onrequest with request info and callbacks to continue, respond, // or abort a request. One of the callbacks is required to be called and only one. - _handleRequest = async (session, event) => { + _handleRequest = async (session, event, serviceWorker = false) => { let { request, requestId, interceptId, resourceType } = event; let redirectChain = []; @@ -213,7 +217,9 @@ export class Network { request.redirectChain = redirectChain; this.#requests.set(requestId, request); - await sendResponseResource(this, request, session); + if (!serviceWorker) { + await sendResponseResource(this, request, session); + } } // Called when a response has been received for a specific request. Associates the response with diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index a739f4ec9..5884a00ac 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -114,6 +114,7 @@ function getSnapshotOptions(options, { config, meta }) { requestHeaders: config.discovery.requestHeaders, authorization: config.discovery.authorization, disableCache: config.discovery.disableCache, + captureMockedServiceWorker: config.discovery.captureMockedServiceWorker, userAgent: config.discovery.userAgent } }, options], (path, prev, next) => { diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 490b8387a..4c9f40d68 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1970,4 +1970,68 @@ describe('Discovery', () => { }); }); }); + + describe('Service Worker =>', () => { + it('captures original request', async () => { + server.reply('/sw.js', () => [200, 'text/javascript', dedent` + const fetchUpstream = async(request) => { + return await fetch(request.clone()); + } + + self.addEventListener('fetch', (event) => { + const { request } = event + event.respondWith(fetchUpstream(request)); + }); + + self.addEventListener("activate", (event) => { + event.waitUntil(clients.claim()); + }); + `]); + + server.reply('/app.js', () => [200, 'text/javascript', dedent` + const registerServiceWorker = async () => { + await navigator.serviceWorker.register('sw.js',{ scope: './', }); + }; + + await registerServiceWorker(); + + // create and insert image element which will be intercepted and resolved by service worker + // adding a sleep of 1s for service worker to get activated + await new Promise(r => setTimeout(r, 1000)); + var img = document.createElement('img'); + img.id = 'injected-image'; + img.src = './img.gif'; + document.getElementById('container').appendChild(img); + `]); + + server.reply('/', () => [200, 'text/html', dedent` + +
+ + + `]); + + await percy.snapshot({ + name: 'first service worker snapshot', + url: 'http://localhost:8000', + waitForSelector: '#injected-image', + discovery: { + captureMockedServiceWorker: true + } + }); + + await percy.idle(); + + let paths = server.requests.map(r => r[0]); + expect(paths).toContain('/img.gif'); + expect(captured).toContain(jasmine.arrayContaining([ + jasmine.objectContaining({ + id: sha256hash(pixel), + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/img.gif' + }) + }) + ])); + }); + }); }); diff --git a/packages/core/types/index.d.ts b/packages/core/types/index.d.ts index 79b170316..a52b7ff09 100644 --- a/packages/core/types/index.d.ts +++ b/packages/core/types/index.d.ts @@ -18,6 +18,7 @@ interface DiscoveryOptions { authorization?: AuthCredentials; allowedHostnames?: string[]; disableCache?: boolean; + captureMockedServiceWorker?: boolean; } interface DiscoveryLaunchOptions {