feat(profiling): Add support for Node v24 in the prune script#18447
feat(profiling): Add support for Node v24 in the prune script#18447
Conversation
| 18: '108', | ||
| 20: '115', | ||
| 22: '127', | ||
| 24: '134', |
There was a problem hiding this comment.
Bug: The NODE_TO_ABI mapping for Node.js v24 is incorrect, using '134' instead of the stable '137' ABI version.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The NODE_TO_ABI mapping at line 66 incorrectly hardcodes the ABI version for Node.js v24 as '134'. The actual stable release of Node.js 24.0.0 uses ABI version '137'. This discrepancy causes the prune() function to incorrectly filter binaries, leading to the deletion of correct profiler binaries (those with ABI 137) or failure to retain them. Consequently, the application will lack a compatible profiler binary at runtime, resulting in functional failures when attempting to load the native module.
💡 Suggested Fix
Update the NODE_TO_ABI mapping in packages/profiling-node/scripts/prune-profiler-binaries.js to set the ABI for Node.js v24 to '137' instead of '134'.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/profiling-node/scripts/prune-profiler-binaries.js#L66
Potential issue: The `NODE_TO_ABI` mapping at line 66 incorrectly hardcodes the ABI
version for Node.js v24 as '134'. The actual stable release of Node.js 24.0.0 uses ABI
version '137'. This discrepancy causes the `prune()` function to incorrectly filter
binaries, leading to the deletion of correct profiler binaries (those with ABI 137) or
failure to retain them. Consequently, the application will lack a compatible profiler
binary at runtime, resulting in functional failures when attempting to load the native
module.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6493649
(closes #18428)
(closes JS-1266)
This adds support for Node v24 in the prune script.
On top this also adds a test that is testing against the current Node version (as suggested in #14491 (review)). Since we have a matrix for our integration tests, this test would fail once we add Node v26 - where we are forced to update the ABI manually.
Theoretically we could also use node-abi, but decided against it to keep the dependencies low.