Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

take ownership of prepareStackTrace for upstream v8 changes #23926

Merged
merged 1 commit into from
May 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 27 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#endif

namespace node {
using errors::TryCatchScope;
using v8::Array;
using v8::Context;
using v8::EscapableHandleScope;
using v8::Function;
Expand Down Expand Up @@ -45,6 +47,41 @@ static bool ShouldAbortOnUncaughtException(Isolate* isolate) {
!env->inside_should_not_abort_on_uncaught_scope();
}

static MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
Local<Value> exception,
Local<Array> trace) {
Environment* env = Environment::GetCurrent(context);
if (env == nullptr) {
MaybeLocal<String> s = exception->ToString(context);
return s.IsEmpty() ?
MaybeLocal<Value>() :
MaybeLocal<Value>(s.ToLocalChecked());
}
Local<Function> prepare = env->prepare_stack_trace_callback();
if (prepare.IsEmpty()) {
MaybeLocal<String> s = exception->ToString(context);
return s.IsEmpty() ?
MaybeLocal<Value>() :
MaybeLocal<Value>(s.ToLocalChecked());
}
Local<Value> 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);
devsnek marked this conversation as resolved.
Show resolved Hide resolved
MaybeLocal<Value> 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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
1 change: 1 addition & 0 deletions src/node_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
V(contextify) \
V(credentials) \
V(domain) \
V(errors) \
V(fs) \
V(fs_event_wrap) \
V(heap_utils) \
Expand Down
18 changes: 18 additions & 0 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -767,6 +768,21 @@ void PerIsolateMessageListener(Local<Message> message, Local<Value> error) {
}
}

void SetPrepareStackTraceCallback(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK(args[0]->IsFunction());
env->set_prepare_stack_trace_callback(args[0].As<Function>());
}

void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
env->SetMethod(
target, "setPrepareStackTraceCallback", SetPrepareStackTraceCallback);
}

} // namespace errors

void DecorateErrorStack(Environment* env,
Expand Down Expand Up @@ -880,3 +896,5 @@ void FatalException(Isolate* isolate,
}

} // namespace node

NODE_MODULE_CONTEXT_AWARE_INTERNAL(errors, node::errors::Initialize)
1 change: 1 addition & 0 deletions test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down