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-botnodejs-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

codecovBot commented May 22, 2026

Copy link
Copy Markdown

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 linesPatch %Lines
src/node_errors.cc14.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 linesCoverage Δ
src/node_errors.cc63.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.

@codeCraft-RitikcodeCraft-Ritik left a comment

Copy link
Copy Markdown

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>
Sign up for freeto 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-ciPRs 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

@Anshikakalpana@codeCraft-Ritik@nodejs-github-bot