From 6f7005465a0c9bc0ce9764e1c0d4299eb22a3480 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Tue, 19 Mar 2019 18:00:16 -0500 Subject: [PATCH] src, lib: take control of prepareStackTrace Refs https://crbug.com/v8/7848 PR-URL: https://github.com/nodejs/node/pull/23926 Reviewed-By: Refael Ackermann Reviewed-By: Joyee Cheung --- lib/internal/bootstrap/node.js | 8 ++++++ lib/internal/errors.js | 28 +++++++++++++++++- src/api/environment.cc | 38 +++++++++++++++++++++++++ src/env.h | 1 + src/node_binding.cc | 1 + src/node_errors.cc | 18 ++++++++++++ test/parallel/test-bootstrap-modules.js | 1 + 7 files changed, 94 insertions(+), 1 deletion(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 321bfabe027d65..8dee38a0e71698 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -35,6 +35,8 @@ // passed by node::RunBootstrapping() /* global process, require, internalBinding, isMainThread, ownsProcessState */ +setupPrepareStackTrace(); + const { JSON, Object, Symbol } = primordials; const config = internalBinding('config'); const { deprecate } = require('internal/util'); @@ -290,6 +292,12 @@ process.emitWarning = emitWarning; // Note: only after this point are the timers effective } +function setupPrepareStackTrace() { + const { setPrepareStackTraceCallback } = internalBinding('errors'); + const { prepareStackTrace } = require('internal/errors'); + setPrepareStackTraceCallback(prepareStackTrace); +} + function setupProcessObject() { const EventEmitter = require('events'); const origProcProto = Object.getPrototypeOf(process); diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 52587f1f4e0971..266358310bf3f7 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -19,6 +19,31 @@ const codes = {}; const { kMaxLength } = internalBinding('buffer'); +const MainContextError = Error; +const ErrorToString = Error.prototype.toString; +// Polyfill of V8's Error.prepareStackTrace API. +// https://crbug.com/v8/7848 +const prepareStackTrace = (globalThis, error, trace) => { + // `globalThis` is the global that contains the constructor which + // created `error`. + if (typeof globalThis.Error.prepareStackTrace === 'function') { + return globalThis.Error.prepareStackTrace(error, trace); + } + // We still have legacy usage that depends on the main context's `Error` + // being used, even when the error is from a different context. + // TODO(devsnek): evaluate if this can be eventually deprecated/removed. + if (typeof MainContextError.prepareStackTrace === 'function') { + return MainContextError.prepareStackTrace(error, trace); + } + + const errorString = ErrorToString.call(error); + if (trace.length === 0) { + return errorString; + } + return `${errorString}\n at ${trace.join('\n at ')}`; +}; + + let excludedStackFn; // Lazily loaded @@ -598,7 +623,8 @@ module.exports = { uvExceptionWithHostPort, SystemError, // This is exported only to facilitate testing. - E + E, + prepareStackTrace, }; // To declare an error message, use the E(sym, val, def) function above. The sym diff --git a/src/api/environment.cc b/src/api/environment.cc index 548b685dfe424a..5dfac00647eddd 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -13,6 +13,8 @@ #endif namespace node { +using errors::TryCatchScope; +using v8::Array; using v8::Context; using v8::EscapableHandleScope; using v8::Function; @@ -45,6 +47,41 @@ static bool ShouldAbortOnUncaughtException(Isolate* isolate) { !env->inside_should_not_abort_on_uncaught_scope(); } +static MaybeLocal PrepareStackTraceCallback(Local context, + Local exception, + Local trace) { + Environment* env = Environment::GetCurrent(context); + if (env == nullptr) { + MaybeLocal s = exception->ToString(context); + return s.IsEmpty() ? + MaybeLocal() : + MaybeLocal(s.ToLocalChecked()); + } + Local prepare = env->prepare_stack_trace_callback(); + if (prepare.IsEmpty()) { + MaybeLocal s = exception->ToString(context); + return s.IsEmpty() ? + MaybeLocal() : + MaybeLocal(s.ToLocalChecked()); + } + Local args[] = { + context->Global(), + exception, + trace, + }; + // This TryCatch + Rethrow is required by V8 due to details around exception + // handling there. For C++ callbacks, V8 expects a scheduled exception (which + // is what ReThrow gives us). Just returning the empty MaybeLocal would leave + // us with a pending exception. + TryCatchScope try_catch(env); + MaybeLocal result = prepare->Call( + context, Undefined(env->isolate()), arraysize(args), args); + if (try_catch.HasCaught() && !try_catch.HasTerminated()) { + try_catch.ReThrow(); + } + return result; +} + void* NodeArrayBufferAllocator::Allocate(size_t size) { if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers) return UncheckedCalloc(size); @@ -166,6 +203,7 @@ void SetIsolateUpForNode(v8::Isolate* isolate, IsolateSettingCategories cat) { isolate->SetAbortOnUncaughtExceptionCallback( ShouldAbortOnUncaughtException); isolate->SetFatalErrorHandler(OnFatalError); + isolate->SetPrepareStackTraceCallback(PrepareStackTraceCallback); break; case IsolateSettingCategories::kMisc: isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit); diff --git a/src/env.h b/src/env.h index 7b7e9f61320a36..5544ac44db5f8a 100644 --- a/src/env.h +++ b/src/env.h @@ -402,6 +402,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2; V(native_module_require, v8::Function) \ V(performance_entry_callback, v8::Function) \ V(performance_entry_template, v8::Function) \ + V(prepare_stack_trace_callback, v8::Function) \ V(process_object, v8::Object) \ V(primordials, v8::Object) \ V(promise_reject_callback, v8::Function) \ diff --git a/src/node_binding.cc b/src/node_binding.cc index 9a9bfa70a8a378..99c2406036e187 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -48,6 +48,7 @@ V(contextify) \ V(credentials) \ V(domain) \ + V(errors) \ V(fs) \ V(fs_event_wrap) \ V(heap_utils) \ diff --git a/src/node_errors.cc b/src/node_errors.cc index 1cd90523830fc9..603c9e415c3f54 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -17,6 +17,7 @@ using v8::Boolean; using v8::Context; using v8::Exception; using v8::Function; +using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Int32; using v8::Isolate; @@ -767,6 +768,21 @@ void PerIsolateMessageListener(Local message, Local error) { } } +void SetPrepareStackTraceCallback(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + CHECK(args[0]->IsFunction()); + env->set_prepare_stack_trace_callback(args[0].As()); +} + +void Initialize(Local target, + Local unused, + Local context, + void* priv) { + Environment* env = Environment::GetCurrent(context); + env->SetMethod( + target, "setPrepareStackTraceCallback", SetPrepareStackTraceCallback); +} + } // namespace errors void DecorateErrorStack(Environment* env, @@ -880,3 +896,5 @@ void FatalException(Isolate* isolate, } } // namespace node + +NODE_MODULE_CONTEXT_AWARE_INTERNAL(errors, node::errors::Initialize) diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 50b0a53724da94..f96aba36863b7c 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -9,6 +9,7 @@ const common = require('../common'); const assert = require('assert'); const expectedModules = new Set([ + 'Internal Binding errors', 'Internal Binding async_wrap', 'Internal Binding buffer', 'Internal Binding config',