Skip to content

errors: handle V8 warnings in DisallowJavascriptExecutionScope#63486

Open
Anshikakalpana wants to merge 1 commit into
nodejs:mainfrom
Anshikakalpana:fix-v8-warning-crash
Open

errors: handle V8 warnings in DisallowJavascriptExecutionScope#63486
Anshikakalpana wants to merge 1 commit into
nodejs:mainfrom
Anshikakalpana:fix-v8-warning-crash

Conversation

@Anshikakalpana
Copy link
Copy Markdown
Contributor

Fixes: #63473

Fix crash when V8 emits warning inside DisallowJavascriptExecutionScope.

When asm.js code is evaluated in the REPL with previews enabled, V8 emits a deprecation warning inside DisallowJavascriptExecutionScope. Node's warning handler tried to call process.emit() which is JS, causing a fatal crash.

Fix:
check can_call_into_js() before calling ProcessEmitWarningGeneric. If JS is not safe, print the warning directly to stderr instead.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels May 22, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (7eab492) to head (3b02778).
⚠️ Report is 195 commits behind head on main.

Files with missing lines Patch % Lines
src/node_errors.cc 14.28% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63486      +/-   ##
==========================================
+ Coverage   89.67%   90.14%   +0.46%     
==========================================
  Files         712      718       +6     
  Lines      221256   227917    +6661     
  Branches    42394    42815     +421     
==========================================
+ Hits       198420   205453    +7033     
+ Misses      14684    14248     -436     
- Partials     8152     8216      +64     
Files with missing lines Coverage Δ
src/node_errors.cc 63.37% <14.28%> (-0.53%) ⬇️

... and 159 files with indirect coverage changes

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

Copy link
Copy Markdown

@codeCraft-Ritik codeCraft-Ritik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix! Falling back to stderr when JS execution is disallowed seems like a safe and practical solution.

Fixes: nodejs#63473
Signed-off-by: Anshikakalpana <anshikajain196872@gmail.com>
@Anshikakalpana Anshikakalpana force-pushed the fix-v8-warning-crash branch from ec60a09 to 3b02778 Compare May 22, 2026 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

V8 warnings crash process within DisallowJavascriptExecutionScope

3 participants