From 95571ac1e9acd09d0b06b2315aabb0cc4e158572 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 27 Jan 2019 14:21:21 +0100 Subject: [PATCH] src: pass along errors from process obj instantiation PR-URL: https://github.com/nodejs/node/pull/25734 Reviewed-By: James M Snell Reviewed-By: Gus Caplan --- src/env.cc | 50 ++++++++++++++++++++------------------ src/env.h | 7 +++--- src/node.cc | 6 +++-- src/node_env_var.cc | 11 ++++----- src/node_process.h | 8 +++--- src/node_process_object.cc | 25 +++++++++++++------ src/node_worker.cc | 6 ++--- 7 files changed, 64 insertions(+), 49 deletions(-) diff --git a/src/env.cc b/src/env.cc index 97cfc72a9dd58e..78ed42f89af9fc 100644 --- a/src/env.cc +++ b/src/env.cc @@ -289,29 +289,10 @@ Environment::~Environment() { } } -void Environment::Start(const std::vector& args, - const std::vector& exec_args, - bool start_profiler_idle_notifier) { +void Environment::Start(bool start_profiler_idle_notifier) { HandleScope handle_scope(isolate()); Context::Scope context_scope(context()); - if (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED( - TRACING_CATEGORY_NODE1(environment)) != 0) { - auto traced_value = tracing::TracedValue::Create(); - traced_value->BeginArray("args"); - for (const std::string& arg : args) - traced_value->AppendString(arg); - traced_value->EndArray(); - traced_value->BeginArray("exec_args"); - for (const std::string& arg : exec_args) - traced_value->AppendString(arg); - traced_value->EndArray(); - TRACE_EVENT_NESTABLE_ASYNC_BEGIN1( - TRACING_CATEGORY_NODE1(environment), - "Environment", this, - "args", std::move(traced_value)); - } - CHECK_EQ(0, uv_timer_init(event_loop(), timer_handle())); uv_unref(reinterpret_cast(timer_handle())); @@ -346,14 +327,37 @@ void Environment::Start(const std::vector& args, StartProfilerIdleNotifier(); } - Local process_object = CreateProcessObject(this, args, exec_args); - set_process_object(process_object); - static uv_once_t init_once = UV_ONCE_INIT; uv_once(&init_once, InitThreadLocalOnce); uv_key_set(&thread_local_env, this); } +MaybeLocal Environment::CreateProcessObject( + const std::vector& args, + const std::vector& exec_args) { + if (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED( + TRACING_CATEGORY_NODE1(environment)) != 0) { + auto traced_value = tracing::TracedValue::Create(); + traced_value->BeginArray("args"); + for (const std::string& arg : args) traced_value->AppendString(arg); + traced_value->EndArray(); + traced_value->BeginArray("exec_args"); + for (const std::string& arg : exec_args) traced_value->AppendString(arg); + traced_value->EndArray(); + TRACE_EVENT_NESTABLE_ASYNC_BEGIN1(TRACING_CATEGORY_NODE1(environment), + "Environment", + this, + "args", + std::move(traced_value)); + } + + Local process_object = + node::CreateProcessObject(this, args, exec_args) + .FromMaybe(Local()); + set_process_object(process_object); + return process_object; +} + void Environment::RegisterHandleCleanups() { HandleCleanupCb close_and_finish = [](Environment* env, uv_handle_t* handle, void* arg) { diff --git a/src/env.h b/src/env.h index 6878f725bf6155..0d44c774184059 100644 --- a/src/env.h +++ b/src/env.h @@ -612,9 +612,10 @@ class Environment { v8::Local context); ~Environment(); - void Start(const std::vector& args, - const std::vector& exec_args, - bool start_profiler_idle_notifier); + void Start(bool start_profiler_idle_notifier); + v8::MaybeLocal CreateProcessObject( + const std::vector& args, + const std::vector& exec_args); typedef void (*HandleCleanupCb)(Environment* env, uv_handle_t* handle, diff --git a/src/node.cc b/src/node.cc index 5b5cff078b423a..881ace6e425a77 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1179,7 +1179,8 @@ Environment* CreateEnvironment(IsolateData* isolate_data, std::vector args(argv, argv + argc); std::vector exec_args(exec_argv, exec_argv + exec_argc); Environment* env = new Environment(isolate_data, context); - env->Start(args, exec_args, per_process::v8_is_profiling); + env->Start(per_process::v8_is_profiling); + env->CreateProcessObject(args, exec_args); return env; } @@ -1253,7 +1254,8 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, Local context = NewContext(isolate); Context::Scope context_scope(context); Environment env(isolate_data, context); - env.Start(args, exec_args, per_process::v8_is_profiling); + env.Start(per_process::v8_is_profiling); + env.CreateProcessObject(args, exec_args); #if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM CHECK(!env.inspector_agent()->IsListening()); diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 60582d7a31c804..d994a2199a5202 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -17,6 +17,7 @@ using v8::EscapableHandleScope; using v8::Integer; using v8::Isolate; using v8::Local; +using v8::MaybeLocal; using v8::Name; using v8::NamedPropertyHandlerConfiguration; using v8::NewStringType; @@ -209,15 +210,13 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { info.GetReturnValue().Set(envarr); } -Local CreateEnvVarProxy(Local context, - Isolate* isolate, - Local data) { +MaybeLocal CreateEnvVarProxy(Local context, + Isolate* isolate, + Local data) { EscapableHandleScope scope(isolate); Local env_proxy_template = ObjectTemplate::New(isolate); env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration( EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data)); - Local env_proxy = - env_proxy_template->NewInstance(context).ToLocalChecked(); - return scope.Escape(env_proxy); + return scope.EscapeMaybe(env_proxy_template->NewInstance(context)); } } // namespace node diff --git a/src/node_process.h b/src/node_process.h index 865223c1634010..452805ada31449 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -7,9 +7,9 @@ namespace node { -v8::Local CreateEnvVarProxy(v8::Local context, - v8::Isolate* isolate, - v8::Local data); +v8::MaybeLocal CreateEnvVarProxy(v8::Local context, + v8::Isolate* isolate, + v8::Local data); // Most of the time, it's best to use `console.error` to write // to the process.stderr stream. However, in some cases, such as @@ -31,7 +31,7 @@ v8::Maybe ProcessEmitDeprecationWarning(Environment* env, const char* warning, const char* deprecation_code); -v8::Local CreateProcessObject( +v8::MaybeLocal CreateProcessObject( Environment* env, const std::vector& args, const std::vector& exec_args); diff --git a/src/node_process_object.cc b/src/node_process_object.cc index 980b5002b69b0b..c1f8806110ffef 100644 --- a/src/node_process_object.cc +++ b/src/node_process_object.cc @@ -19,6 +19,7 @@ using v8::Integer; using v8::Isolate; using v8::Just; using v8::Local; +using v8::MaybeLocal; using v8::Name; using v8::NewStringType; using v8::None; @@ -66,19 +67,22 @@ static void GetParentProcessId(Local property, info.GetReturnValue().Set(uv_os_getppid()); } -Local CreateProcessObject(Environment* env, - const std::vector& args, - const std::vector& exec_args) { +MaybeLocal CreateProcessObject( + Environment* env, + const std::vector& args, + const std::vector& exec_args) { Isolate* isolate = env->isolate(); EscapableHandleScope scope(isolate); Local context = env->context(); Local process_template = FunctionTemplate::New(isolate); process_template->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "process")); - Local process = process_template->GetFunction(context) - .ToLocalChecked() - ->NewInstance(context) - .ToLocalChecked(); + Local process_ctor; + Local process; + if (!process_template->GetFunction(context).ToLocal(&process_ctor) || + !process_ctor->NewInstance(context).ToLocal(&process)) { + return MaybeLocal(); + } // process.title auto title_string = FIXED_ONE_BYTE_STRING(env->isolate(), "title"); @@ -145,11 +149,16 @@ Local CreateProcessObject(Environment* env, ToV8Value(env->context(), exec_args) .ToLocalChecked()).FromJust(); + Local env_var_proxy; + if (!CreateEnvVarProxy(context, isolate, env->as_external()) + .ToLocal(&env_var_proxy)) + return MaybeLocal(); + // process.env process ->Set(env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "env"), - CreateEnvVarProxy(context, isolate, env->as_external())) + env_var_proxy) .FromJust(); READONLY_PROPERTY(process, "pid", diff --git a/src/node_worker.cc b/src/node_worker.cc index cb496005844e21..e5ba438bc1501c 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -132,9 +132,9 @@ Worker::Worker(Environment* env, env_->set_worker_context(this); env_->set_thread_id(thread_id_); - env_->Start(std::vector{}, - std::vector{}, - env->profiler_idle_notifier_started()); + env_->Start(env->profiler_idle_notifier_started()); + env_->CreateProcessObject(std::vector{}, + std::vector{}); // Done while on the parent thread AddWorkerInspector(env, env_.get(), thread_id_, url_); }