Skip to content

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

Merged
nekevss merged 1 commit intoboa-dev:mainfrom
KaustubhOG:fix/convert-panics-jstypedarray
Mar 3, 2026
Merged

fix(jstypedarray): convert panics to EngineError::Panic using js_expect#4844
nekevss merged 1 commit intoboa-dev:mainfrom
KaustubhOG:fix/convert-panics-jstypedarray

Conversation

@KaustubhOG
Copy link
Copy Markdown
Contributor

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.

@KaustubhOG KaustubhOG requested a review from a team as a code owner March 3, 2026 20:17
@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: 53feace8965b027c992133fa224c1278f051033b

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 35.29412% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.34%. Comparing base (6ddc2b4) to head (53feace).
⚠️ Report is 756 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/object/builtins/jstypedarray.rs 35.29% 11 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4844       +/-   ##
===========================================
+ Coverage   47.24%   57.34%   +10.09%     
===========================================
  Files         476      554       +78     
  Lines       46892    60566    +13674     
===========================================
+ Hits        22154    34730    +12576     
- Misses      24738    25836     +1098     

☔ 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 nekevss added this pull request to the merge queue Mar 3, 2026
Merged via the queue into boa-dev:main with commit 41a9ab5 Mar 3, 2026
19 checks passed
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
…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.
@jedel1043 jedel1043 added A-Technical Debt Changes related to technical debt A-Internal Changes that don't modify execution behaviour labels Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Internal Changes that don't modify execution behaviour A-Technical Debt Changes related to technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants