Improve panic error ergonomics by jedel1043 · Pull Request #4828 · boa-dev/boa

jedel1043 · 2026-03-03T03:59:29Z

I was experimenting with the new PanicError and I found it really useful, but kind of a pain to use. This adjusts its definition such that it is kind of a replacement for expect: you just call js_expect on either a JsResult or an Option and it automatically creates the PanicError.

Before

let promise = self
            .stack
            .get(frame.promise_capability_promise_register_index())
            .expect("stack must have a promise capability")
            .as_object()?;

After:

let promise = self
            .stack
            .get(frame.promise_capability_promise_register_index())
            .and_then(JsValue::as_object)
            .js_expect("stack must have a promise capability")?;

cc @KaustubhOG so that you're aware about the API changes

github-actions · 2026-03-03T04:09:56Z

Test262 conformance changes

Test result	main count	PR count	difference
Total	52,862	52,862	0
Passed	49,627	49,627	0
Ignored	2,263	2,263	0
Failed	972	972	0
Panics	0	0	0
Conformance	93.88%	93.88%	0.00%

Tested PR commit: a029fd94f8a81a9ca29b08144620c19555fe793d

codecov · 2026-03-03T04:12:50Z

Codecov Report

❌ Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.36%. Comparing base (6ddc2b4) to head (a029fd9).
⚠️ Report is 729 commits behind head on main.

Files with missing lines	Patch %	Lines
core/engine/src/error/mod.rs	0.00%	19 Missing ⚠️
core/engine/src/lib.rs	0.00%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #4828       +/-   ##
===========================================
+ Coverage   47.24%   57.36%   +10.11%     
===========================================
  Files         476      554       +78     
  Lines       46892    60564    +13672     
===========================================
+ Hits        22154    34740    +12586     
- Misses      24738    25824     +1086

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nekevss

LGTM

@KaustubhOG

I was experimenting with the new PanicError and I found it really useful, but kind of a pain to use. This adjusts its definition such that it is kind of a replacement for `expect`: you just call `js_expect` on either a `JsResult` or an `Option` and it automatically creates the `PanicError`. ### Before ```rust let promise = self .stack .get(frame.promise_capability_promise_register_index()) .expect("stack must have a promise capability") .as_object()?; ``` After: ```rust let promise = self .stack .get(frame.promise_capability_promise_register_index()) .and_then(JsValue::as_object) .js_expect("stack must have a promise capability")?; ``` cc @KaustubhOG so that you're aware about the API changes

KaustubhOG · 2026-03-03T15:28:51Z

I was experimenting with the new PanicError and I found it really useful, but kind of a pain to use. This adjusts its definition such that it is kind of a replacement for expect: you just call js_expect on either a JsResult or an Option and it automatically creates the PanicError.

Before
let promise = self
            .stack
            .get(frame.promise_capability_promise_register_index())
            .expect("stack must have a promise capability")
            .as_object()?;
After:
let promise = self
            .stack
            .get(frame.promise_capability_promise_register_index())
            .and_then(JsValue::as_object)
            .js_expect("stack must have a promise capability")?;
cc @KaustubhOG so that you're aware about the API changes

Thanks for the cc! Already using js_expect in #4837

…4837) Part of #3241 Converts all 14 panics in `jsarray.rs` to use `js_expect` introduced in #4828, replacing `.expect()` calls that would crash the process with recoverable `EngineError::Panic` errors that bubble up to the host application. Changes: - `JsArray::new` now returns `JsResult<Self>` instead of `Self` - All 14 `.expect()` calls replaced with `.js_expect()?` - Updated all callers of `JsArray::new` across the codebase - Fixed doc examples in `jsmap.rs` and `jsobject.rs`

…ct (#4844) Part of #3241 Converts all 26 panics in `jstypedarray.rs` to `EngineError::Panic` using the `js_expect` ergonomics introduced in #4828. Unlike #4837 which touched 8 files due to a public API change, this PR only modifies 1 file since no public API signatures were changed all methods already returned `JsResult<T>`, only the internal implementations were updated. Patterns converted: - `.as_number().map(...).expect()` → `.js_expect()?` - `.as_object().expect()` → `.as_object().js_expect()?` - `.as_boolean().expect()` → `.as_boolean().js_expect()?` - `.map(|x| x.as_string().expect())` → `.and_then(|x| x.as_string().js_expect().map_err(Into::into))` - 3 macro panics inside `JsTypedArrayType!` (`from_array_buffer`, `from_shared_array_buffer`, `from_iter`) The 4 remaining `.expect(` in the file are inside `///` doc comment examples, not real code.

…ct (boa-dev#4844) Part of boa-dev#3241 Converts all 26 panics in `jstypedarray.rs` to `EngineError::Panic` using the `js_expect` ergonomics introduced in boa-dev#4828. Unlike boa-dev#4837 which touched 8 files due to a public API change, this PR only modifies 1 file since no public API signatures were changed all methods already returned `JsResult<T>`, only the internal implementations were updated. Patterns converted: - `.as_number().map(...).expect()` → `.js_expect()?` - `.as_object().expect()` → `.as_object().js_expect()?` - `.as_boolean().expect()` → `.as_boolean().js_expect()?` - `.map(|x| x.as_string().expect())` → `.and_then(|x| x.as_string().js_expect().map_err(Into::into))` - 3 macro panics inside `JsTypedArrayType!` (`from_array_buffer`, `from_shared_array_buffer`, `from_iter`) The 4 remaining `.expect(` in the file are inside `///` doc comment examples, not real code.

…#4842) Part of #3241 Converts all 12 panics in `jsdataview.rs` to use `js_expect` introduced in #4828, replacing `.expect()` calls that would crash the process with recoverable `EngineError::Panic` errors that bubble up to the host application. No public API changes — all methods already returned `JsResult<T>`, only the internal implementation was updated.

…4854) Part of #3241 Converts all 13 panics in `jsproxy.rs` to `EngineError::Panic` using `js_expect` introduced in #4828. `build()` and `build_revocable()` return types changed from `JsProxy`/ `JsRevocableProxy` to `JsResult<JsProxy>`/`JsResult<JsRevocableProxy>`. Checked for external callers —> only a type re-export in `mod.rs`, so the cascade is limited to 1 file. Note: The two `JsNativeError::typ()` usages in `from_object()` and `TryFromJs` are intentionally left unchanged (becauze i think these are correct user-facing TypeErrors, not internal implementation failures.)

…oa-dev#4837) Part of boa-dev#3241 Converts all 14 panics in `jsarray.rs` to use `js_expect` introduced in boa-dev#4828, replacing `.expect()` calls that would crash the process with recoverable `EngineError::Panic` errors that bubble up to the host application. Changes: - `JsArray::new` now returns `JsResult<Self>` instead of `Self` - All 14 `.expect()` calls replaced with `.js_expect()?` - Updated all callers of `JsArray::new` across the codebase - Fixed doc examples in `jsmap.rs` and `jsobject.rs`

…ct (boa-dev#4844) Part of boa-dev#3241 Converts all 26 panics in `jstypedarray.rs` to `EngineError::Panic` using the `js_expect` ergonomics introduced in boa-dev#4828. Unlike boa-dev#4837 which touched 8 files due to a public API change, this PR only modifies 1 file since no public API signatures were changed all methods already returned `JsResult<T>`, only the internal implementations were updated. Patterns converted: - `.as_number().map(...).expect()` → `.js_expect()?` - `.as_object().expect()` → `.as_object().js_expect()?` - `.as_boolean().expect()` → `.as_boolean().js_expect()?` - `.map(|x| x.as_string().expect())` → `.and_then(|x| x.as_string().js_expect().map_err(Into::into))` - 3 macro panics inside `JsTypedArrayType!` (`from_array_buffer`, `from_shared_array_buffer`, `from_iter`) The 4 remaining `.expect(` in the file are inside `///` doc comment examples, not real code.

…boa-dev#4842) Part of boa-dev#3241 Converts all 12 panics in `jsdataview.rs` to use `js_expect` introduced in boa-dev#4828, replacing `.expect()` calls that would crash the process with recoverable `EngineError::Panic` errors that bubble up to the host application. No public API changes — all methods already returned `JsResult<T>`, only the internal implementation was updated.

…oa-dev#4854) Part of boa-dev#3241 Converts all 13 panics in `jsproxy.rs` to `EngineError::Panic` using `js_expect` introduced in boa-dev#4828. `build()` and `build_revocable()` return types changed from `JsProxy`/ `JsRevocableProxy` to `JsResult<JsProxy>`/`JsResult<JsRevocableProxy>`. Checked for external callers —> only a type re-export in `mod.rs`, so the cascade is limited to 1 file. Note: The two `JsNativeError::typ()` usages in `from_object()` and `TryFromJs` are intentionally left unchanged (becauze i think these are correct user-facing TypeErrors, not internal implementation failures.)

…4846) Part of #3241 Converts all 17 panics in `jsregexp.rs` to `EngineError::Panic` using `js_expect` introduced in #4828. No public API signatures were changed — only 1 file modified. Patterns converted: - `.as_object().expect()` → `.as_object().js_expect()?` - `.map(|v| v.as_boolean().expect())` → `.and_then(|v| v.as_boolean().js_expect().map_err(Into::into))` - `.map(|v| v.as_string().expect()...to_std_string().expect())` → `.and_then(|v| v.as_string().js_expect()?.to_std_string().map_err(...))` - nested `.expect()` calls in `exec()` converted using `?` inside `.and_then()`

…4875) Part of #3241 Converts all 23 panics in jspromise.rs to EngineError::Panic using js_expect introduced in #4828. Patterns converted: - `.js_expect()` on internal promise operations that cannot fail - `.js_expect().map_err(Into::into)` for methods returning `JsResult<JsPromise>` Signature changes: - `resolve()`, `reject()`, `from_result()` → `JsResult<Self>` - `then()`, `catch()`, `finally()` → `JsResult<JsPromise>` - `all()`, `all_settled()`, `any()`, `race()` → `JsResult<JsPromise>` - `into_js_future()` → `JsResult<JsFuture>` Files modified: 6

Improve panic error ergonomics

c74156a

jedel1043 requested a review from a team as a code owner March 3, 2026 03:59

jedel1043 added A-Enhancement New feature or request A-API Changes related to public APIs labels Mar 3, 2026

cargo fmt

161d4c7

Merge branch 'main' into improve-panic-error

a029fd9

nekevss approved these changes Mar 3, 2026

View reviewed changes

nekevss added this pull request to the merge queue Mar 3, 2026

Merged via the queue into boa-dev:main with commit b165d16 Mar 3, 2026
19 checks passed

jedel1043 deleted the improve-panic-error branch March 3, 2026 07:42

KaustubhOG mentioned this pull request Mar 3, 2026

fix(jsarray): convert panics to EngineError::Panic using js_expect #4837

Merged

This was referenced Mar 3, 2026

fix(jsdataview): convert panics to EngineError::Panic using js_expect #4842

Merged

fix(jstypedarray): convert panics to EngineError::Panic using js_expect #4844

Merged

Address Panics in boa_engine #3241

Open

KaustubhOG mentioned this pull request Mar 3, 2026

fix(jsregexp): convert panics to EngineError::Panic using js_expect #4846

Merged

KaustubhOG mentioned this pull request Mar 4, 2026

fix(jsproxy): convert panics to EngineError::Panic using js_expect #4854

Merged

KaustubhOG mentioned this pull request Mar 5, 2026

fix(jspromise): convert panics to EngineError::Panic using js_expect #4875

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve panic error ergonomics#4828

Improve panic error ergonomics#4828
nekevss merged 3 commits intoboa-dev:mainfrom
jedel1043:improve-panic-error

jedel1043 commented Mar 3, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Mar 3, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Mar 3, 2026 •

edited

Loading

Uh oh!

nekevss left a comment

Uh oh!

Uh oh!

KaustubhOG commented Mar 3, 2026

Before

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jedel1043 commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before

Uh oh!

github-actions Bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test262 conformance changes

Uh oh!

codecov Bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nekevss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

KaustubhOG commented Mar 3, 2026

Before

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jedel1043 commented Mar 3, 2026 •

edited

Loading

github-actions Bot commented Mar 3, 2026 •

edited

Loading

codecov Bot commented Mar 3, 2026 •

edited

Loading