chore: throw on context.close() if it was closed externally (#21347)

Author: pavelfeldman

packages/playwright-core/src/client/browserContext.ts

@@ -30,7 +30,6 @@ import { Waiter } from './waiter';
 import type { URLMatch, Headers, WaitForEventOptions, BrowserContextOptions, StorageState, LaunchOptions } from './types';
 import { headersObjectToArray, isRegExp, isString } from '../utils';
 import { mkdirIfNeeded } from '../utils/fileUtils';
-import { isSafeCloseError } from '../common/errors';
 import type * as api from '../../types/types';
 import type * as structs from '../../types/structs';
 import { CDPSession } from './cdpSession';
@@ -59,6 +58,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
   readonly _serviceWorkers = new Set<Worker>();
   readonly _isChromium: boolean;
   private _harRecorders = new Map<string, { path: string, content: 'embed' | 'attach' | 'omit' | undefined }>();
+  private _closeWasCalled = false;
 
   static from(context: channels.BrowserContextChannel): BrowserContext {
     return (context as any)._object;
@@ -344,31 +344,28 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
   }
 
   async close(): Promise<void> {
-    try {
-      await this._wrapApiCall(async () => {
-        await this._browserType?._onWillCloseContext?.(this);
-        for (const [harId, harParams] of this._harRecorders) {
-          const har = await this._channel.harExport({ harId });
-          const artifact = Artifact.from(har.artifact);
-          // Server side will compress artifact if content is attach or if file is .zip.
-          const isCompressed = harParams.content === 'attach' || harParams.path.endsWith('.zip');
-          const needCompressed = harParams.path.endsWith('.zip');
-          if (isCompressed && !needCompressed) {
-            await artifact.saveAs(harParams.path + '.tmp');
-            await this._connection.localUtils()._channel.harUnzip({ zipFile: harParams.path + '.tmp', harFile: harParams.path });
-          } else {
-            await artifact.saveAs(harParams.path);
-          }
-          await artifact.delete();
+    if (this._closeWasCalled)
+      return;
+    this._closeWasCalled = true;
+    await this._wrapApiCall(async () => {
+      await this._browserType?._onWillCloseContext?.(this);
+      for (const [harId, harParams] of this._harRecorders) {
+        const har = await this._channel.harExport({ harId });
+        const artifact = Artifact.from(har.artifact);
+        // Server side will compress artifact if content is attach or if file is .zip.
+        const isCompressed = harParams.content === 'attach' || harParams.path.endsWith('.zip');
+        const needCompressed = harParams.path.endsWith('.zip');
+        if (isCompressed && !needCompressed) {
+          await artifact.saveAs(harParams.path + '.tmp');
+          await this._connection.localUtils()._channel.harUnzip({ zipFile: harParams.path + '.tmp', harFile: harParams.path });
+        } else {
+          await artifact.saveAs(harParams.path);
         }
-      }, true);
-      await this._channel.close();
-      await this._closedPromise;
-    } catch (e) {
-      if (isSafeCloseError(e))
-        return;
-      throw e;
-    }
+        await artifact.delete();
+      }
+    }, true);
+    await this._channel.close();
+    await this._closedPromise;
   }
 
   async _enableRecorder(params: {

packages/playwright-core/src/client/page.ts

@@ -532,7 +532,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
       else
         await this._channel.close(options);
     } catch (e) {
-      if (isSafeCloseError(e))
+      if (isSafeCloseError(e) && !options.runBeforeUnload)
         return;
       throw e;
     }

tests/library/browsertype-connect.spec.ts

@@ -426,29 +426,6 @@ for (const kind of ['launchServer', 'run-server'] as const) {
       await browser.close();
     });
 
-    test('should not throw on context.close after disconnect', async ({ connect, startRemoteServer }) => {
-      const remoteServer = await startRemoteServer(kind);
-      const browser = await connect(remoteServer.wsEndpoint());
-      const context = await browser.newContext();
-      await context.newPage();
-      await Promise.all([
-        new Promise(f => browser.on('disconnected', f)),
-        remoteServer.close()
-      ]);
-      await context.close();
-    });
-
-    test('should not throw on page.close after disconnect', async ({ connect, startRemoteServer }) => {
-      const remoteServer = await startRemoteServer(kind);
-      const browser = await connect(remoteServer.wsEndpoint());
-      const page = await browser.newPage();
-      await Promise.all([
-        new Promise(f => browser.on('disconnected', f)),
-        remoteServer.close()
-      ]);
-      await page.close();
-    });
-
     test('should saveAs videos from remote browser', async ({ connect, startRemoteServer }, testInfo) => {
       const remoteServer = await startRemoteServer(kind);
       const browser = await connect(remoteServer.wsEndpoint());