Skip to content

fix(engine): Prefer js_error! over JsError::from_opaque#5116

Open
p0lyw0lf wants to merge 1 commit intoboa-dev:mainfrom
p0lyw0lf:js_error-instead-of-from_opaque
Open

fix(engine): Prefer js_error! over JsError::from_opaque#5116
p0lyw0lf wants to merge 1 commit intoboa-dev:mainfrom
p0lyw0lf:js_error-instead-of-from_opaque

Conversation

@p0lyw0lf
Copy link
Contributor

In a few places, native code was throwing
JsError::from_opaque(js_string!("some literal")). This is allowed, but it is not exactly standard: most users will expect thrown values to be of type Error.

So, where possible, let's convert code using that to instead use the provided js_error! macro, which does generate proper Error objects.

This doesn't touch AbortError, because (1) that's a larger code change, and (2) it might actually be relied on as a sentinel value.

In a few places, native code was throwing
`JsError::from_opaque(js_string!("some literal"))`. This is allowed, but
it is not exactly standard: most users will expect thrown values to be
of type `Error`.

So, where possible, let's convert code using that to instead use the
provided `js_error!` macro, which does generate proper `Error` objects.

This doesn't touch `AbortError`, because (1) that's a larger code
change, and (2) it might actually be relied on as a sentinel value.
@p0lyw0lf p0lyw0lf requested a review from a team as a code owner March 17, 2026 02:28
@github-actions github-actions bot added the Waiting On Review Waiting on reviews from the maintainers label Mar 17, 2026
@github-actions github-actions bot added this to the v1.0.0 milestone Mar 17, 2026
@github-actions github-actions bot added the C-Builtins PRs and Issues related to builtins/intrinsics label Mar 17, 2026
@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 52,963 52,963 0
Passed 50,073 50,073 0
Ignored 2,072 2,072 0
Failed 818 818 0
Panics 0 0 0
Conformance 94.54% 94.54% 0.00%

Tested main commit: 197b736bb87523a14b41834c078adbec3059d237
Tested PR commit: e5bd33f426546531ea323801867d5a6d2bb77963
Compare commits: 197b736...e5bd33f

@codecov
Copy link

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 25.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.30%. Comparing base (6ddc2b4) to head (e5bd33f).
⚠️ Report is 874 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/module/loader/mod.rs 42.85% 4 Missing ⚠️
core/engine/src/builtins/date/mod.rs 0.00% 3 Missing ⚠️
core/engine/src/interop/into_js_function_impls.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5116       +/-   ##
===========================================
+ Coverage   47.24%   59.30%   +12.06%     
===========================================
  Files         476      563       +87     
  Lines       46892    62785    +15893     
===========================================
+ Hits        22154    37235    +15081     
- Misses      24738    25550      +812     

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Builtins PRs and Issues related to builtins/intrinsics Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant