fix: implement throwIfDetached (#10826)

docs/api/puppeteer.frame.addstyletag.md

@@ -4,7 +4,7 @@ sidebar_label: Frame.addStyleTag
 
 # Frame.addStyleTag() method
 
-Adds a `<link rel="stylesheet">` tag into the page with the desired URL or a `<style type="text/css">` tag with the content.
+Adds a `HTMLStyleElement` into the frame with the desired URL
 
 #### Signature:
 
@@ -26,4 +26,4 @@ class Frame {
 
 Promise&lt;[ElementHandle](./puppeteer.elementhandle.md)&lt;HTMLStyleElement&gt;&gt;
 
-An [element handle](./puppeteer.elementhandle.md) to the loaded `<link>` or `<style>` element.
+An [element handle](./puppeteer.elementhandle.md) to the loaded `<style>` element.

docs/api/puppeteer.frame.addstyletag_1.md

@@ -4,6 +4,8 @@ sidebar_label: Frame.addStyleTag_1
 
 # Frame.addStyleTag() method
 
+Adds a `HTMLLinkElement` into the frame with the desired URL
+
 #### Signature:
 
 ```typescript
@@ -23,3 +25,5 @@ class Frame {
 **Returns:**
 
 Promise<[ElementHandle](./puppeteer.elementhandle.md)<HTMLLinkElement>>
+
+An [element handle](./puppeteer.elementhandle.md) to the loaded `<link>` element.

docs/api/puppeteer.frame.isdetached.md

@@ -4,13 +4,17 @@ sidebar_label: Frame.isDetached
 
 # Frame.isDetached() method
 
+> Warning: This API is now obsolete.
+>
+> Use the `detached` getter.
+
 Is`true` if the frame has been detached. Otherwise, `false`.
 
 #### Signature:
 
 ```typescript
 class Frame {
-  abstract isDetached(): boolean;
+  isDetached(): boolean;
 }
 ```
 

docs/api/puppeteer.frame.md

@@ -61,6 +61,12 @@ const text = await frame.$eval('.selector', element => element.textContent);
 console.log(text);
 ```
 
+## Properties
+
+| Property | Modifiers             | Type    | Description |
+| -------- | --------------------- | ------- | ----------- |
+| detached | <code>readonly</code> | boolean |             |
+
 ## Methods
 
 | Method                                                                               | Modifiers | Description                                                                                                                                                                                                                                                                |
@@ -71,8 +77,8 @@ console.log(text);
 | [$eval(selector, pageFunction, args)](./puppeteer.frame._eval.md)                    |           | <p>Runs the given function on the first element matching the given selector in the frame.</p><p>If the given function returns a promise, then this method will wait till the promise resolves.</p>                                                                         |
 | [$x(expression)](./puppeteer.frame._x.md)                                            |           |                                                                                                                                                                                                                                                                            |
 | [addScriptTag(options)](./puppeteer.frame.addscripttag.md)                           |           | Adds a <code>&lt;script&gt;</code> tag into the page with the desired url or content.                                                                                                                                                                                      |
-| [addStyleTag(options)](./puppeteer.frame.addstyletag.md)                             |           | Adds a <code>&lt;link rel=&quot;stylesheet&quot;&gt;</code> tag into the page with the desired URL or a <code>&lt;style type=&quot;text/css&quot;&gt;</code> tag with the content.                                                                                         |
+| [addStyleTag(options)](./puppeteer.frame.addstyletag.md)                             |           | Adds a <code>HTMLStyleElement</code> into the frame with the desired URL                                                                                                                                                                                                   |
-| [addStyleTag(options)](./puppeteer.frame.addstyletag_1.md)                           |           |                                                                                                                                                                                                                                                                            |
+| [addStyleTag(options)](./puppeteer.frame.addstyletag_1.md)                           |           | Adds a <code>HTMLLinkElement</code> into the frame with the desired URL                                                                                                                                                                                                    |
 | [childFrames()](./puppeteer.frame.childframes.md)                                    |           | An array of child frames.                                                                                                                                                                                                                                                  |
 | [click(selector, options)](./puppeteer.frame.click.md)                               |           | Clicks the first element found that matches <code>selector</code>.                                                                                                                                                                                                         |
 | [content()](./puppeteer.frame.content.md)                                            |           | The full HTML contents of the frame, including the DOCTYPE.                                                                                                                                                                                                                |

packages/puppeteer-core/src/api/Frame.ts

@@ -40,6 +40,7 @@ import {
   importFSPromises,
   withSourcePuppeteerURLIfNone,
 } from '../common/util.js';
+import {throwIfDisposed} from '../util/decorators.js';
 
 import {KeyboardTypeOptions} from './Input.js';
 import {FunctionLocator, Locator, NodeLocator} from './locators/locators.js';
@@ -125,6 +126,13 @@ export interface FrameAddStyleTagOptions {
   content?: string;
 }
 
+/**
+ * @internal
+ */
+export const throwIfDetached = throwIfDisposed<Frame>(frame => {
+  return `Attempted to use detached Frame '${frame._id}'.`;
+});
+
 /**
  * Represents a DOM frame.
  *
@@ -323,6 +331,7 @@ export abstract class Frame extends EventEmitter {
   /**
    * @internal
    */
+  @throwIfDetached
   async frameElement(): Promise<HandleFor<HTMLIFrameElement> | null> {
     const parentFrame = this.parentFrame();
     if (!parentFrame) {
@@ -346,6 +355,7 @@ export abstract class Frame extends EventEmitter {
    *
    * @see {@link Page.evaluateHandle} for details.
    */
+  @throwIfDetached
   async evaluateHandle<
     Params extends unknown[],
     Func extends EvaluateFunc<Params> = EvaluateFunc<Params>,
@@ -366,6 +376,7 @@ export abstract class Frame extends EventEmitter {
    *
    * @see {@link Page.evaluate} for details.
    */
+  @throwIfDetached
   async evaluate<
     Params extends unknown[],
     Func extends EvaluateFunc<Params> = EvaluateFunc<Params>,
@@ -401,6 +412,11 @@ export abstract class Frame extends EventEmitter {
    * change in the Locators API.
    */
   locator<Ret>(func: () => Awaitable<Ret>): Locator<Ret>;
+
+  /**
+   * @internal
+   */
+  @throwIfDetached
   locator<Selector extends string, Ret>(
     selectorOrFunc: Selector | (() => Awaitable<Ret>)
   ): Locator<NodeFor<Selector>> | Locator<Ret> {
@@ -417,6 +433,7 @@ export abstract class Frame extends EventEmitter {
    * @returns A {@link ElementHandle | element handle} to the first element
    * matching the given selector. Otherwise, `null`.
    */
+  @throwIfDetached
   async $<Selector extends string>(
     selector: Selector
   ): Promise<ElementHandle<NodeFor<Selector>> | null> {
@@ -430,6 +447,7 @@ export abstract class Frame extends EventEmitter {
    * @returns An array of {@link ElementHandle | element handles} that point to
    * elements matching the given selector.
    */
+  @throwIfDetached
   async $$<Selector extends string>(
     selector: Selector
   ): Promise<Array<ElementHandle<NodeFor<Selector>>>> {
@@ -456,6 +474,7 @@ export abstract class Frame extends EventEmitter {
    * @param args - Additional arguments to pass to `pageFunction`.
    * @returns A promise to the result of the function.
    */
+  @throwIfDetached
   $eval<
     Selector extends string,
     Params extends unknown[],
@@ -492,6 +511,7 @@ export abstract class Frame extends EventEmitter {
    * @param args - Additional arguments to pass to `pageFunction`.
    * @returns A promise to the result of the function.
    */
+  @throwIfDetached
   $$eval<
     Selector extends string,
     Params extends unknown[],
@@ -518,6 +538,7 @@ export abstract class Frame extends EventEmitter {
    * automatically.
    * @param expression - the XPath expression to evaluate.
    */
+  @throwIfDetached
   $x(expression: string): Promise<Array<ElementHandle<Node>>> {
     return this.mainRealm().$x(expression);
   }
@@ -557,6 +578,7 @@ export abstract class Frame extends EventEmitter {
    * @returns An element matching the given selector.
    * @throws Throws if an element matching the given selector doesn't appear.
    */
+  @throwIfDetached
   async waitForSelector<Selector extends string>(
     selector: Selector,
     options: WaitForSelectorOptions = {}
@@ -592,6 +614,7 @@ export abstract class Frame extends EventEmitter {
    * @param options - options to configure the visibility of the element and how
    * long to wait before timing out.
    */
+  @throwIfDetached
   async waitForXPath(
     xpath: string,
     options: WaitForSelectorOptions = {}
@@ -635,6 +658,7 @@ export abstract class Frame extends EventEmitter {
    * @param args - arguments to pass to the `pageFunction`.
    * @returns the promise which resolve when the `pageFunction` returns a truthy value.
    */
+  @throwIfDetached
   waitForFunction<
     Params extends unknown[],
     Func extends EvaluateFunc<Params> = EvaluateFunc<Params>,
@@ -652,6 +676,7 @@ export abstract class Frame extends EventEmitter {
   /**
    * The full HTML contents of the frame, including the DOCTYPE.
    */
+  @throwIfDetached
   content(): Promise<string> {
     return this.isolatedRealm().content();
   }
@@ -701,9 +726,25 @@ export abstract class Frame extends EventEmitter {
   abstract childFrames(): Frame[];
 
   /**
-   * Is`true` if the frame has been detached. Otherwise, `false`.
+   * @returns `true` if the frame has detached. `false` otherwise.
    */
-  abstract isDetached(): boolean;
+  abstract get detached(): boolean;
+
+  /**
+   * Is`true` if the frame has been detached. Otherwise, `false`.
+   *
+   * @deprecated Use the `detached` getter.
+   */
+  isDetached(): boolean {
+    return this.detached;
+  }
+
+  /**
+   * @internal
+   */
+  get disposed(): boolean {
+    return this.detached;
+  }
 
   /**
    * Adds a `<script>` tag into the page with the desired url or content.
@@ -712,6 +753,7 @@ export abstract class Frame extends EventEmitter {
    * @returns An {@link ElementHandle | element handle} to the injected
    * `<script>` element.
    */
+  @throwIfDetached
   async addScriptTag(
     options: FrameAddScriptTagOptions
   ): Promise<ElementHandle<HTMLScriptElement>> {
@@ -775,18 +817,29 @@ export abstract class Frame extends EventEmitter {
   }
 
   /**
-   * Adds a `<link rel="stylesheet">` tag into the page with the desired URL or
+   * Adds a `HTMLStyleElement` into the frame with the desired URL
-   * a `<style type="text/css">` tag with the content.
    *
-   * @returns An {@link ElementHandle | element handle} to the loaded `<link>`
+   * @returns An {@link ElementHandle | element handle} to the loaded `<style>`
-   * or `<style>` element.
+   * element.
    */
   async addStyleTag(
     options: Omit<FrameAddStyleTagOptions, 'url'>
   ): Promise<ElementHandle<HTMLStyleElement>>;
+
+  /**
+   * Adds a `HTMLLinkElement` into the frame with the desired URL
+   *
+   * @returns An {@link ElementHandle | element handle} to the loaded `<link>`
+   * element.
+   */
   async addStyleTag(
     options: FrameAddStyleTagOptions
   ): Promise<ElementHandle<HTMLLinkElement>>;
+
+  /**
+   * @internal
+   */
+  @throwIfDetached
   async addStyleTag(
     options: FrameAddStyleTagOptions
   ): Promise<ElementHandle<HTMLStyleElement | HTMLLinkElement>> {
@@ -868,6 +921,7 @@ export abstract class Frame extends EventEmitter {
    *
    * @param selector - The selector to query for.
    */
+  @throwIfDetached
   click(selector: string, options: Readonly<ClickOptions> = {}): Promise<void> {
     return this.isolatedRealm().click(selector, options);
   }
@@ -878,7 +932,8 @@ export abstract class Frame extends EventEmitter {
    * @param selector - The selector to query for.
    * @throws Throws if there's no element matching `selector`.
    */
-  async focus(selector: string): Promise<void> {
+  @throwIfDetached
+  focus(selector: string): Promise<void> {
     return this.isolatedRealm().focus(selector);
   }
 
@@ -889,6 +944,7 @@ export abstract class Frame extends EventEmitter {
    * @param selector - The selector to query for.
    * @throws Throws if there's no element matching `selector`.
    */
+  @throwIfDetached
   hover(selector: string): Promise<void> {
     return this.isolatedRealm().hover(selector);
   }
@@ -911,6 +967,7 @@ export abstract class Frame extends EventEmitter {
    * @returns the list of values that were successfully selected.
    * @throws Throws if there's no `<select>` matching `selector`.
    */
+  @throwIfDetached
   select(selector: string, ...values: string[]): Promise<string[]> {
     return this.isolatedRealm().select(selector, ...values);
   }
@@ -921,6 +978,7 @@ export abstract class Frame extends EventEmitter {
    * @param selector - The selector to query for.
    * @throws Throws if there's no element matching `selector`.
    */
+  @throwIfDetached
   tap(selector: string): Promise<void> {
     return this.isolatedRealm().tap(selector);
   }
@@ -946,6 +1004,7 @@ export abstract class Frame extends EventEmitter {
    * @param options - takes one option, `delay`, which sets the time to wait
    * between key presses in milliseconds. Defaults to `0`.
    */
+  @throwIfDetached
   type(
     selector: string,
     text: string,
@@ -983,6 +1042,7 @@ export abstract class Frame extends EventEmitter {
   /**
    * The frame's title.
    */
+  @throwIfDetached
   title(): Promise<string> {
     return this.isolatedRealm().title();
   }
@@ -1013,6 +1073,10 @@ export abstract class Frame extends EventEmitter {
   waitForDevicePrompt(
     options?: WaitTimeoutOptions
   ): Promise<DeviceRequestPrompt>;
+
+  /**
+   * @internal
+   */
   waitForDevicePrompt(): Promise<DeviceRequestPrompt> {
     throw new Error('Not implemented');
   }

packages/puppeteer-core/src/common/Frame.ts

@@ -16,8 +16,7 @@
 
 import {Protocol} from 'devtools-protocol';
 
-import {ElementHandle} from '../api/ElementHandle.js';
+import {Frame, throwIfDetached} from '../api/Frame.js';
-import {Frame} from '../api/Frame.js';
 import {HTTPResponse} from '../api/HTTPResponse.js';
 import {Page, WaitTimeoutOptions} from '../api/Page.js';
 import {assert} from '../util/assert.js';
@@ -34,7 +33,6 @@ import {FrameManager} from './FrameManager.js';
 import {IsolatedWorld} from './IsolatedWorld.js';
 import {MAIN_WORLD, PUPPETEER_WORLD} from './IsolatedWorlds.js';
 import {LifecycleWatcher, PuppeteerLifeCycleEvent} from './LifecycleWatcher.js';
-import {NodeFor} from './types.js';
 
 /**
  * We use symbols to prevent external parties listening to these events.
@@ -118,6 +116,7 @@ export class CDPFrame extends Frame {
     return this.#client !== this._frameManager.client;
   }
 
+  @throwIfDetached
   override async goto(
     url: string,
     options: {
@@ -201,6 +200,7 @@ export class CDPFrame extends Frame {
     }
   }
 
+  @throwIfDetached
   override async waitForNavigation(
     options: {
       timeout?: number;
@@ -254,18 +254,7 @@ export class CDPFrame extends Frame {
     return this.worlds[PUPPETEER_WORLD];
   }
 
-  override async $<Selector extends string>(
+  @throwIfDetached
-    selector: Selector
-  ): Promise<ElementHandle<NodeFor<Selector>> | null> {
-    return this.mainRealm().$(selector);
-  }
-
-  override async $$<Selector extends string>(
-    selector: Selector
-  ): Promise<Array<ElementHandle<NodeFor<Selector>>>> {
-    return this.mainRealm().$$(selector);
-  }
-
   override async setContent(
     html: string,
     options: {
@@ -288,10 +277,6 @@ export class CDPFrame extends Frame {
     return this._frameManager._frameTree.childFrames(this._id);
   }
 
-  override isDetached(): boolean {
-    return this.#detached;
-  }
-
   #deviceRequestPromptManager(): DeviceRequestPromptManager {
     if (this.isOOPFrame()) {
       return this._frameManager._deviceRequestPromptManager(this.#client);
@@ -301,6 +286,7 @@ export class CDPFrame extends Frame {
     return parentFrame.#deviceRequestPromptManager();
   }
 
+  @throwIfDetached
   override waitForDevicePrompt(
     options: WaitTimeoutOptions = {}
   ): Promise<DeviceRequestPrompt> {
@@ -333,7 +319,14 @@ export class CDPFrame extends Frame {
     this._hasStartedLoading = true;
   }
 
+  override get detached(): boolean {
+    return this.#detached;
+  }
+
   [Symbol.dispose](): void {
+    if (this.#detached) {
+      return;
+    }
     this.#detached = true;
     this.worlds[MAIN_WORLD][Symbol.dispose]();
     this.worlds[PUPPETEER_WORLD][Symbol.dispose]();

packages/puppeteer-core/src/common/bidi/Frame.ts

@@ -16,7 +16,7 @@
 
 import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
 
-import {Frame} from '../../api/Frame.js';
+import {Frame, throwIfDetached} from '../../api/Frame.js';
 import {Deferred} from '../../util/Deferred.js';
 import {CDPSession} from '../Connection.js';
 import {UTILITY_WORLD_NAME} from '../FrameManager.js';
@@ -47,7 +47,7 @@ export class BidiFrame extends Frame {
   #context: BrowsingContext;
   #timeoutSettings: TimeoutSettings;
   #abortDeferred = Deferred.create<Error>();
-  #detached = false;
+  #disposed = false;
   sandboxes: SandboxChart;
   override _id: string;
 
@@ -102,6 +102,7 @@ export class BidiFrame extends Frame {
     return this.#page.childFrames(this.#context.id);
   }
 
+  @throwIfDetached
   override async goto(
     url: string,
     options?: {
@@ -118,6 +119,7 @@ export class BidiFrame extends Frame {
     return this.#page.getNavigationResponse(navigationId);
   }
 
+  @throwIfDetached
   override setContent(
     html: string,
     options: {
@@ -135,6 +137,7 @@ export class BidiFrame extends Frame {
     return this.#context;
   }
 
+  @throwIfDetached
   override async waitForNavigation(
     options: {
       timeout?: number;
@@ -189,12 +192,15 @@ export class BidiFrame extends Frame {
     return this.#page.getNavigationResponse(info.navigation);
   }
 
-  override isDetached(): boolean {
+  override get detached(): boolean {
-    return this.#detached;
+    return this.#disposed;
   }
 
   [Symbol.dispose](): void {
-    this.#detached = true;
+    if (this.#disposed) {
+      return;
+    }
+    this.#disposed = true;
     this.#abortDeferred.reject(new Error('Frame detached'));
     this.#context.dispose();
     this.sandboxes[MAIN_SANDBOX][Symbol.dispose]();

packages/puppeteer-core/src/util/decorators.ts

@@ -58,16 +58,15 @@ export function moveable<
   return Class;
 }
 
-export function throwIfDisposed(message?: string) {
+export function throwIfDisposed<This extends Disposed>(
-  return <This extends Disposed, Args extends unknown[], Ret>(
+  message: (value: This) => string = value => {
-    target: (this: This, ...args: Args) => Ret,
+    return `Attempted to use disposed ${value.constructor.name}.`;
-    _: unknown
+  }
-  ) => {
+) {
-    return function (this: This, ...args: Args): Ret {
+  return (target: (this: This, ...args: any[]) => any, _: unknown) => {
+    return function (this: This, ...args: any[]): any {
       if (this.disposed) {
-        throw new Error(
+        throw new Error(message(this));
-          message ?? `Attempted to use disposed ${this.constructor.name}.`
-        );
       }
       return target.call(this, ...args);
     };