[Feature]: Add debug logging to `_takePageSnapshot` catch block

[faisalaffan]

🚀 Feature Request

Related PR: #41502 (closed — ready to reopen once this issue is accepted)

Replace the empty catch {} in ArtifactsRecorder._takePageSnapshot with a debugLogger.log call so that AI snapshot failures during error context capture are observable via DEBUG=pw:api.

Location: packages/playwright/src/index.ts:747

- } catch {}
+ } catch {
+   debugLogger.log('api', 'ariaSnapshot: failed to capture snapshot during error (may be expected)');
+ }

Example

In a large test suite (500+ tests, AI mode enabled), some failing tests produce error-context.md without an aria snapshot. Today there is no way to tell whether the snapshot was never attempted or failed silently.

After this change, running with DEBUG=pw:api would show:

pw:api ariaSnapshot: failed to capture snapshot during error (may be expected) +0ms

No change to user-facing behavior — the original test error still propagates. The debug log is purely for troubleshooting.

Motivation

_takePageSnapshot is best-effort artifact collection. When it fails, the failure should be observable. Currently the empty catch {} makes it impossible to distinguish "snapshot was never attempted" from "snapshot failed due to timeout/network/model error." A single debugLogger.log call provides this visibility without changing control flow or adding noise for regular users.

[github-actions]

Hi, I'm the Playwright bot and I took a first look at this.

This is a valid, small request. The code is exactly as described: ArtifactsRecorder._takePageSnapshot in packages/playwright/src/index.ts:747 ends in an empty } catch {}, so a failed best-effort AI snapshot during error-context capture is swallowed with no trace. Logging it under pw:api is consistent with how the rest of the codebase reports best-effort failures (debugLogger.log('api', …) is already used in client/channelOwner.ts, client/harRouter.ts, and server/instrumentation.ts), and @utils/** is already allowed by packages/playwright/src/DEPS.list, so the import is in bounds. It's purely diagnostic — the original test error still propagates and regular users see nothing new.

One thing worth deciding: there are two call sites for _takePageSnapshot (willCloseBrowserContext and didFinishTest), and the ariaSnapshot({ mode: 'ai', timeout: 5000 }) call can fail for ordinary reasons (5s timeout, page already closing during teardown). So the log line should read as expected-and-harmless rather than alarming — the wording in the proposal ("may be expected") does that well. Including the caught error (debugLogger.log('api', \ariaSnapshot: … ${error}`)`) would make it more useful than a static string.

What I checked

• packages/playwright/src/index.ts:729-748 — _takePageSnapshot, confirmed the empty catch {} at line 747.
• debugLogger lives at packages/utils/debugLogger.ts, imported elsewhere as import { debugLogger } from '@utils/debugLogger'.
• packages/playwright/src/DEPS.list allows @utils/**; the file already imports @utils/debug, so adding @utils/debugLogger is fine.
• Existing debugLogger.log('api', …) precedents in client/channelOwner.ts:227, client/harRouter.ts:60, server/instrumentation.ts:64.
• Related PR fix: improve error handling for ariaSnapshot and fixture integrity #41502 was closed by a maintainer with "Please start with filing an issue" — which is this issue.

_{— triage run}