Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0fd6524

Browse filesBrowse files
joyeecheungtargos
joyeecheung
authored and
targos
committed
process: split routines used to enhance fatal exception stack traces
Previously the enhancement were done right after emitting `'uncaughtException'`, which meant by the time we knew the exception was fatal in C++, the error.stack had already been patched. This patch moves those routines to be called later during the fatal exception handling, and split them into two stages: before and after the inspector is notified by the invocation of `V8Inspector::exceptionThrown`. We now expand the stack to include additional informations about unhandled 'error' events before the inspector is notified, but delay the highlighting of the frames until after the inspector is notified, so that the ANSI escape sequences won't show up in the inspector console. PR-URL: #28308 Fixes: #28287 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent b744bd9 commit 0fd6524
Copy full SHA for 0fd6524

File tree

Expand file treeCollapse file tree

10 files changed

+166
-55
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

10 files changed

+166
-55
lines changed
Open diff view settings
Collapse file

‎lib/events.js‎

Copy file name to clipboardExpand all lines: lib/events.js
+8-5Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,15 @@ const { Math, Object, Reflect } = primordials;
2525

2626
var spliceOne;
2727

28+
const {
29+
kEnhanceStackBeforeInspector,
30+
codes
31+
} = require('internal/errors');
2832
const {
2933
ERR_INVALID_ARG_TYPE,
3034
ERR_OUT_OF_RANGE,
3135
ERR_UNHANDLED_ERROR
32-
} = require('internal/errors').codes;
36+
} = codes;
3337

3438
const {
3539
inspect
@@ -142,8 +146,8 @@ function enhanceStackTrace(err, own) {
142146
ownStack.splice(off + 1, len - 2,
143147
' [... lines matching original stack trace ...]');
144148
}
145-
// Do this last, because it is the only operation with side effects.
146-
err.stack = err.stack + sep + ownStack.join('\n');
149+
150+
return err.stack + sep + ownStack.join('\n');
147151
}
148152

