From 5d6253f7437609eae21ff1fccff32ebfb7f9747d Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 31 Aug 2022 21:51:38 -0700 Subject: [PATCH] fix: stop har recording when APIRequestContext is disposed (#17007) --- .../src/server/browserContext.ts | 4 +-- packages/playwright-core/src/server/fetch.ts | 3 +- .../src/server/trace/recorder/tracing.ts | 7 ++-- .../playwright-test/playwright.fetch.spec.ts | 34 +++++++++++++++++++ 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 87d3ce3672..ee77c4e533 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -216,7 +216,7 @@ export abstract class BrowserContext extends SdkObject { this._closedStatus = 'closed'; this._deleteAllDownloads(); this._downloads.clear(); - this.tracing.dispose(); + this.tracing.dispose().catch(() => {}); if (this._isPersistentContext) this.onClosePersistent(); this._closePromiseFulfill!(new Error('Context closed')); @@ -389,7 +389,7 @@ export abstract class BrowserContext extends SdkObject { for (const harRecorder of this._harRecorders.values()) await harRecorder.flush(); - await this.tracing.flush(); + await this.tracing.dispose(); // Cleanup. const promises: Promise[] = []; diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index f731c37c96..0fcf72eaf8 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -509,9 +509,8 @@ export class GlobalAPIRequestContext extends APIRequestContext { } override async dispose() { - await this._tracing.flush(); + await this._tracing.dispose(); await this._tracing.deleteTmpTracesDir(); - this._tracing.dispose(); this._disposeImpl(); } diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index 837b9ff524..c2e64a165e 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -200,13 +200,10 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps return this._tracesTmpDir; } - async flush() { - this._snapshotter?.dispose(); - await this._writeChain; - } - async dispose() { this._snapshotter?.dispose(); + this._harTracer.stop(); + await this._writeChain; } async stopChunk(params: TracingTracingStopChunkParams): Promise<{ artifact: Artifact | null, sourceEntries: NameValue[] | undefined }> { diff --git a/tests/playwright-test/playwright.fetch.spec.ts b/tests/playwright-test/playwright.fetch.spec.ts index 2b7ddee880..883777ab1c 100644 --- a/tests/playwright-test/playwright.fetch.spec.ts +++ b/tests/playwright-test/playwright.fetch.spec.ts @@ -50,3 +50,37 @@ test('should use baseURL in request fixture', async ({ runInlineTest, server }) expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); }); + +test('should stop tracing on requestContex.dispose()', async ({ runInlineTest, server }) => { + server.setRoute('/slow', (req, resp) => { + resp.writeHead(200, { + 'Content-Type': 'text/plain; charset=utf-8', + 'Content-Length': '3', + }); + setTimeout(() => { + resp.end('Hi!'); + }, 500); + }); + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { + reporter: 'html', + use: { + browserName: 'firefox', + trace:'retain-on-failure' + } + }; + `, + 'a.test.ts': ` + const { test } = pwt; + test('hanging request', async ({ page, request }) => { + const response = await page.goto('${server.EMPTY_PAGE}'); + expect(response.status()).toBe(200); + await request.get('${server.PREFIX}/slow'); + }); + `, + }, { workers: 1, timeout: 1000 }); + expect(result.output).not.toContain('ENOENT'); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); +});