Skip to content

fix(browser): stop reporting runtime SyntaxErrors as caller syntax errors - #358

Merged
ankitranjan7 merged 1 commit into
fix/browser-run-syntax-positionfrom
fix/browser-run-runtime-syntax-classification
Aug 19, 2026
Merged

ankitranjan7 merged 1 commit into
fix/browser-run-syntax-positionfrom
fix/browser-run-runtime-syntax-classification

Conversation

@ankitranjan7

Copy link
Copy Markdown
Contributor

Closes #355

Stacked on #356 (fix/browser-run-syntax-position) — merge that one first; this PR targets its branch and only makes sense on top of it.

What was wrong

normalizeExecutionError classified by error name:

if (/syntaxerror/i.test(`${errorKind}: ${message}`)) { … BROWSER_RUN_SYNTAX_ERROR … }

That matches any SyntaxError reaching the host. JSON.parse on malformed input throws one, so a program with no syntax error at all was reported as having one, with a hint sending the reader to audit correct code.

What changed, and why this option

The branch is deleted, not rewritten.

#356 makes a genuine compile failure tag itself where it happens: the bootstrap catches the error from new AsyncFunction(source) and throws with code = 'BROWSER_RUN_SYNTAX_ERROR' plus line/column/excerpt/hint, which the earlier code.startsWith('BROWSER_RUN_') branch returns untouched. So the regex is now only ever reached by SyntaxErrors that are not compile failures of the caller's program — runtime ones.

Runtime TypeErrors and RangeErrors already fall through to the generic normalized error (errorCode: runtime_command_failed on the wire). Keeping a special branch for runtime SyntaxError — under any code or hint — would preserve exactly the arbitrariness the issue is about: an error singled out for special treatment because of its name rather than its origin. There is also no honest code for it in BrowserRunErrorCode, and inventing one that fires only for SyntaxError would be a new inconsistency, not a fix. Deleting the branch makes origin, not name, the classifier: compile failure → BROWSER_RUN_SYNTAX_ERROR with position; everything else → treated as the runtime error it is.

The runner-level error keeps its own identity (name === 'SyntaxError', original message). The daemon wire drops name and the details payload for all uncoded runtime errors — pre-existing behaviour for every runtime TypeError today, deliberately not widened here.

Before / after

Verified against a daemon restarted from this branch (npx tsx src/main.ts daemon restart → v0.7.2).

1. Genuine compile error — unchanged, still positioned (const a = 1; / const s = 'x'y'; / return a;):

✖  BROWSER_RUN_SYNTAX_ERROR: expecting ';' at line 2, column 14
  const s = 'x'y';
               ^
✖  Your program failed to compile; the browser was never touched. A common cause is an
   unescaped quote when embedding a URL or HTML in a string literal …

2. Runtime JSON.parse failure (return JSON.parse('1,2,3');):

before

✖  BROWSER_RUN_SYNTAX_ERROR: QuickJS promise rejected: unexpected data at the end
✖  Fix the browser-run JavaScript syntax and retry.

after

✖  runtime_command_failed: QuickJS promise rejected: unexpected data at the end

which is identical in shape to how a runtime TypeError reports today, confirmed in the same session:

✖  runtime_command_failed: QuickJS promise rejected: 'nope' is not defined

The underlying response.json() bug (#354) is out of scope here and untouched.

Tests

Extended #356's block in src/browser/run/runner.test.ts rather than duplicating it: its compile-error test now asserts through a shared runError helper, and a new case asserts JSON.parse('1,2,3') yields no BROWSER_RUN_* code, no hint, and keeps name === 'SyntaxError'.

npx vitest run --project unit: 117 failing tests across 7 files, identical to the #356 baseline measured before the edit (all in hosted/* and plugin-manifest suites plus the known doctor profile-display failure — none touch browser-run). src/browser/run/runner.test.ts passes 75/75.

npx tsc --noEmit: clean. The 5 pre-existing errors were #356's new test indexing a BrowserRunResult | Error union off .catch(); the runError helper types that away, which is why they are gone rather than carried forward.

Nothing else depended on the old behaviour

Grepped BROWSER_RUN_SYNTAX_ERROR, SyntaxError, and the removed hint string across src/, skills/, plugins/, and docs. The only references were the code itself, the BrowserRunErrorCode union (unchanged — the code is still produced by the compile path), and #356's test. skills/webcmd-browser/references/browser-run-playwright.md describes BROWSER_RUN_* errors only in the aggregate and never documented syntax-error classification, so it needs no update.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

🟠 Maintainer review suggested — low confidence

The automated review could not reach a fully supported conclusion.

Limitations

  • The automated review returned an invalid structured result.

This review is advisory and does not block merging.

@ankitranjan7
ankitranjan7 force-pushed the fix/browser-run-syntax-position branch from e9e3ae2 to 3997e54 Compare August 19, 2026 05:59
…rors

normalizeExecutionError matched /syntaxerror/i against any error reaching the
host, so a runtime JSON.parse failure in a perfectly valid program was reported
as BROWSER_RUN_SYNTAX_ERROR with a hint telling the reader to fix correct code.

Since compile failures now tag themselves at the point of compilation, the only
SyntaxErrors reaching this classifier are runtime ones. Drop the branch: a
runtime SyntaxError keeps its own name and message, exactly like the runtime
TypeErrors and RangeErrors that already flow through the generic path.

Closes #355

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ankitranjan7
ankitranjan7 force-pushed the fix/browser-run-runtime-syntax-classification branch from bbc459a to ac9a130 Compare August 19, 2026 06:00
@ankitranjan7
ankitranjan7 merged commit 5d1b08f into fix/browser-run-syntax-position Aug 19, 2026
36 checks passed
ankitranjan7 pushed a commit that referenced this pull request Aug 19, 2026
…pile errors (#356)

* fix(browser): report the caller's line and column for browser-run compile errors

A program that fails to compile was surfaced as
"QuickJS promise rejected: invalid number literal" with no position, because
new AsyncFunction(source) runs inside the promise and its SyntaxError carried
lineNumber/columnNumber that were then discarded. The prefix pointed at async
plumbing rather than the program, so readers hunted heredocs and await instead
of the actual defect: an unescaped quote in a string literal holding a URL.

Compile the program before racing it, and turn a compile failure into a
BROWSER_RUN_SYNTAX_ERROR carrying the caller's own line and column (the
AsyncFunction wrapper adds two lines, so subtract them), a windowed excerpt
with a caret, and a hint naming the quoting cause. Errors that already carry a
BROWSER_RUN_ code keep their message instead of gaining a host stage prefix.

runner.test.ts imported MemorySnapshotBaselineStore twice, so the file failed
to load and its 74 tests never ran; the duplicate is removed here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(browser): stop reporting runtime SyntaxErrors as caller syntax errors (#358)

normalizeExecutionError matched /syntaxerror/i against any error reaching the
host, so a runtime JSON.parse failure in a perfectly valid program was reported
as BROWSER_RUN_SYNTAX_ERROR with a hint telling the reader to fix correct code.

Since compile failures now tag themselves at the point of compilation, the only
SyntaxErrors reaching this classifier are runtime ones. Drop the branch: a
runtime SyntaxError keeps its own name and message, exactly like the runtime
TypeErrors and RangeErrors that already flow through the generic path.

Closes #355

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@ankitranjan7
ankitranjan7 deleted the fix/browser-run-runtime-syntax-classification branch September 1, 2026 12:42
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.

1 participant