149153
EventEmitter.prototype.emit = function emit(type, ...args) {
@@ -162,11 +166,10 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
162166
er = args[0];
163167
if (er instanceof Error) {
164168
try {
165-
const { kExpandStackSymbol } = require('internal/util');
166169
const capture = {};
167170
// eslint-disable-next-line no-restricted-syntax
168171
Error.captureStackTrace(capture, EventEmitter.prototype.emit);
169-
Object.defineProperty(er, kExpandStackSymbol, {
172+
Object.defineProperty(er, kEnhanceStackBeforeInspector, {
170173
value: enhanceStackTrace.bind(null, er, capture),
171174
configurable: true
172175
});
Collapse file

‎lib/internal/bootstrap/node.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap/node.js
+15-2Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,22 @@ process.emitWarning = emitWarning;
299299
}
300300

301301
function setupPrepareStackTrace() {
302-
const { setPrepareStackTraceCallback } = internalBinding('errors');
303-
const { prepareStackTrace } = require('internal/errors');
302+
const {
303+
setEnhanceStackForFatalException,
304+
setPrepareStackTraceCallback
305+
} = internalBinding('errors');
306+
const {
307+
prepareStackTrace,
308+
fatalExceptionStackEnhancers: {
309+
beforeInspector,
310+
afterInspector
311+
}
312+
} = require('internal/errors');
313+
// Tell our PrepareStackTraceCallback passed to the V8 API
314+
// to call prepareStackTrace().
304315
setPrepareStackTraceCallback(prepareStackTrace);
316+
// Set the function used to enhance the error stack for printing
317+
setEnhanceStackForFatalException(beforeInspector, afterInspector);
305318
}
306319

307320
function setupProcessObject() {
Collapse file

‎lib/internal/errors.js‎

Copy file name to clipboardExpand all lines: lib/internal/errors.js
+41Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,45 @@ function addNumericalSeparator(val) {
617617
return `${val.slice(0, i)}${res}`;
618618
}
619619

620+
// Used to enhance the stack that will be picked up by the inspector
621+
const kEnhanceStackBeforeInspector = Symbol('kEnhanceStackBeforeInspector');
622+
623+
// These are supposed to be called only on fatal exceptions before
624+
// the process exits.
625+
const fatalExceptionStackEnhancers = {
626+
beforeInspector(error) {
627+
if (typeof error[kEnhanceStackBeforeInspector] !== 'function') {
628+
return error.stack;
629+
}
630+
631+
try {
632+
// Set the error.stack here so it gets picked up by the
633+
// inspector.
634+
error.stack = error[kEnhanceStackBeforeInspector]();
635+
} catch {
636+
// We are just enhancing the error. If it fails, ignore it.
637+
}
638+
return error.stack;
639+
},
640+
afterInspector(error) {
641+
const originalStack = error.stack;
642+
const {
643+
inspect,
644+
inspectDefaultOptions: {
645+
colors: defaultColors
646+
}
647+
} = lazyInternalUtilInspect();
648+
const colors = internalBinding('util').guessHandleType(2) === 'TTY' &&
649+
require('internal/tty').hasColors() ||
650+
defaultColors;
651+
try {
652+
return inspect(error, { colors });
653+
} catch {
654+
return originalStack;
655+
}
656+
}
657+
};
658+
620659
module.exports = {
621660
addCodeToName, // Exported for NghttpError
622661
codes,
@@ -633,6 +672,8 @@ module.exports = {
633672
// This is exported only to facilitate testing.
634673
E,
635674
prepareStackTrace,
675+
kEnhanceStackBeforeInspector,
676+
fatalExceptionStackEnhancers
636677
};
637678

638679
// To declare an error message, use the E(sym, val, def) function above. The sym
Collapse file

‎lib/internal/process/execution.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/execution.js
-12Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,6 @@ function createOnGlobalUncaughtException() {
150150
} else if (!process.emit('uncaughtException', er, type)) {
151151
// If someone handled it, then great. Otherwise, die in C++ land
152152
// since that means that we'll exit the process, emit the 'exit' event.
153-
const { inspect } = require('internal/util/inspect');
154-
const colors = internalBinding('util').guessHandleType(2) === 'TTY' &&
155-
require('internal/tty').hasColors() ||
156-
inspect.defaultOptions.colors;
157153
try {
158154
if (!process._exiting) {
159155
process._exiting = true;
@@ -163,14 +159,6 @@ function createOnGlobalUncaughtException() {
163159
} catch {
164160
// Nothing to be done about it at this point.
165161
}
166-
try {
167-
const { kExpandStackSymbol } = require('internal/util');
168-
if (typeof er[kExpandStackSymbol] === 'function')
169-
er[kExpandStackSymbol]();
170-
er.stack = inspect(er, { colors });
171-
} catch {
172-
// Nothing to be done about it at this point.
173-
}
174162
return false;
175163
}
176164

Collapse file

‎lib/internal/util.js‎

Copy file name to clipboardExpand all lines: lib/internal/util.js
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,5 @@ module.exports = {
414414
// Used by the buffer module to capture an internal reference to the
415415
// default isEncoding implementation, just in case userland overrides it.
416416
kIsEncodingSymbol: Symbol('kIsEncodingSymbol'),
417-
kExpandStackSymbol: Symbol('kExpandStackSymbol'),
418417
kVmBreakFirstLineSymbol: Symbol('kVmBreakFirstLineSymbol')
419418
};
Collapse file

‎lib/internal/util/inspect.js‎

Copy file name to clipboardExpand all lines: lib/internal/util/inspect.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1658,5 +1658,6 @@ function formatWithOptions(inspectOptions, ...args) {
16581658
module.exports = {
16591659
inspect,
16601660
format,
1661-
formatWithOptions
1661+
formatWithOptions,
1662+
inspectDefaultOptions
16621663
};
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,8 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
386386
V(crypto_key_object_constructor, v8::Function) \
387387
V(domain_callback, v8::Function) \
388388
V(domexception_function, v8::Function) \
389+
V(enhance_fatal_stack_after_inspector, v8::Function) \
390+
V(enhance_fatal_stack_before_inspector, v8::Function) \
389391
V(fs_use_promises_symbol, v8::Symbol) \
390392
V(host_import_module_dynamically_callback, v8::Function) \
391393
V(host_initialize_import_meta_object_callback, v8::Function) \
Collapse file

‎src/inspector_agent.cc‎

Copy file name to clipboardExpand all lines: src/inspector_agent.cc
+4-3Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ class NodeInspectorClient : public V8InspectorClient {
570570
}
571571
}
572572

573-
void FatalException(Local<Value> error, Local<Message> message) {
573+
void ReportUncaughtException(Local<Value> error, Local<Message> message) {
574574
Isolate* isolate = env_->isolate();
575575
Local<Context> context = env_->context();
576576

@@ -836,10 +836,11 @@ void Agent::WaitForDisconnect() {
836836
}
837837
}
838838

839-
void Agent::FatalException(Local<Value> error, Local<Message> message) {
839+
void Agent::ReportUncaughtException(Local<Value> error,
840+
Local<Message> message) {
840841
if (!IsListening())
841842
return;
842-
client_->FatalException(error, message);
843+
client_->ReportUncaughtException(error, message);
843844
WaitForDisconnect();
844845
}
845846

Collapse file

‎src/inspector_agent.h‎

Copy file name to clipboardExpand all lines: src/inspector_agent.h
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ class Agent {
6565
void WaitForConnect();
6666
// Blocks till all the sessions with "WaitForDisconnectOnShutdown" disconnect
6767
void WaitForDisconnect();
68-
void FatalException(v8::Local<v8::Value> error,
69-
v8::Local<v8::Message> message);
68+
void ReportUncaughtException(v8::Local<v8::Value> error,
69+
v8::Local<v8::Message> message);
7070

7171
// Async stack traces instrumentation.
7272
void AsyncTaskScheduled(const v8_inspector::StringView& taskName, void* task,

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.