From 88581626927a520d45e92854d68eb1de17adbb3a Mon Sep 17 00:00:00 2001 From: Ross Wollman Date: Tue, 12 Jul 2022 13:23:35 -0700 Subject: [PATCH] fix: Service Workers+Interception: missing page-level Network events (#15549) Fixes #15474. Notes: * page-level requests that are also handled by a SW's fetch handler, should not be interceptable at the page-level * `Network.requestWillBeSent` does not provide enough metadata for Playwright to fire the `request` event at that time, so it does it as soon as it gets to the end of the request lifecycle --- .../src/server/chromium/crNetworkManager.ts | 14 ++- tests/assets/serviceworkers/fetchdummy/sw.js | 4 + tests/library/chromium/chromium.spec.ts | 110 +++++++++++++----- 3 files changed, 99 insertions(+), 29 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crNetworkManager.ts b/packages/playwright-core/src/server/chromium/crNetworkManager.ts index ab46ec66f5..72c3576e21 100644 --- a/packages/playwright-core/src/server/chromium/crNetworkManager.ts +++ b/packages/playwright-core/src/server/chromium/crNetworkManager.ts @@ -363,7 +363,19 @@ export class CRNetworkManager { } _onResponseReceived(event: Protocol.Network.responseReceivedPayload) { - const request = this._requestIdToRequest.get(event.requestId); + let request = this._requestIdToRequest.get(event.requestId); + // For frame-level Requests that are handled by a Service Worker's fetch handler, we'll never get a requestPaused event, so we need to + // manually create the request. In an ideal world, crNetworkManager would be able to know this on Network.requestWillBeSent, but there + // is not enough metadata there. + if (!request && event.response.fromServiceWorker) { + const requestWillBeSentEvent = this._requestIdToRequestWillBeSentEvent.get(event.requestId); + const frame = requestWillBeSentEvent?.frameId ? this._page?._frameManager.frame(requestWillBeSentEvent.frameId) : null; + if (requestWillBeSentEvent && frame) { + this._onRequest(frame, requestWillBeSentEvent, null /* requestPausedPayload */); + request = this._requestIdToRequest.get(event.requestId); + this._requestIdToRequestWillBeSentEvent.delete(event.requestId); + } + } // FileUpload sends a response without a matching request. if (!request) return; diff --git a/tests/assets/serviceworkers/fetchdummy/sw.js b/tests/assets/serviceworkers/fetchdummy/sw.js index ad4a3bea1c..4e82651a44 100644 --- a/tests/assets/serviceworkers/fetchdummy/sw.js +++ b/tests/assets/serviceworkers/fetchdummy/sw.js @@ -3,6 +3,10 @@ self.addEventListener('fetch', event => { event.respondWith(fetch(event.request)); return; } + if (event.request.url.includes('error')) { + event.respondWith(Promise.reject(new Error('uh oh'))); + return; + } const slash = event.request.url.lastIndexOf('/'); const name = event.request.url.substring(slash + 1); const blob = new Blob(["responseFromServiceWorker:" + name], {type : 'text/css'}); diff --git a/tests/library/chromium/chromium.spec.ts b/tests/library/chromium/chromium.spec.ts index 8d105f2b5f..27d0984bf4 100644 --- a/tests/library/chromium/chromium.spec.ts +++ b/tests/library/chromium/chromium.spec.ts @@ -358,38 +358,92 @@ test('setOffline', async ({ context, page, server }) => { expect(error).toMatch(/REJECTED.*Failed to fetch/); }); -test('should emit page-level request event for respondWith', async ({ page, server }) => { - await page.goto(server.PREFIX + '/serviceworkers/fetchdummy/sw.html'); - await page.evaluate(() => window['activationPromise']); +test.describe('should emit page-level network events with service worker fetch handler', () => { + test.describe('when not using routing', () => { + test('successful request', async ({ page, server }) => { + await page.goto(server.PREFIX + '/serviceworkers/fetchdummy/sw.html'); + await page.evaluate(() => window['activationPromise']); - // Sanity check. - const [pageReq, swResponse] = await Promise.all([ - page.waitForEvent('request'), - page.evaluate(() => window['fetchDummy']('foo')), - ]); - expect(swResponse).toBe('responseFromServiceWorker:foo'); - expect(pageReq.url()).toMatch(/fetchdummy\/foo$/); - expect(pageReq.serviceWorker()).toBe(null); - expect((await pageReq.response()).fromServiceWorker()).toBe(true); -}); + const [pageReq, pageResp, /* pageFinished */, swResponse] = await Promise.all([ + page.waitForEvent('request'), + page.waitForEvent('response'), + page.waitForEvent('requestfinished'), + page.evaluate(() => window['fetchDummy']('foo')), + ]); + expect(swResponse).toBe('responseFromServiceWorker:foo'); + expect(pageReq.url()).toMatch(/fetchdummy\/foo$/); + expect(pageReq.serviceWorker()).toBe(null); + expect(pageResp.fromServiceWorker()).toBe(true); + expect(pageResp).toBe(await pageReq.response()); + expect((await pageReq.response()).fromServiceWorker()).toBe(true); + }); -test('should emit page-level request event for respondWith when interception enabled', async ({ page, server, context }) => { - test.fixme(); - test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/15474' }); + test('failed request', async ({ page, server }) => { + await page.goto(server.PREFIX + '/serviceworkers/fetchdummy/sw.html'); + await page.evaluate(() => window['activationPromise']); - await context.route('**', route => route.continue()); - await page.goto(server.PREFIX + '/serviceworkers/fetchdummy/sw.html'); - await page.evaluate(() => window['activationPromise']); + const [pageReq] = await Promise.all([ + page.waitForEvent('request'), + page.waitForEvent('requestfailed'), + page.evaluate(() => window['fetchDummy']('error')).catch(e => e), + ]); + expect(pageReq.url()).toMatch(/fetchdummy\/error$/); + expect(pageReq.failure().errorText).toMatch(/net::ERR_FAILED/); + expect(pageReq.serviceWorker()).toBe(null); + expect(await pageReq.response()).toBe(null); + }); + }); - // Sanity check. - const [pageReq, swResponse] = await Promise.all([ - page.waitForEvent('request'), - page.evaluate(() => window['fetchDummy']('foo')), - ]); - expect(swResponse).toBe('responseFromServiceWorker:foo'); - expect(pageReq.url()).toMatch(/fetchdummy\/foo$/); - expect(pageReq.serviceWorker()).toBe(null); - expect((await pageReq.response()).fromServiceWorker()).toBe(true); + test.describe('when routing', () => { + test('successful request', async ({ page, server, context }) => { + await context.route('**', route => route.continue()); + let markFailureIfPageRoutesARequestAlreadyHandledByServiceWorker = false; + await page.route('**', route => { + if (route.request().url().endsWith('foo')) + markFailureIfPageRoutesARequestAlreadyHandledByServiceWorker = true; + route.continue(); + }); + await page.goto(server.PREFIX + '/serviceworkers/fetchdummy/sw.html'); + await page.evaluate(() => window['activationPromise']); + + const [pageReq, pageResp, /* pageFinished */, swResponse] = await Promise.all([ + page.waitForEvent('request'), + page.waitForEvent('response'), + page.waitForEvent('requestfinished'), + page.evaluate(() => window['fetchDummy']('foo')), + ]); + expect(swResponse).toBe('responseFromServiceWorker:foo'); + expect(pageReq.url()).toMatch(/fetchdummy\/foo$/); + expect(pageReq.serviceWorker()).toBe(null); + expect(pageResp.fromServiceWorker()).toBe(true); + expect(pageResp).toBe(await pageReq.response()); + expect((await pageReq.response()).fromServiceWorker()).toBe(true); + expect(markFailureIfPageRoutesARequestAlreadyHandledByServiceWorker).toBe(false); + }); + + test('failed request', async ({ page, server, context }) => { + await context.route('**', route => route.continue()); + let markFailureIfPageRoutesARequestAlreadyHandledByServiceWorker = false; + await page.route('**', route => { + if (route.request().url().endsWith('foo')) + markFailureIfPageRoutesARequestAlreadyHandledByServiceWorker = true; + route.continue(); + }); + await page.goto(server.PREFIX + '/serviceworkers/fetchdummy/sw.html'); + await page.evaluate(() => window['activationPromise']); + + const [pageReq] = await Promise.all([ + page.waitForEvent('request'), + page.waitForEvent('requestfailed'), + page.evaluate(() => window['fetchDummy']('error')).catch(e => e), + ]); + expect(pageReq.url()).toMatch(/fetchdummy\/error$/); + expect(pageReq.failure().errorText).toMatch(/net::ERR_FAILED/); + expect(pageReq.serviceWorker()).toBe(null); + expect(await pageReq.response()).toBe(null); + expect(markFailureIfPageRoutesARequestAlreadyHandledByServiceWorker).toBe(false); + }); + }); }); test('setExtraHTTPHeaders', async ({ context, page, server }) => {