fix(client): report a failed BrowserContext.close() instead of resolving

A close() that rejected left _closingStatus === 'closing' behind, so
isClosed() reported true for a context that was still live, and every
later close() returned early through the entry guard without doing
anything. Callers could neither detect the leftover context nor close
it: the context stayed alive until the browser exited.

Remember the failure, restore the status so isClosed() stays truthful,
and rethrow it on subsequent calls.

Author: skvortsov-dev

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

@@ -82,6 +82,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
 
   readonly _serviceWorkers = new Set<Worker>();
   private _closingStatus: 'none' | 'closing' | 'closed' = 'none';
+  private _closeFailure: Error | undefined;
   private _closeReason: string | undefined;
   private _harRouters: HarRouter[] = [];
   private _onRecorderEventSink: RecorderEventSink | undefined;
@@ -521,15 +522,25 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
   }
 
   async close(options: { reason?: string } = {}): Promise<void> {
-    if (this.isClosed())
+    if (this._closingStatus === 'closed')
       return;
+    if (this._closeFailure)
+      throw this._closeFailure;
     this._closeReason = options.reason;
     this._closingStatus = 'closing';
-    await this.request.dispose(options);
-    await this._instrumentation.runBeforeCloseBrowserContext(this);
-    await this.tracing._exportAllHars();
-    await this._channel.close(options, kNoTimeout);
-    await this._closedPromise;
+    try {
+      await this.request.dispose(options);
+      await this._instrumentation.runBeforeCloseBrowserContext(this);
+      await this.tracing._exportAllHars();
+      await this._channel.close(options, kNoTimeout);
+      await this._closedPromise;
+    } catch (error) {
+      this._closeFailure = error;
+      // The context did not close, so isClosed() must not claim otherwise.
+      if (this._closingStatus === 'closing')
+        this._closingStatus = 'none';
+      throw error;
+    }
   }
 
   async _enableRecorder(params: channels.BrowserContextEnableRecorderParams, eventSink?: RecorderEventSink) {

tests/library/browsercontext-basic.spec.ts

@@ -17,6 +17,7 @@
 
 import { kTargetClosedErrorMessage } from '../config/errors';
 import { browserTest as it, expect } from '../config/browserTest';
+import os from 'os';
 import { attachFrame, verifyViewport } from '../config/utils';
 import type { Page } from '@playwright/test';
 
@@ -212,6 +213,20 @@ it('close() should be callable twice', async ({ browser }) => {
   await context.close();
 });
 
+it('close() should report a failure instead of silently succeeding', async ({ browserType }) => {
+  const browser = await browserType.launch();
+  // Writing the HAR into a directory fails, which makes close() reject.
+  const context = await browser.newContext({ recordHar: { path: os.tmpdir() } });
+  await context.newPage();
+  const firstError = await context.close().catch(e => e);
+  expect(firstError).toBeInstanceOf(Error);
+  // The context is still alive, so it must not report itself as closed.
+  expect(context.isClosed()).toBe(false);
+  const secondError = await context.close().catch(e => e);
+  expect(secondError).toBeInstanceOf(Error);
+  await browser.close();
+});
+
 it('should pass self to close event', async ({ browser }) => {
   const newContext = await browser.newContext();
   const [closedContext] = await Promise.all([