Skip to content

Conversation

miguelg719
Copy link
Collaborator

@miguelg719 miguelg719 commented Jun 30, 2025

why

  • when the LLM returns an ID, we look up that ID in our ID->xpath mapping. If that element does not exist, it returns undefined. downstream, we attempt to run some regex on the xpath
  • when running the regex on the undefined xpath, we get the following error:
    TypeError: Cannot read properties of undefined (reading 'replace')
  • the actual fix for this is to add support for taking actions on elements within the shadow dom
  • for now, we should have better error handling for this case

what changed

  • added a check in the ObserveHandler to check if the returned ID contains -, which indicates that it will be present in our idToXpath mapping
  • if it does not contain -, then we log an error, and return "not-supported" as the selector, and "an element inside a shadow DOM" as the description

test plan

  • added an eval to ensure that we are returning "not-supported" as the selector, and "an element inside a shadow DOM" for elements that exist inside the shadow dom

Copy link

changeset-bot bot commented Jun 30, 2025

⚠️ No Changeset found

Latest commit: 4aba5a4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@seanmcguire12 seanmcguire12 marked this pull request as ready for review June 30, 2025 22:00
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Implemented graceful error handling for shadow DOM elements to prevent crashes when accessing unsupported elements in the shadow DOM.

  • Added check in lib/handlers/observeHandler.ts to detect shadow DOM elements by ID format and return 'not-supported' instead of crashing
  • Created new test file evals/tasks/shadow_dom.ts to verify graceful handling of shadow DOM interactions
  • Added 'shadow_dom' task to evals/evals.config.json under 'act' category for testing
  • Returns descriptive message "an element inside a shadow DOM" when encountering shadow elements

3 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile

Comment on lines +194 to +195
message: `Element is inside a shadow DOM: ${elementId}`,
level: 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Log level 0 suggests critical error, but this is more of a warning/info. Consider using level 1 to match other similar logs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seanmcguire12 we need warning level logs haha

@seanmcguire12 seanmcguire12 merged commit 890ffcc into main Jun 30, 2025
13 checks passed
seanmcguire12 pushed a commit that referenced this pull request Jul 4, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @browserbasehq/[email protected]

### Patch Changes

- [#856](#856)
[`8a43c5a`](8a43c5a)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - set
download behaviour by default

- [#857](#857)
[`890ffcc`](890ffcc)
Thanks [@miguelg719](https://github.com/miguelg719)! - return
"not-supported" for elements inside the shadow-dom

- [#844](#844)
[`64c1072`](64c1072)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - don't
automatically close tabs

- [#860](#860)
[`b077d3f`](b077d3f)
Thanks [@miguelg719](https://github.com/miguelg719)! - Set default
schema on extract options with no schema

- [#842](#842)
[`8bcb5d7`](8bcb5d7)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - improved
handling for OS level dropdowns

- [#846](#846)
[`7bf10c5`](7bf10c5)
Thanks [@miguelg719](https://github.com/miguelg719)! - Filter attaching
to target worker / shared_worker

## @browserbasehq/[email protected]

### Patch Changes

- Updated dependencies
\[[`8a43c5a`](8a43c5a),
[`890ffcc`](890ffcc),
[`64c1072`](64c1072),
[`b077d3f`](b077d3f),
[`8bcb5d7`](8bcb5d7),
[`7bf10c5`](7bf10c5)]:
    -   @browserbasehq/[email protected]

## @browserbasehq/[email protected]

### Patch Changes

- Updated dependencies
\[[`8a43c5a`](8a43c5a),
[`890ffcc`](890ffcc),
[`64c1072`](64c1072),
[`b077d3f`](b077d3f),
[`8bcb5d7`](8bcb5d7),
[`7bf10c5`](7bf10c5)]:
    -   @browserbasehq/[email protected]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants