Skip to content

Commit

Permalink
src: handle errors while printing error objects
Browse files Browse the repository at this point in the history
Handle situations where accessing `.name` or `.stack` on an object
fails.

Fixes: #25718

PR-URL: #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>
  • Loading branch information
addaleax committed Feb 3, 2019
1 parent f027290 commit 84358b5
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 17 deletions.
35 changes: 18 additions & 17 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace node {

using errors::TryCatchScope;
using v8::Context;
using v8::Exception;
using v8::Function;
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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());

Expand All @@ -691,7 +692,7 @@ void DecorateErrorStack(Environment* env,
return;
}

if (stack.IsEmpty() || !stack->IsString()) {
if (stack.IsEmpty() || !stack.ToLocalChecked()->IsString()) {
return;
}

Expand All @@ -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()));
}
Expand Down
10 changes: 10 additions & 0 deletions test/message/throw_error_with_getter_throw.js
Original file line number Diff line number Diff line change
@@ -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');
},
};
5 changes: 5 additions & 0 deletions test/message/throw_error_with_getter_throw.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

*:3
throw { // eslint-disable-line no-throw-literal
^
[object Object]

0 comments on commit 84358b5

Please sign in to comment.