Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧Release
Other
🤖 This preview updates automatically when you update the PR. |
There was a problem hiding this comment.
Pull request overview
This PR aims to restore compatibility with V8 v14 (e.g., Electron v38) by avoiding v8::JSON::Parse usage in the native addon and shifting JSON parsing of thread state into the JavaScript wrapper, alongside updates to build/install behavior and CI coverage.
Changes:
- Move
asyncState/pollStateJSON parsing frommodule.ccintosrc/index.ts. - Update
node-abito a newer version to cover newer Node.js ABIs. - Ensure source-built binaries are copied into the expected
lib/target on install; expand CI Node.js matrix to include Node 25.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/index.ts |
Parses native-returned JSON strings into JS objects for asyncState/pollState. |
module.cc |
Stops calling v8::JSON::Parse and returns JSON strings for state fields. |
scripts/check-build.mjs |
Makes source compilation path also copy the built .node into the expected target location. |
package.json |
Bumps node-abi dependency range. |
yarn.lock |
Locks updated node-abi resolution. |
.github/workflows/ci.yml |
Adds Node.js 25 to the test matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| const thread: Thread<A, P> = { | ||
| frames: value.frames, | ||
| ...(value.asyncState && { asyncState: JSON.parse(value.asyncState) }), | ||
| ...(value.pollState && { pollState: JSON.parse(value.pollState) }), |
There was a problem hiding this comment.
Unhandled JSON.parse can crash entire stack capture
Medium Severity
The JSON.parse calls for asyncState and pollState have no error handling. The previous C++ code using v8::JSON::Parse gracefully skipped a property if parsing failed, preserving the rest of the thread data. Now, if JSON.parse throws for any thread's state, the entire captureStackTrace() call fails and all thread data (including stack frames for every thread) is lost. Since this function is used in watchdog/monitoring contexts, a total failure means monitoring silently stops working. Wrapping these in try-catch to match the old resilient behavior would be prudent.


This PR adds support for V8 v14 by moving the json parsing from the native module into JavaScript. The previous solution was either causing build errors or runtime crashes depending on build config.
This PR also:
node-abito support newer Node.js versionsThanks to @nikitakot for supplying a thoroughly tested fix with a diff!