ronantakizawa/codereview-bench huggingface dataset:
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/util/disposable.ts (TypeScript)

Code before:
        }
      }
      throw suppressed;
    }
  }

  readonly [Symbol.toStringTag] = 'AsyncDisposableStack';
}

/**
 * Represents an error that occurs when multiple errors are thrown during
 * the disposal of resources. This class encapsulates the primary error and
 * any suppressed errors that occurred subsequently.
 */
export class SuppressedError extends Error {
  #error: unknown;
  #suppressed: unknown;

  constructor(
    error: unknown,
    suppressed: unknown,
    message = 'An error was suppressed during disposal',
  ) {
    super(message);
    this.name = 'SuppressedError';
    this.#error = error;
    this.#suppressed = suppressed;
    // Restore the prototype chain
    Object.setPrototypeOf(this, new.target.prototype);
  }

  /**
   * The primary error that occurred during disposal.
   */
  get error(): unknown {
    return this.#error;
  }

  /**
   * The suppressed error i.e. the error that was suppressed
   * because it occurred later in the flow after the original error.
   */
  get suppressed(): unknown {
    return this.#suppressed;
  }
}


Review comment (suggestion):
Needs to be marked as internal in the comment, so it does not show up in the docs.
```suggestion
 * any suppressed errors that occurred subsequently.
 * @internal
```
Remember to run `npm run docs` after that.

Code after:
        errors.push(e);
      }
    }
    if (errors.length === 1) {
      throw errors[0];
    } else if (errors.length > 1) {
      let suppressed = null;
      for (const error of errors.reverse()) {
        if (suppressed === null) {
          suppressed = error;
        } else {
          suppressed = new SuppressedError(error, suppressed);
        }
      }
      throw suppressed;
    }
  }

  readonly [Symbol.toStringTag] = 'AsyncDisposableStack';
}

/**
 * @internal
 * Represents an error that occurs when multiple errors are thrown during
 * the disposal of resources. This class encapsulates the primary error and
 * any suppressed errors that occurred subsequently.
 */
export class SuppressedError extends Error {
  #error: unknown;
  #suppressed: unknown;

  constructor(
    error: unknown,
    suppressed: unknown,
    message = 'An error was suppressed during disposal',
  ) {
    super(message);
    this.name = 'SuppressedError';
    this.#error = error;
    this.#suppressed = suppressed;
  }

  /**
   * The primary error that occurred during disposal.
   */
  get error(): unknown {
    return this.#error;
  }

  /**
   * The suppressed error i.e. the error that was suppressed
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/api/Page.ts (TypeScript)

Code before:
   * @defaultValue `false`
   */
  fullPage?: boolean;
  /**
   * Hides default white background and allows capturing screenshots with transparency.
   *
   * @defaultValue `false`
   */
  omitBackground?: boolean;
  /**
   * The file path to save the image to. The screenshot type will be inferred
   * from file extension. If path is a relative path, then it is resolved
   * relative to current working directory. If no path is provided, the image
   * won't be saved to the disk.
   */
  path?: string;
  /**
   * Specifies the region of the page/element to clip.
   */
  clip?: ScreenshotClip;
  /**
   * Encoding of the image.
   *
   * @defaultValue `'binary'`
   */
  encoding?: 'base64' | 'binary';
  /**
   * Capture the screenshot beyond the viewport.
   *
   * @defaultValue `false` if there is no `clip`. `true` otherwise.
   */
  captureBeyondViewport?: boolean;
}

/**
 * @public
 */
export type FileFormat = 'gif' | 'webm' | 'mp4';

/**
 * @public
 * @experimental
 */
export interface ScreencastOptions {
  /**
   * File path to save the screencast to.
   */
  path?: `${string}.${FileFormat}`;
  /**
   * Specifies the output file format.
   *

Review comment (suggestion):
This makes it unnecessarily annoying to use a computed value for `path`. For example, ``await page.screenshot({ path: path.join(dir, `${name}.png`) })`` leads to error TS2769: ``Type 'string' is not assignable to type '`${string}.webp` | `${string}.jpeg` | `${string}.png`'.``

Code after:
  quality?: number;
  /**
   * Capture the screenshot from the surface, rather than the view.
   *
   * @defaultValue `true`
   */
  fromSurface?: boolean;
  /**
   * When `true`, takes a screenshot of the full page.
   *
   * @defaultValue `false`
   */
  fullPage?: boolean;
  /**
   * Hides default white background and allows capturing screenshots with transparency.
   *
   * @defaultValue `false`
   */
  omitBackground?: boolean;
  /**
   * The file path to save the image to. The screenshot type will be inferred
   * from file extension. If path is a relative path, then it is resolved
   * relative to current working directory. If no path is provided, the image
   * won't be saved to the disk.
   */
  path?: `${string}.${ImageFormat}`;
  /**
   * Specifies the region of the page/element to clip.
   */
  clip?: ScreenshotClip;
  /**
   * Encoding of the image.
   *
   * @defaultValue `'binary'`
   */
  encoding?: 'base64' | 'binary';
  /**
   * Capture the screenshot beyond the viewport.
   *
   * @defaultValue `false` if there is no `clip`. `true` otherwise.
   */
  captureBeyondViewport?: boolean;
}

/**
 * @public
 * @experimental
 */
export interface ScreencastOptions {
  /**
   * File path to save the screencast to.
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/bidi/core/BrowsingContext.ts (TypeScript)

Code before:
    // SAFETY: Disposal implies this exists.
    return context.#reason!;
  })
  async setTimezoneOverride(timezoneId?: string): Promise<void> {
    if (timezoneId?.startsWith('GMT')) {
      // CDP requires `GMT` prefix before timezone offset, while BiDi does not. Remove the
      // `GMT` for interop between CDP and BiDi.
      timezoneId = timezoneId?.replace('GMT', '');
    }
    await this.userContext.browser.session.send(
      'emulation.setTimezoneOverride',
      {
        timezone: timezoneId ?? null,
        contexts: [this.id],
      },
    );
  }

  @throwIfDisposed<BrowsingContext>(context => {
    // SAFETY: Disposal implies this exists.
    return context.#reason!;
  })
  async setScreenOrientationOverride(
    screenOrientation: Bidi.Emulation.ScreenOrientation | null,
  ): Promise<void> {
    await this.userContext.browser.session.send(
      'emulation.setScreenOrientationOverride',
      {
        screenOrientation,
        contexts: [this.id],
      },
    );
  }

  @throwIfDisposed<BrowsingContext>(context => {
    // SAFETY: Disposal implies this exists.
    return context.#reason!;
  })
  async getCookies(
    options: GetCookiesOptions = {},
  ): Promise<Bidi.Network.Cookie[]> {
    const {
      result: {cookies},
    } = await this.#session.send('storage.getCookies', {
      ...options,
      partition: {
        type: 'context',
        context: this.id,
      },
    });
    return cookies;

Review comment (refactor):
Nit: lets use the helper getter.
```suggestion
    await this.#session.send(
```

Code after:
    // SAFETY: Disposal implies this exists.
    return context.#reason!;
  })
  async setTimezoneOverride(timezoneId?: string): Promise<void> {
    if (timezoneId?.startsWith('GMT')) {
      // CDP requires `GMT` prefix before timezone offset, while BiDi does not. Remove the
      // `GMT` for interop between CDP and BiDi.
      timezoneId = timezoneId?.replace('GMT', '');
    }
    await this.userContext.browser.session.send(
      'emulation.setTimezoneOverride',
      {
        timezone: timezoneId ?? null,
        contexts: [this.id],
      },
    );
  }

  @throwIfDisposed<BrowsingContext>(context => {
    // SAFETY: Disposal implies this exists.
    return context.#reason!;
  })
  async setScreenOrientationOverride(
    screenOrientation: Bidi.Emulation.ScreenOrientation | null,
  ): Promise<void> {
    await this.#session.send('emulation.setScreenOrientationOverride', {
      screenOrientation,
      contexts: [this.id],
    });
  }

  @throwIfDisposed<BrowsingContext>(context => {
    // SAFETY: Disposal implies this exists.
    return context.#reason!;
  })
  async getCookies(
    options: GetCookiesOptions = {},
  ): Promise<Bidi.Network.Cookie[]> {
    const {
      result: {cookies},
    } = await this.#session.send('storage.getCookies', {
      ...options,
      partition: {
        type: 'context',
        context: this.id,
      },
    });
    return cookies;
  }

  @throwIfDisposed<BrowsingContext>(context => {
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/cdp/Accessibility.ts (TypeScript)

Code before:
      properties.set('description', this.payload.description.value);
    }

    const node: SerializedAXNode = {
      role: this.#role,
      elementHandle: async (): Promise<ElementHandle | null> => {
        if (!this.payload.backendDOMNodeId) {
          return null;
        }
        const handle = (await this.#realm.adoptBackendNode(
          this.payload.backendDOMNodeId,
        )) as ElementHandle<Element>;

        if (!handle) {
          return null;
        }

        // Text nodes are not elements, so we can't return a handle to them.
        // Instead, we find the parent element and return a handle to the parent.
        if (await handle.evaluate(node => {
          return node.nodeType === Node.TEXT_NODE;
        })) {
          using parent = await handle.evaluateHandle(node => {
            return node.parentElement;
          });
          await handle.dispose();

          return parent.move() as ElementHandle<Element>;
        }

        return handle;
      },
    };

    type UserStringProperty =
      | 'name'
      | 'value'
      | 'description'
      | 'keyshortcuts'
      | 'roledescription'
      | 'valuetext';

    const userStringProperties: UserStringProperty[] = [
      'name',
      'value',
      'description',
      'keyshortcuts',
      'roledescription',
      'valuetext',
    ];
    const getUserStringPropertyValue = (key: UserStringProperty): string => {

Review comment (bug):
This can throw so we should do something like we do in other places.
```
void handle.dispose().catch(debugError);
```

Code after:
      properties.set('description', this.payload.description.value);
    }

    const node: SerializedAXNode = {
      role: this.#role,
      elementHandle: async (): Promise<ElementHandle | null> => {
        if (!this.payload.backendDOMNodeId) {
          return null;
        }
        using handle = await this.#realm.adoptBackendNode(
          this.payload.backendDOMNodeId,
        );

        // Since Text nodes are not elements, we want to
        // return a handle to the parent element for them.
        return (await handle.evaluateHandle(node => {
          return node.nodeType === Node.TEXT_NODE ? node.parentElement : node;
        })) as ElementHandle<Element>;
      },
    };

    type UserStringProperty =
      | 'name'
      | 'value'
      | 'description'
      | 'keyshortcuts'
      | 'roledescription'
      | 'valuetext';

    const userStringProperties: UserStringProperty[] = [
      'name',
      'value',
      'description',
      'keyshortcuts',
      'roledescription',
      'valuetext',
    ];
    const getUserStringPropertyValue = (key: UserStringProperty): string => {
      return properties.get(key) as string;
    };

    for (const userStringProperty of userStringProperties) {
      if (!properties.has(userStringProperty)) {
        continue;
      }

      node[userStringProperty] = getUserStringPropertyValue(userStringProperty);
    }

    type BooleanProperty =
      | 'disabled'
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/cdp/Accessibility.ts (TypeScript)

Code before:
    }

    // Roles whose children are only presentational according to the ARIA and
    // HTML5 Specs should be hidden from screen readers.
    // (Note that whilst ARIA buttons can have only presentational children, HTML5
    // buttons are allowed to have content.)
    switch (this.#role) {
      case 'doc-cover':
      case 'graphics-symbol':
      case 'img':
      case 'image':
      case 'Meter':
      case 'scrollbar':
      case 'slider':
      case 'separator':
      case 'progressbar':
        return true;
      default:
        break;
    }

    // Here and below: Android heuristics
    if (this.#hasFocusableChild()) {
      return false;
    }
    if (this.#focusable && this.#name && this.#name !== 'Document') {
      return true;
    }
    if (this.#role === 'heading' && this.#name) {
      return true;
    }
    return false;
  }

  public isControl(): boolean {
    switch (this.#role) {
      case 'button':
      case 'checkbox':
      case 'ColorWell':
      case 'combobox':
      case 'DisclosureTriangle':
      case 'listbox':
      case 'menu':
      case 'menubar':
      case 'menuitem':
      case 'menuitemcheckbox':
      case 'menuitemradio':
      case 'radio':
      case 'scrollbar':
      case 'searchbox':
      case 'slider':

Review comment (suggestion):
@Lightning00Blade I think we might have needed to keep `this.#focusable`  check as it might not be related to name checks

Code after:
      case 'separator':
      case 'progressbar':
        return true;
      default:
        break;
    }

    if (this.#role === 'heading' && this.#name) {
      return true;
    }

    return false;
  }

  public isControl(): boolean {
    switch (this.#role) {
      case 'button':
      case 'checkbox':
      case 'ColorWell':
      case 'combobox':
      case 'DisclosureTriangle':
      case 'listbox':
      case 'menu':
      case 'menubar':
      case 'menuitem':
      case 'menuitemcheckbox':
      case 'menuitemradio':
      case 'radio':
      case 'scrollbar':
      case 'searchbox':
      case 'slider':
      case 'spinbutton':
      case 'switch':
      case 'tab':
      case 'textbox':
      case 'tree':
      case 'treeitem':
        return true;
      default:
        return false;
    }
  }

  public isInteresting(insideControl: boolean): boolean {
    const role = this.#role;
    if (role === 'Ignored' || this.#hidden || this.#ignored) {
      return false;
    }

    if (this.#focusable || this.#richlyEditable) {
      return true;
Repository: puppeteer/puppeteer
File: packages/browsers/src/browser-data/browser-data.ts (TypeScript)

Code before:
      return tag;
    case Browser.CHROMIUM:
      return tag;
  }
}

/**
 * @public
 */
export async function createProfile(
  browser: Browser,
  opts: ProfileOptions,
): Promise<void> {
  switch (browser) {
    case Browser.FIREFOX:
      return await firefox.createProfile(opts);
    case Browser.CHROME:
    case Browser.CHROMIUM:
      throw new Error(`Profile creation is not support for ${browser} yet`);
  }
}

/**
 * @public
 *
 * Get's the most first resolved system path
 */
export function resolveSystemExecutablePath(
  browser: Browser,
  platform: BrowserPlatform,
  channel: ChromeReleaseChannel,
): string {
  switch (browser) {
    case Browser.CHROMEDRIVER:
    case Browser.CHROMEHEADLESSSHELL:
    case Browser.FIREFOX:
    case Browser.CHROMIUM:
      throw new Error(
        `System browser detection is not supported for ${browser} yet.`,
      );
    case Browser.CHROME:
      return chrome.resolveSystemExecutablePaths(platform, channel)[0];
  }
}

/**
 * @internal
 *
 * Tries to find multiple paths that the executable may be.
 * Return them priority based on heuristics.
 */

Review comment (suggestion):
```suggestion
 * Get's the first resolved system path
```

Code after:
      return tag;
    case Browser.CHROMIUM:
      return tag;
  }
}

/**
 * @public
 */
export async function createProfile(
  browser: Browser,
  opts: ProfileOptions,
): Promise<void> {
  switch (browser) {
    case Browser.FIREFOX:
      return await firefox.createProfile(opts);
    case Browser.CHROME:
    case Browser.CHROMIUM:
      throw new Error(`Profile creation is not support for ${browser} yet`);
  }
}

/**
 * @public
 *
 * Get's the first resolved system path
 */
export function resolveSystemExecutablePath(
  browser: Browser,
  platform: BrowserPlatform,
  channel: ChromeReleaseChannel,
): string {
  switch (browser) {
    case Browser.CHROMEDRIVER:
    case Browser.CHROMEHEADLESSSHELL:
    case Browser.FIREFOX:
    case Browser.CHROMIUM:
      throw new Error(
        `System browser detection is not supported for ${browser} yet.`,
      );
    case Browser.CHROME:
      return chrome.resolveSystemExecutablePaths(platform, channel)[0];
  }
}

/**
 * @internal
 *
 * Returns multiple paths where the executable may be located at on the current system
 * ordered by likelihood (based on heuristics).
 */
Repository: puppeteer/puppeteer
File: packages/browsers/src/browser-data/browser-data.ts (TypeScript)

Code before:
 *
 * Get's the most first resolved system path
 */
export function resolveSystemExecutablePath(
  browser: Browser,
  platform: BrowserPlatform,
  channel: ChromeReleaseChannel,
): string {
  switch (browser) {
    case Browser.CHROMEDRIVER:
    case Browser.CHROMEHEADLESSSHELL:
    case Browser.FIREFOX:
    case Browser.CHROMIUM:
      throw new Error(
        `System browser detection is not supported for ${browser} yet.`,
      );
    case Browser.CHROME:
      return chrome.resolveSystemExecutablePaths(platform, channel)[0];
  }
}

/**
 * @internal
 *
 * Tries to find multiple paths that the executable may be.
 * Return them priority based on heuristics.
 */
export function resolveSystemExecutablePaths(
  browser: Browser,
  platform: BrowserPlatform,
  channel: ChromeReleaseChannel,
): [string, ...string[]] {
  switch (browser) {
    case Browser.CHROMEDRIVER:
    case Browser.CHROMEHEADLESSSHELL:
    case Browser.FIREFOX:
    case Browser.CHROMIUM:
      throw new Error(
        `System browser detection is not supported for ${browser} yet.`,
      );
    case Browser.CHROME:
      return chrome.resolveSystemExecutablePaths(platform, channel);
  }
}

/**
 * Returns a version comparator for the given browser that can be used to sort
 * browser versions.
 *
 * @public
 */

Review comment (suggestion):
```suggestion
 * Returns multiple paths where the executable may be located at on the current system ordered by likelihood (based on heuristics).
```

Code after:
 *
 * Get's the first resolved system path
 */
export function resolveSystemExecutablePath(
  browser: Browser,
  platform: BrowserPlatform,
  channel: ChromeReleaseChannel,
): string {
  switch (browser) {
    case Browser.CHROMEDRIVER:
    case Browser.CHROMEHEADLESSSHELL:
    case Browser.FIREFOX:
    case Browser.CHROMIUM:
      throw new Error(
        `System browser detection is not supported for ${browser} yet.`,
      );
    case Browser.CHROME:
      return chrome.resolveSystemExecutablePaths(platform, channel)[0];
  }
}

/**
 * @internal
 *
 * Returns multiple paths where the executable may be located at on the current system
 * ordered by likelihood (based on heuristics).
 */
export function resolveSystemExecutablePaths(
  browser: Browser,
  platform: BrowserPlatform,
  channel: ChromeReleaseChannel,
): [string, ...string[]] {
  switch (browser) {
    case Browser.CHROMEDRIVER:
    case Browser.CHROMEHEADLESSSHELL:
    case Browser.FIREFOX:
    case Browser.CHROMIUM:
      throw new Error(
        `System browser detection is not supported for ${browser} yet.`,
      );
    case Browser.CHROME:
      return chrome.resolveSystemExecutablePaths(platform, channel);
  }
}

/**
 * Returns a version comparator for the given browser that can be used to sort
 * browser versions.
 *
 * @public
 */
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/bidi/core/Request.ts (TypeScript)

Code before:
      redirect = redirect.#redirect;
    }
    return redirect;
  }
  get response(): Bidi.Network.ResponseData | undefined {
    return this.#response;
  }
  get url(): string {
    return this.#event.request.url;
  }
  get isBlocked(): boolean {
    return this.#event.isBlocked;
  }

  get resourceType(): string | undefined {
    // @ts-expect-error non-standard attribute.
    return this.#event.request['goog:resourceType'] ?? undefined;
  }

  get postData(): string | undefined {
    // @ts-expect-error non-standard attribute.
    return this.#event.request['goog:postData'] ?? undefined;
  }

  get hasPostData(): boolean {
    // @ts-expect-error non-standard attribute.
    return this.#event.request.bodySize > 0;
  }

  async continueRequest({
    url,
    method,
    headers,
    cookies,
    body,
  }: Omit<Bidi.Network.ContinueRequestParameters, 'request'>): Promise<void> {
    await this.#session.send('network.continueRequest', {
      request: this.id,
      url,
      method,
      headers,
      body,
      cookies,
    });
  }

  async failRequest(): Promise<void> {
    await this.#session.send('network.failRequest', {
      request: this.id,
    });
  }

Review comment (refactor):
Can we remove this `@ts-expect-error` or at least update the comment on why we need it.

Code after:
      redirect = redirect.#redirect;
    }
    return redirect;
  }
  get response(): Bidi.Network.ResponseData | undefined {
    return this.#response;
  }
  get url(): string {
    return this.#event.request.url;
  }
  get isBlocked(): boolean {
    return this.#event.isBlocked;
  }

  get resourceType(): string | undefined {
    // @ts-expect-error non-standard attribute.
    return this.#event.request['goog:resourceType'] ?? undefined;
  }

  get postData(): string | undefined {
    // @ts-expect-error non-standard attribute.
    return this.#event.request['goog:postData'] ?? undefined;
  }

  get hasPostData(): boolean {
    return (this.#event.request.bodySize ?? 0) > 0;
  }

  async continueRequest({
    url,
    method,
    headers,
    cookies,
    body,
  }: Omit<Bidi.Network.ContinueRequestParameters, 'request'>): Promise<void> {
    await this.#session.send('network.continueRequest', {
      request: this.id,
      url,
      method,
      headers,
      body,
      cookies,
    });
  }

  async failRequest(): Promise<void> {
    await this.#session.send('network.failRequest', {
      request: this.id,
    });
  }