Skip to content

fix: avoid retrying user JavaScript errors - #523

Open
ShiroKSH wants to merge 1 commit into
browser-use:mainfrom
ShiroKSH:fix/js-return-retry
Open

fix: avoid retrying user JavaScript errors#523
ShiroKSH wants to merge 1 commit into
browser-use:mainfrom
ShiroKSH:fix/js-return-retry

Conversation

@ShiroKSH

@ShiroKSH ShiroKSH commented Jul 14, 2026

Copy link
Copy Markdown

Summary

  • Retry a JavaScript snippet only for Chrome's exact top-level-return parser error.
  • Avoid a second evaluation after user code throws an error containing the same text.
  • Add regressions for user-thrown Error and SyntaxError values.

Root cause

The retry decision used a substring of a flattened runtime error. User code could throw that text after changing page state, causing the harness to execute the expression twice.

Checks

  • uv run --with pytest pytest tests/integration/test_js.py tests/unit/test_helpers.py -q
  • git diff --check

Summary by cubic

Only retry JavaScript evaluation for Chrome’s exact top‑level return SyntaxError. This prevents double execution and side effects when user code throws errors that mention “Illegal return statement”.

  • Bug Fixes
    • Detect Chrome’s top‑level return via result fields and raise _IllegalTopLevelReturn; helpers.js retries once by wrapping the expression.
    • Remove substring-based matching; do not retry on user‑thrown Error/SyntaxError that include “Illegal return statement”.
    • Add tests covering Chrome’s error path and regressions for user‑thrown errors.

Written for commit 182783e. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Re-trigger cubic

@Kastan97 Kastan97 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ollie (CTO) Review: APPROVED

What this does: Correctly identifies Illegal return statement via CDP's className (SyntaxError) instead of fragile string matching on the error message. Prevents retrying genuine user throw errors that happen to mention 'Illegal return' in their message.

Good to merge once CI passes.

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