diff --git a/packages/playwright-core/src/server/har/harTracer.ts b/packages/playwright-core/src/server/har/harTracer.ts index 923ab77d2c5e3..af0ecaa8b3e75 100644 --- a/packages/playwright-core/src/server/har/harTracer.ts +++ b/packages/playwright-core/src/server/har/harTracer.ts @@ -66,6 +66,11 @@ type HarTracerOptions = { export class HarTracer { private _context: BrowserContext | APIRequestContext; private _barrierPromises = new Set>(); + // Barriers that only upgrade already-recorded provisional data (raw headers, + // sizes, compression) and wait for the network "extra info" that may never + // arrive, see https://github.com/microsoft/playwright/issues/42448. They can + // be dropped when the recording stops instead of hanging the flush. + private _upgradeBarriers = new Set>(); private _delegate: HarTracerDelegate; private _options: HarTracerOptions; private _pageEntries: har.Page[] = []; @@ -198,14 +203,23 @@ export class HarTracer { this._addBarrier(page, promise); } - private _addBarrier(target: Page | Worker | null, promise: Promise) { + private _addBarrier(target: Page | Worker | null, promise: Promise): Promise | null { if (!target) return null; if (!this._options.waitForContentOnStop) - return; + return null; const race = target.openScope.safeRace(promise); this._barrierPromises.add(race); race.then(() => this._barrierPromises.delete(race)); + return race; + } + + private _addUpgradeBarrier(target: Page | Worker | null, promise: Promise) { + const race = this._addBarrier(target, promise); + if (!race) + return; + this._upgradeBarriers.add(race); + race.then(() => this._upgradeBarriers.delete(race)); } private _onAPIRequest(event: APIRequestEvent) { @@ -366,7 +380,7 @@ export class HarTracer { } }; if (compressionCalculationBarrier) - this._addBarrier(page || request.serviceWorker(), compressionCalculationBarrier.barrier); + this._addUpgradeBarrier(page || request.serviceWorker(), compressionCalculationBarrier.barrier); const promise = response.internalBody().then(buffer => { if (this._options.omitScripts && request.resourceType() === 'script') { @@ -396,7 +410,7 @@ export class HarTracer { this._computeHarEntryTotalTime(harEntry); if (!this._options.omitSizes) { - this._addBarrier(page || request.serviceWorker(), response.internalSizes().then(sizes => { + this._addUpgradeBarrier(page || request.serviceWorker(), response.internalSizes().then(sizes => { harEntry.response.bodySize = sizes.responseBodySize; harEntry.response.headersSize = sizes.responseHeadersSize; harEntry.response._transferSize = sizes.transferSize; @@ -619,13 +633,13 @@ export class HarTracer { } this._recordRequestOverrides(harEntry, request); - this._addBarrier(page || request.serviceWorker(), request.internalRawRequestHeaders().then(headers => { + this._addUpgradeBarrier(page || request.serviceWorker(), request.internalRawRequestHeaders().then(headers => { this._recordRequestHeadersAndCookies(harEntry, headers); })); // Record available headers including redirect location in case the tracing is stopped before // response extra info is received (in Chromium). this._recordResponseHeaders(harEntry, response.headers()); - this._addBarrier(page || request.serviceWorker(), response.internalRawResponseHeaders().then(headers => { + this._addUpgradeBarrier(page || request.serviceWorker(), response.internalRawResponseHeaders().then(headers => { this._recordResponseHeaders(harEntry, headers); })); } @@ -653,6 +667,14 @@ export class HarTracer { } async flush() { + // Drop the pending data upgrade barriers: provisional data is already + // recorded for them as a fallback. The extra info they wait for can arrive + // late (e.g. after loadingFinished), so it is not resolved on the server + // side - but once the recording stops, waiting for the upgrade would hang + // the flush, e.g. when saving the HAR at the context close. + // See https://github.com/microsoft/playwright/issues/42448. + for (const barrier of this._upgradeBarriers) + this._barrierPromises.delete(barrier); await Promise.all(this._barrierPromises); } @@ -660,6 +682,7 @@ export class HarTracer { this._started = false; eventsHelper.removeEventListeners(this._eventListeners); this._barrierPromises.clear(); + this._upgradeBarriers.clear(); const context = this._context instanceof BrowserContext ? this._context : undefined; const log: har.Log = { diff --git a/tests/library/har.spec.ts b/tests/library/har.spec.ts index 73d2e8bf52aa4..c8a2e17e9e3c0 100644 --- a/tests/library/har.spec.ts +++ b/tests/library/har.spec.ts @@ -18,7 +18,9 @@ import { browserTest as it, expect } from '../config/browserTest'; import fs from 'fs'; import path from 'path'; -import type { BrowserContext, BrowserContextOptions } from 'playwright-core'; +import http from 'http'; +import { WebSocket, WebSocketServer } from 'ws'; +import type { BrowserContext, BrowserContextOptions, BrowserType } from 'playwright-core'; import type { AddressInfo } from 'net'; import type { Log } from '../../packages/isomorphic/trace/versions/har'; import { parseHar } from '../config/utils'; @@ -940,6 +942,128 @@ it('should not hang on slow chunked response', async ({ browserName, browser, co expect(log.browser!.version).toBe(browser.version()); }); +// Chromium may advertise `hasExtraInfo` on a response and then never deliver the +// extra info, or stop reporting the request at all after `responseReceived`. +// To cover this, we connect over a CDP websocket proxy that withholds the +// corresponding events for a single request identified by url. +// See https://github.com/microsoft/playwright/issues/42448. +async function recordHarWithCensoredCdpEvents( + browserType: BrowserType, + server: TestServer, + testInfo: any, + censor: 'extraInfo' | 'extraInfoAndFinish'): Promise { + const port = 9339 + testInfo.workerIndex; + const browserServer = await browserType.launch({ args: ['--remote-debugging-port=' + port] }); + const json = await new Promise((resolve, reject) => { + http.get(`http://127.0.0.1:${port}/json/version/`, resp => { + let data = ''; + resp.on('data', chunk => data += chunk); + resp.on('end', () => resolve(data)); + }).on('error', reject); + }); + const upstreamUrl: string = JSON.parse(json).webSocketDebuggerUrl; + + let targetRequestId: string | undefined; + let dropped = 0; + const wss = new WebSocketServer({ port: 0, perMessageDeflate: false }); + wss.on('connection', (client: WebSocket) => { + const upstream = new WebSocket(upstreamUrl, { perMessageDeflate: false }); + const pending: string[] = []; + upstream.on('open', () => pending.splice(0).forEach(message => upstream.send(message))); + client.on('message', data => { + if (upstream.readyState === WebSocket.OPEN) + upstream.send(data.toString()); + else + pending.push(data.toString()); + }); + upstream.on('message', data => { + const text = data.toString(); + let message: any; + try { + message = JSON.parse(text); + } catch { + client.send(text); + return; + } + const params = message.params || {}; + if (message.method === 'Network.requestWillBeSent' && params.request?.url?.includes('/target.js')) + targetRequestId = params.requestId; + const isTarget = !!targetRequestId && params.requestId === targetRequestId; + if (isTarget && ( + message.method === 'Network.responseReceivedExtraInfo' || + (censor === 'extraInfoAndFinish' && message.method === 'Network.loadingFinished') + )) { + dropped++; + return; + } + client.send(text); + }); + const bye = () => { + try { client.close(); } catch {} + try { upstream.close(); } catch {} + }; + client.on('close', bye); + upstream.on('close', bye); + client.on('error', bye); + upstream.on('error', bye); + }); + + try { + const cdpBrowser = await browserType.connectOverCDP(`ws://127.0.0.1:${(wss.address() as AddressInfo).port}/`); + server.setRoute('/target.js', (req, res) => { + res.writeHead(200, { 'Content-Type': 'application/javascript' }); + res.end('window.targetLoaded = true;'); + }); + const harPath = testInfo.outputPath('test.har'); + const context = await cdpBrowser.newContext({ recordHar: { path: harPath } }); + const page = await context.newPage(); + await page.goto(server.EMPTY_PAGE); + await page.evaluate(url => { + const script = document.createElement('script'); + script.src = url; + document.body.appendChild(script); + }, server.PREFIX + '/target.js'); + + // Wait for the events to be withheld rather than guessing with a timeout: + // closing before the request was issued would pass for the wrong reason. + const expectedDropped = censor === 'extraInfoAndFinish' ? 2 : 1; + const deadline = Date.now() + 10000; + while (dropped < expectedDropped && Date.now() < deadline) + await page.waitForTimeout(50); + expect(dropped, 'number of withheld CDP events').toBe(expectedDropped); + + // Give the tracer a chance to register its raw header barriers before closing. + await page.waitForTimeout(500); + await context.close(); + return JSON.parse(fs.readFileSync(harPath).toString())['log'] as Log; + } finally { + await browserServer.close(); + await new Promise(f => wss.close(() => f(null))); + } +} + +it('should not hang when response extra info never arrives', async ({ browserName, browserType, server }, testInfo) => { + it.skip(browserName !== 'chromium', 'CDP-specific'); + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42448' }); + // `Network.loadingFinished` is delivered, but `Network.responseReceivedExtraInfo` never arrives. + const log = await recordHarWithCensoredCdpEvents(browserType, server, testInfo, 'extraInfo'); + const entry = log.entries.find(e => e.request.url.includes('/target.js'))!; + // Provisional response headers are recorded as a fallback for the raw ones. + expect(entry.response.status).toBe(200); + expect(entry.response.headers.some(h => h.name.toLowerCase() === 'content-type')).toBe(true); +}); + +it('should not hang when a response never finishes', async ({ browserName, browserType, server }, testInfo) => { + it.skip(browserName !== 'chromium', 'CDP-specific'); + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42448' }); + // Neither `Network.responseReceivedExtraInfo` nor `Network.loadingFinished` ever arrive. + const log = await recordHarWithCensoredCdpEvents(browserType, server, testInfo, 'extraInfoAndFinish'); + const entry = log.entries.find(e => e.request.url.includes('/target.js'))!; + // Provisional response headers are recorded as a fallback for the raw ones. + expect(entry.response.status).toBe(200); + expect(entry.response.headers.some(h => h.name.toLowerCase() === 'content-type')).toBe(true); +}); + it('should close the context when saving the har fails', async ({ contextFactory, server }, testInfo) => { it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42231' }); const filePath = testInfo.outputPath('not-a-directory');