Skip to content

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

Merged
jedel1043 merged 3 commits intoboa-dev:mainfrom
KaustubhOG:fix/convert-panics-jsarray
Mar 3, 2026
Merged

fix(jsarray): convert panics to EngineError::Panic using js_expect#4837
jedel1043 merged 3 commits intoboa-dev:mainfrom
KaustubhOG:fix/convert-panics-jsarray

Conversation

@KaustubhOG
Copy link
Copy Markdown
Contributor

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

@KaustubhOG KaustubhOG requested a review from a team as a code owner March 3, 2026 13:14
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 3, 2026

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: 28610ffa6c478d3736c3bd86bef2aeae655e2280

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.32%. Comparing base (6ddc2b4) to head (28610ff).
⚠️ Report is 730 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/object/builtins/jsarray.rs 28.57% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4837       +/-   ##
===========================================
+ Coverage   47.24%   57.32%   +10.08%     
===========================================
  Files         476      554       +78     
  Lines       46892    60581    +13689     
===========================================
+ Hits        22154    34730    +12576     
- Misses      24738    25851     +1113     

☔ 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.

Copy link
Copy Markdown
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@jedel1043 jedel1043 added A-Technical Debt Changes related to technical debt C-Builtins PRs and Issues related to builtins/intrinsics labels Mar 3, 2026
@jedel1043 jedel1043 added this pull request to the merge queue Mar 3, 2026
Merged via the queue into boa-dev:main with commit 54c5d42 Mar 3, 2026
19 checks passed
@KaustubhOG
Copy link
Copy Markdown
Contributor Author

@jedel1043 thanks

github-merge-queue Bot pushed a commit that referenced this pull request Mar 3, 2026
…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.
hansl pushed a commit to hansl/boa that referenced this pull request Mar 3, 2026
…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.
ashddev pushed a commit to ashddev/boa that referenced this pull request Mar 4, 2026
…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`
ashddev pushed a commit to ashddev/boa that referenced this pull request Mar 4, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Technical Debt Changes related to technical debt C-Builtins PRs and Issues related to builtins/intrinsics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants