Raise on evaluate_js failure instead of returning a null DataSpace

[giordano-lucas]

Summary

execute(type="evaluate_js") failed silently, while scrape() — the same contract, in the same file — got it right.

Both eval-js failure branches in NotteSession._aexecute_impl set success = False and a descriptive message, but left exception = None. Both raise gates key off exception, not success:

• notte-browser/src/notte_browser/session.py: if _raise_on_failure and exception is not None
• notte-sdk/src/notte_sdk/endpoints/sessions.py: if _raise_on_failure and result.exception is not None

raise_on_session_execution_failure = true is the shipped default, so it never fired. The caller got success=False, data=None, exception=None and crashed two lines later on result.data.markdown with 'NoneType' object has no attribute 'markdown', with the real reason ("JavaScript evaluation timed out after 45000ms") sitting unread in .message. Five deployed marketplace Functions failed this way; one turned "the page never loaded" into a reported JSON parse error.

scrape() already does the right thing: it records the failure to the trajectory with exception=e and re-raises unless raise_on_failure=False, in which case it returns a value that says it failed rather than None. This makes eval-js behave the same way.

What changed

1. Raise on evaluate_js failure instead of returning a null DataSpace

• attach an ActionExecutionError carrying message as its reason on both the asyncio.TimeoutError and PlaywrightError branches, the way controller.py already does for "Element is disabled"
• extend the SDK's existing generic-message fallback to recognise ActionExecutionError's user-facing message, so a remote caller is raised the actual reason instead of "Sorry, this action cannot be executed at the moment."

The success path is untouched. A JS null is still a successful evaluation returning data.markdown == "null", which is what makes result.data is None after an eval an unambiguous failure signal.

2. Gate raise_on_failure on failure, not on an exception having been thrown — ⚠️ behaviour change, droppable

Kept as a separate commit so it can be dropped in review without losing the eval-js fix.

Both gates now read not success instead of exception is not None, synthesising an ActionExecutionError from .message when no exception is available. This is a real behaviour change for external callers who today receive a silent success=False — most relevantly anyone with a custom BaseTool whose ExecutionResult carries success=False, which will now raise under the default.

Evidence it is safe in-repo — every consumer that wants quiet failures already opts out explicitly:

• notte-agent/src/notte_agent/agent.py:251 — raise_on_failure=False
• notte-agent/src/notte_agent/agent_fallback.py:127 — raise_on_failure=False (and it rejects raise_on_failure=True outright at line 104)
• notte-mcp/src/notte_mcp/server.py:138 — builds its session with raise_on_failure=False
• the documented pattern for optional actions is already raise_on_failure=False (docs/src/guides/web_automation_tips.mdx, browser-controls/{conditional_actions,error_handling}.mdx, guides/handle_optional_popup.mdx)

And the practical blast radius is narrower than it looks: controller.execute() returns a bool, but it raises on interaction failures (ActionExecutionError, InvalidActionError, FailedToUploadFileError, …) rather than returning False — the only False it returns is for HelpAction, which is agent-only. So a failed click already raises today. No in-repo tool returns success=False. The paths this commit actually newly covers are third-party tools and HelpAction.

3. Document raise_on_failure's actual default — docs/src/features/sessions/configuration.mdx claimed default={false}; notte-core/src/notte_core/config.toml sets it to true.

The remote path

Catalog Functions run against the API, so the action executes server-side and the failure has to survive serialisation. Verified by round-tripping a real ExecutionResult through model_dump_json() / model_validate_json():

• ExecutionResult.exception serialises via json_encoders to str(e), and the field_validator rebuilds it as a NotteBaseError. The concrete type is not preserved, so a remote caller can never receive ActionExecutionError itself — only a NotteBaseError.
• str(e) is the message captured at construction time, i.e. whatever ErrorConfig mode the API is in. The existing _GENERIC_UNEXPECTED_MESSAGES entries are verbatim user_message strings from notte_browser/errors.py, which is good evidence the API serialises in user mode. In that mode ActionExecutionError reduces to "Sorry, this action cannot be executed at the moment. …" and the reason is lost — hence the prefix check added in commit 1. tests/sdk/test_execute_raise_on_failure.py covers both developer and user server modes and fails on the user case without it.
• Commit 2's SDK-side gate additionally covers version skew: until the API ships this fix it will keep returning success=False, exception=None, and the client raises the reason from .message anyway.

Tests

tests/test_session.py (local) and tests/sdk/test_execute_raise_on_failure.py (remote, new file):

• eval-js timeout raises under the default; Playwright error raises under the default
• raise_on_failure=False still returns a result that says it failed — success=False, message intact, exception set — and does not regress to returning None
• success path untouched, including a JS null → data.markdown == "null", success=True, exception is None
• an action that fails by returning False raises under the default and stays quiet with raise_on_failure=False
• remote: raise survives the JSON round trip in both developer and user server error modes, and when the server attaches no exception at all

Ran:

• uv run pytest tests/test_session.py -q — 33 passed, 1 skipped, 1 failed: test_step_should_return_valid_timed_span, which calls Gemini and fails with key=None in a worktree with no .env. It passes in the main checkout, and it fails identically with these commits stashed.
• uv run pytest tests/sdk/test_execute_raise_on_failure.py -q — 5 passed
• uv run pytest tests/browser tests/test_trajectory.py tests/actions tests/mcp tests/code tests/config -q — 173 passed, 7 skipped. The 2 failures + 2 errors are all missing-credentials or network flakiness (test_tools.py needs a real NOTTE_API_KEY; test_screenshot_types.py depends on google.com's live DOM and fails identically with these commits stashed).
• Each new failure-path test was confirmed to fail against the unpatched source before being confirmed to pass against the patched source.
• pre-commit on every commit: ruff check, ruff format, basedpyright (0 errors, 0 warnings), detect-secrets, forbidden/playwright import checks, docs link checks — all pass. The docs-sdk-generate hook was skipped: it re-fetches https://api.notte.cc/openapi.json and rewrites docs/src/llms.txt with unrelated live-API drift (mailboxes, profile-duplicate, …). No public signature or docstring changed, so no reference doc regeneration is warranted.

Not run: integration suites requiring API credentials (tests/integration/**), and no marketplace sweeps.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by CodeRabbit

• New Features

  • Session actions now raise clear execution errors for timeouts, browser failures, and unsuccessful results by default.
  • Failure messages preserve the original reason, including when no exception is provided.
  • JavaScript evaluation failures are reported consistently across result types.

• Configuration

  • raise_on_failure now defaults to enabled.
  • Disable this option to receive failed results without raising an error.

• Bug Fixes

  • Improved failure handling for local and remote session execution.
  • Actions returning unsuccessful results are now handled consistently.

---

Added: the generated code, as a test

The unit tests above cover the two failure branches. This covers the thing the branches exist for — the pattern the anything-api builder actually emitted, reduced from the real tapology.com, pokerdb.thehendonmob.com and carrefour.be sources:

extraction = session.execute(type="evaluate_js", code=BOUT_SEARCH_SCRIPT)
raw_payload = extraction.data.markdown          # ← the line five Functions died on

tests/integration/test_generated_function_patterns.py, which the CI suite already picks up (only test_webvoyager_resolution and test_e2e are excluded). Runs in ~9s against example.com, the page the other integration tests here use.

The failure is provoked the way it happened in production rather than synthetically: a script that reads a property off an element it expects, run against a page that does not have it. That is what a block page, an interstitial or a redesign looks like from inside evaluate_js, and it is how at least three of the five actually failed.

Checked against the code before this branch, the first test reports:

Tapology bout search request failed: 'NoneType' object has no attribute 'markdown'

which is verbatim what the catalogue ledger recorded for that Function in production. With the branch, the same call reports JavaScript evaluation failed: ... and names the page.

Three cases:

test	catches the regression?
a failed extraction names the failure, not the attribute	yes — red before, green after
the or "" variant no longer reports bad JSON	yes — red before, green after
opting out still hands back the reason	no — raise_on_failure=False returned .message before this branch too

The third is a characterisation test, not a regression test, and is included because it pins the path the builder prompt now teaches callers to take. Calling that out so nobody reads three green ticks as three guarantees.

Suite: 36 passed. One unrelated pre-existing failure, test_step_should_return_valid_timed_span, which makes a live LLM call and fails on credentials in a worktree — it fails identically without these commits.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
Nottelabs	🟢 Ready	View Preview	Aug 24, 2026, 4:54 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2d52cedd-6268-4c7e-8153-3466ce58f3d0

📥 Commits

Reviewing files that changed from the base of the PR and between 18ab81d and f80671f.

📒 Files selected for processing (2)

• packages/notte-browser/src/notte_browser/session.py
• packages/notte-sdk/src/notte_sdk/endpoints/sessions.py

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

Walkthrough

The default raise_on_failure setting is now true. Browser sessions create ActionExecutionError values for JavaScript and Playwright failures and for unsuccessful actions without exceptions. RemoteSession.execute now detects unsuccessful serialized results, creates fallback errors, and preserves action-specific messages. Tests cover local and remote failures, disabled raising, and successful JavaScript results.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to f8067

The PR makes evaluate_js failures raise descriptive errors while preserving successful results and opt-out behavior; no actionable merge-blocking risk remains, so it is merge-ready after normal checks.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: raising on evaluate_js failures instead of returning a null result.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/eval-js-failure-raises

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

The PR makes local and remote JavaScript-evaluation failures raise actionable errors under the default failure policy while preserving explicitly non-raising execution. It also corrects the documented default and adds local-browser and serialized SDK coverage.

• Attaches ActionExecutionError to JavaScript timeout and Playwright failure results.
• Gates local and remote raising on unsuccessful execution, including failures without attached exceptions.
• Restores action-specific remote diagnostics when serialization reduces an exception to a generic user-facing message.
• Documents raise_on_failure as enabled by default.

Confidence Score: 5/5

The PR appears safe to merge, with failure propagation and opt-out behavior covered across local and remote execution paths.

The accepted changes consistently convert unsuccessful evaluations into actionable exceptions while retaining failed ExecutionResult objects when raising is explicitly disabled, and no concrete blocking or non-blocking defect remains.

Important Files Changed

Filename	Overview
packages/notte-browser/src/notte_browser/session.py	Adds explicit evaluate-JavaScript exceptions and consistently applies the configured failure-raising policy.
packages/notte-sdk/src/notte_sdk/endpoints/sessions.py	Raises remote unsuccessful execution results and recovers detailed messages from generic serialized action errors.
tests/test_session.py	Covers local timeout, Playwright error, successful JavaScript values, and non-exception failure behavior.
tests/sdk/test_execute_raise_on_failure.py	Covers serialized remote failures across error modes and both raising and non-raising configurations.
docs/src/features/sessions/configuration.mdx	Corrects the documented raise_on_failure default to true.

_{Reviews (1): Last reviewed commit: "Document raise_on_failure's actual defau..." | Re-trigger Greptile}

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

[blacksmith-sh]

Found 1 test failure on Blacksmith runners:

Failure

Test	View Logs
test_snippets/test_python_testers[vaults_index]	View Logs

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[github-actions]

Tests	Skipped	Failures	Errors	Time
896	32 💤	0 ❌	0 🔥	8m 44s ⏱️