src: pass along errors from process obj instantiation

PR-URL: #25734 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
nodejs · Jan 29, 2019 · 95571ac · 95571ac
1 parent 4dbff09
commit 95571ac
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 49 deletions.
diff --git a/src/env.cc b/src/env.cc
@@ -289,29 +289,10 @@ Environment::~Environment() {
   }
 }
 
-void Environment::Start(const std::vector<std::string>& args,
-                        const std::vector<std::string>& 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<uv_handle_t*>(timer_handle()));
 
@@ -346,14 +327,37 @@ void Environment::Start(const std::vector<std::string>& args,
     StartProfilerIdleNotifier();
   }
 
-  Local<Object> 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<Object> Environment::CreateProcessObject(
+    const std::vector<std::string>& args,
+    const std::vector<std::string>& 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<Object> process_object =
+      node::CreateProcessObject(this, args, exec_args)
+          .FromMaybe(Local<Object>());
+  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
@@ -612,9 +612,10 @@ class Environment {
               v8::Local<v8::Context> context);
   ~Environment();
 
-  void Start(const std::vector<std::string>& args,
-             const std::vector<std::string>& exec_args,
-             bool start_profiler_idle_notifier);
+  void Start(bool start_profiler_idle_notifier);
+  v8::MaybeLocal<v8::Object> CreateProcessObject(
+      const std::vector<std::string>& args,
+      const std::vector<std::string>& exec_args);
 
   typedef void (*HandleCleanupCb)(Environment* env,
                                   uv_handle_t* handle,

diff --git a/src/node.cc b/src/node.cc
@@ -1179,7 +1179,8 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
   std::vector<std::string> args(argv, argv + argc);
   std::vector<std::string> 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> 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
@@ -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<Array>& info) {
   info.GetReturnValue().Set(envarr);
 }
 
-Local<Object> CreateEnvVarProxy(Local<Context> context,
-                                Isolate* isolate,
-                                Local<Value> data) {
+MaybeLocal<Object> CreateEnvVarProxy(Local<Context> context,
+                                     Isolate* isolate,
+                                     Local<Value> data) {
   EscapableHandleScope scope(isolate);
   Local<ObjectTemplate> env_proxy_template = ObjectTemplate::New(isolate);
   env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration(
       EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data));
-  Local<Object> 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
@@ -7,9 +7,9 @@
 
 namespace node {
 
-v8::Local<v8::Object> CreateEnvVarProxy(v8::Local<v8::Context> context,
-                                        v8::Isolate* isolate,
-                                        v8::Local<v8::Value> data);
+v8::MaybeLocal<v8::Object> CreateEnvVarProxy(v8::Local<v8::Context> context,
+                                             v8::Isolate* isolate,
+                                             v8::Local<v8::Value> 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<bool> ProcessEmitDeprecationWarning(Environment* env,
                                               const char* warning,
                                               const char* deprecation_code);
 
-v8::Local<v8::Object> CreateProcessObject(
+v8::MaybeLocal<v8::Object> CreateProcessObject(
     Environment* env,
     const std::vector<std::string>& args,
     const std::vector<std::string>& exec_args);

diff --git 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<Name> property,
   info.GetReturnValue().Set(uv_os_getppid());
 }
 
-Local<Object> CreateProcessObject(Environment* env,
-                                  const std::vector<std::string>& args,
-                                  const std::vector<std::string>& exec_args) {
+MaybeLocal<Object> CreateProcessObject(
+    Environment* env,
+    const std::vector<std::string>& args,
+    const std::vector<std::string>& exec_args) {
   Isolate* isolate = env->isolate();
   EscapableHandleScope scope(isolate);
   Local<Context> context = env->context();
 
   Local<FunctionTemplate> process_template = FunctionTemplate::New(isolate);
   process_template->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "process"));
-  Local<Object> process = process_template->GetFunction(context)
-                              .ToLocalChecked()
-                              ->NewInstance(context)
-                              .ToLocalChecked();
+  Local<Function> process_ctor;
+  Local<Object> process;
+  if (!process_template->GetFunction(context).ToLocal(&process_ctor) ||
+      !process_ctor->NewInstance(context).ToLocal(&process)) {
+    return MaybeLocal<Object>();
+  }
 
   // process.title
   auto title_string = FIXED_ONE_BYTE_STRING(env->isolate(), "title");
@@ -145,11 +149,16 @@ Local<Object> CreateProcessObject(Environment* env,
                ToV8Value(env->context(), exec_args)
                    .ToLocalChecked()).FromJust();
 
+  Local<Object> env_var_proxy;
+  if (!CreateEnvVarProxy(context, isolate, env->as_external())
+           .ToLocal(&env_var_proxy))
+    return MaybeLocal<Object>();
+
   // 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
@@ -132,9 +132,9 @@ Worker::Worker(Environment* env,
     env_->set_worker_context(this);
     env_->set_thread_id(thread_id_);
 
-    env_->Start(std::vector<std::string>{},
-                std::vector<std::string>{},
-                env->profiler_idle_notifier_started());
+    env_->Start(env->profiler_idle_notifier_started());
+    env_->CreateProcessObject(std::vector<std::string>{},
+                              std::vector<std::string>{});
     // Done while on the parent thread
     AddWorkerInspector(env, env_.get(), thread_id_, url_);
   }