From 84358b501031cf3fdc3a545e1b36995c8c6b6abf Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 30 Jan 2019 23:26:04 +0100
Subject: [PATCH] src: handle errors while printing error objects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Handle situations where accessing `.name` or `.stack` on an object
fails.

Fixes: https://github.com/nodejs/node/issues/25718

PR-URL: https://github.com/nodejs/node/pull/25834
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 src/node_errors.cc                            | 35 ++++++++++---------
 test/message/throw_error_with_getter_throw.js | 10 ++++++
 .../message/throw_error_with_getter_throw.out |  5 +++
 3 files changed, 33 insertions(+), 17 deletions(-)
 create mode 100644 test/message/throw_error_with_getter_throw.js
 create mode 100644 test/message/throw_error_with_getter_throw.out

diff --git a/src/node_errors.cc b/src/node_errors.cc
index c5e0f4a3269370..8fb0e0122f542d 100644
--- a/src/node_errors.cc
+++ b/src/node_errors.cc
@@ -9,6 +9,7 @@
 
 namespace node {
 
+using errors::TryCatchScope;
 using v8::Context;
 using v8::Exception;
 using v8::Function;
@@ -201,8 +202,10 @@ void ReportException(Environment* env,
   } else {
     Local<Object> err_obj = er->ToObject(env->context()).ToLocalChecked();
 
-    trace_value = err_obj->Get(env->context(),
-                               env->stack_string()).ToLocalChecked();
+    if (!err_obj->Get(env->context(), env->stack_string())
+             .ToLocal(&trace_value)) {
+      trace_value = Undefined(env->isolate());
+    }
     arrow =
         err_obj->GetPrivate(env->context(), env->arrow_message_private_symbol())
             .ToLocalChecked();
@@ -222,27 +225,25 @@ void ReportException(Environment* env,
     // this really only happens for RangeErrors, since they're the only
     // kind that won't have all this info in the trace, or when non-Error
     // objects are thrown manually.
-    Local<Value> message;
-    Local<Value> name;
+    MaybeLocal<Value> message;
+    MaybeLocal<Value> name;
 
     if (er->IsObject()) {
       Local<Object> err_obj = er.As<Object>();
-      message = err_obj->Get(env->context(),
-                             env->message_string()).ToLocalChecked();
-      name = err_obj->Get(env->context(),
-          FIXED_ONE_BYTE_STRING(env->isolate(), "name")).ToLocalChecked();
+      message = err_obj->Get(env->context(), env->message_string());
+      name = err_obj->Get(env->context(), env->name_string());
     }
 
-    if (message.IsEmpty() || message->IsUndefined() || name.IsEmpty() ||
-        name->IsUndefined()) {
+    if (message.IsEmpty() || message.ToLocalChecked()->IsUndefined() ||
+        name.IsEmpty() || name.ToLocalChecked()->IsUndefined()) {
       // Not an error object. Just print as-is.
       String::Utf8Value message(env->isolate(), er);
 
       PrintErrorString("%s\n",
                        *message ? *message : "<toString() threw exception>");
     } else {
-      node::Utf8Value name_string(env->isolate(), name);
-      node::Utf8Value message_string(env->isolate(), message);
+      node::Utf8Value name_string(env->isolate(), name.ToLocalChecked());
+      node::Utf8Value message_string(env->isolate(), message.ToLocalChecked());
 
       if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
         PrintErrorString("%s: %s\n", *name_string, *message_string);
@@ -681,8 +682,8 @@ void DecorateErrorStack(Environment* env,
   if (IsExceptionDecorated(env, err_obj)) return;
 
   AppendExceptionLine(env, exception, try_catch.Message(), CONTEXTIFY_ERROR);
-  Local<Value> stack =
-      err_obj->Get(env->context(), env->stack_string()).ToLocalChecked();
+  TryCatchScope try_catch_scope(env);  // Ignore exceptions below.
+  MaybeLocal<Value> stack = err_obj->Get(env->context(), env->stack_string());
   MaybeLocal<Value> maybe_value =
       err_obj->GetPrivate(env->context(), env->arrow_message_private_symbol());
 
@@ -691,7 +692,7 @@ void DecorateErrorStack(Environment* env,
     return;
   }
 
-  if (stack.IsEmpty() || !stack->IsString()) {
+  if (stack.IsEmpty() || !stack.ToLocalChecked()->IsString()) {
     return;
   }
 
@@ -700,8 +701,8 @@ void DecorateErrorStack(Environment* env,
       String::Concat(env->isolate(),
                      arrow.As<String>(),
                      FIXED_ONE_BYTE_STRING(env->isolate(), "\n")),
-      stack.As<String>());
-  err_obj->Set(env->context(), env->stack_string(), decorated_stack).FromJust();
+      stack.ToLocalChecked().As<String>());
+  USE(err_obj->Set(env->context(), env->stack_string(), decorated_stack));
   err_obj->SetPrivate(
       env->context(), env->decorated_private_symbol(), True(env->isolate()));
 }
diff --git a/test/message/throw_error_with_getter_throw.js b/test/message/throw_error_with_getter_throw.js
new file mode 100644
index 00000000000000..a807ff3e2b6504
--- /dev/null
+++ b/test/message/throw_error_with_getter_throw.js
@@ -0,0 +1,10 @@
+'use strict';
+require('../common');
+throw {  // eslint-disable-line no-throw-literal
+  get stack() {
+    throw new Error('weird throw but ok');
+  },
+  get name() {
+    throw new Error('weird throw but ok');
+  },
+};
diff --git a/test/message/throw_error_with_getter_throw.out b/test/message/throw_error_with_getter_throw.out
new file mode 100644
index 00000000000000..62d546d0bce862
--- /dev/null
+++ b/test/message/throw_error_with_getter_throw.out
@@ -0,0 +1,5 @@
+
+*:3
+throw {  // eslint-disable-line no-throw-literal
+^
+[object Object]