[rishigupta1599]

Why

PER-8903: when DOM capture/serialization threw, snapshot() logged at debug level and still posted a snapshot with a null domSnapshot. The test author saw an apparently successful snapshot (fail-open, CWE-754).

What

• On WebDriverException/Exception during DOM capture, log at error level and return null (same as other failure paths) without calling postSnapshot. Public API unchanged.
• Updated the two existing tests that encoded the old behaviour to assert null is returned and no /percy/snapshot request is made. They fail without the fix and pass with it.
• Behaviour note: callers that got a non-null result after a failed capture now get null.

Related PRs

Open PRs #319 (PPLT-6242) and #312 touch DOM capture code. This diff only changes the two catch blocks in snapshot(), so any conflict should be trivial.

Verification

• mvn test -Dtest=PercyDriverPathTest,PercyLogicTest in maven:3.9-eclipse-temurin-11: 65 tests pass. The two updated tests fail on master without the fix.
• The full suite (SdkTest, Firefox) and the JaCoCo gate are left to CI.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • DOM-capture failures now produce clearer error logs, and snapshots return no result when capture fails.
  • Snapshot requests are no longer sent when DOM capture fails.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Central YAML (base), Organization UI (inherited), Workspace UI (inherited)
• Review profile: ASSERTIVE
• Plan: Enterprise
• Run ID: 8029d417-d847-421f-b873-e5d02f03a333

Commits

Reviewing files that changed from the base of the PR and between 19b6083 and cdaa5d4.

Files selected for processing: 2

• src/main/java/io/percy/selenium/Percy.java
• src/test/java/io/percy/selenium/PercyDriverPathTest.java

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

Recent review details

Context from checks skipped due to timeout: 5

• GitHub Check: semgrep/ci
• GitHub Check: Test (ubuntu-latest, 17)
• GitHub Check: Test (ubuntu-latest, 21)
• GitHub Check: Test (ubuntu-latest, 11)
• GitHub Check: Test (ubuntu-latest, 8)

Walkthrough

When DOM capture fails, Percy.snapshot now logs the failure at error level and returns null. Tests verify that no snapshot request is made for WebDriver or generic exceptions.

Changes: DOM capture failure handling

Layer / File(s)	Summary
Handle DOM capture failures src/main/java/io/percy/selenium/Percy.java, src/test/java/io/percy/selenium/PercyDriverPathTest.java	Percy.snapshot logs both WebDriver and other exceptions at error level and returns null. Tests assert that no snapshot request occurs after either failure.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Minimal · up to cdaa5

The change is narrowly scoped and its capture-failure behavior is covered by tests for both exception types.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title contains the Jira key PER-8903, which matches the required pattern, and it has a plain-English description of the change: skipping the snapshot when DOM capture fails. It describes the main …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title contains the Jira key PER-8903, which matches the required pattern, and it has a plain-English description of the change: skipping the snapshot when DOM capture fails. It describes the main change in the changeset, which makes Percy.snapshot return null and skip the /percy/snapshot request on capture failure.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}