Skip to content

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

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

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

Conversation

@KaustubhOG
Copy link
Copy Markdown
Contributor

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.

@KaustubhOG KaustubhOG requested a review from a team as a code owner March 3, 2026 18:55
@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: e74e04ba494fa34250901e9c2982b71cd10a21c9

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 3, 2026

Codecov Report

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

Files with missing lines Patch % Lines
core/engine/src/object/builtins/jsdataview.rs 0.00% 59 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4842       +/-   ##
===========================================
+ Coverage   47.24%   57.32%   +10.08%     
===========================================
  Files         476      554       +78     
  Lines       46892    60611    +13719     
===========================================
+ Hits        22154    34746    +12592     
- Misses      24738    25865     +1127     

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

@KaustubhOG
Copy link
Copy Markdown
Contributor Author

image hey @jedel1043 is that internal server error is because of github or any mistake fromm my side

@nekevss
Copy link
Copy Markdown
Member

nekevss commented Mar 3, 2026

GitHub has been having some service issues. 500 points to GitHub

@KaustubhOG KaustubhOG force-pushed the fix/convert-panics-jsdataview branch from 0b762c7 to e74e04b Compare March 3, 2026 20:57
@nekevss nekevss added this pull request to the merge queue Mar 4, 2026
@jedel1043 jedel1043 added A-Enhancement New feature or request A-API Changes related to public APIs labels Mar 4, 2026
Merged via the queue into boa-dev:main with commit f5f367d Mar 4, 2026
19 checks passed
ashddev pushed a commit to ashddev/boa that referenced this pull request Mar 4, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-API Changes related to public APIs A-Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants