Make Error.prototype.toString spec compliant; and fix various side-effect-free error printing metho…

src/messages.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/messages.h"
 
 #include "src/api.h"
 #include "src/execution.h"
 #include "src/isolate-inl.h"
 #include "src/string-builder.h"
@@ skipping 64 matching lines @@
   Handle<Object> exception(exception_object, isolate);
 
   Isolate::ExceptionScope exception_scope(isolate);
   isolate->clear_pending_exception();
   isolate->set_external_caught_exception(false);
 
   // Turn the exception on the message into a string if it is an object.
   if (message->argument()->IsJSObject()) {
     HandleScope scope(isolate);
     Handle<Object> argument(message->argument(), isolate);
-    Handle<Object> args[] = {argument};
-    MaybeHandle<Object> maybe_stringified = Execution::TryCall(
-        isolate, isolate->to_detail_string_fun(),
-        isolate->factory()->undefined_value(), arraysize(args), args);
+
+    MaybeHandle<Object> maybe_stringified;
     Handle<Object> stringified;
+    // Make sure we don't leak uncaught internally generated Error objects.
+    if (Object::IsErrorObject(isolate, argument)) {

[Yang, 2015/12/09 15:08:10]

this also happens if the user creates an Error object via new Error, right? so
this doesnt guarantee that it's an internally thrown error.

we *could* do the distinction though, if we want, e.g. by installing some
private symbol on the code path for internally thrown errors...

+      Handle<Object> args[] = {argument};
+      maybe_stringified = Execution::TryCall(
+          isolate, isolate->no_side_effects_to_string_fun(),
+          isolate->factory()->undefined_value(), arraysize(args), args);
+    } else {
+      v8::TryCatch catcher(reinterpret_cast<v8::Isolate*>(isolate));
+      catcher.SetVerbose(false);
+      catcher.SetCaptureMessage(false);
+
+      maybe_stringified = Object::ToString(isolate, argument);
+    }
+
     if (!maybe_stringified.ToHandle(&stringified)) {
       stringified = isolate->factory()->NewStringFromAsciiChecked("exception");
     }
     message->set_argument(*stringified);
   }
 
   v8::Local<v8::Message> api_message_obj = v8::Utils::MessageToLocal(message);
   v8::Local<v8::Value> api_exception_obj = v8::Utils::ToLocal(exception);
 
   v8::NeanderArray global_listeners(isolate->factory()->message_listeners());
@@ skipping 210 matching lines @@
 
 
 Handle<String> MessageTemplate::FormatMessage(Isolate* isolate,
                                               int template_index,
                                               Handle<Object> arg) {
   Factory* factory = isolate->factory();
   Handle<String> result_string;
   if (arg->IsString()) {
     result_string = Handle<String>::cast(arg);
   } else {
-    Handle<JSFunction> fun = isolate->no_side_effect_to_string_fun();
+    Handle<JSFunction> fun = isolate->no_side_effects_to_string_fun();
 
     MaybeHandle<Object> maybe_result =
         Execution::TryCall(isolate, fun, factory->undefined_value(), 1, &arg);
     Handle<Object> result;
     if (!maybe_result.ToHandle(&result) || !result->IsString()) {
       return factory->InternalizeOneByteString(STATIC_CHAR_VECTOR("<error>"));
     }
     result_string = Handle<String>::cast(result);
   }
   MaybeHandle<String> maybe_result_string = MessageTemplate::FormatMessage(
@@ skipping 53 matching lines @@
       }
     } else {
       builder.AppendCharacter(*c);
     }
   }
 
   return builder.Finish();
 }
 
 
-MaybeHandle<String> ErrorToStringHelper::Stringify(Isolate* isolate,
-                                                   Handle<JSObject> error) {
-  VisitedScope scope(this, error);
-  if (scope.has_visited()) return isolate->factory()->empty_string();
-
-  Handle<String> name;
-  Handle<String> message;
-  Handle<Name> internal_key = isolate->factory()->internal_error_symbol();
-  Handle<String> message_string =
-      isolate->factory()->NewStringFromStaticChars("message");
-  Handle<String> name_string = isolate->factory()->name_string();
-  LookupIterator internal_error_lookup(
-      error, internal_key, LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
-
-  // Find out whether an internally created error object is on the prototype
-  // chain. If the name property is found on a holder prior to the internally
-  // created error object, use that name property. Otherwise just use the
-  // constructor name to avoid triggering possible side effects.
-  // Similar for the message property. If the message property shadows the
-  // internally created error object, use that message property. Otherwise
-  // use empty string as message.
-  LookupIterator name_lookup(error, name_string,
-                             LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
-  if (internal_error_lookup.IsFound() &&
-      !ShadowsInternalError(isolate, &name_lookup, &internal_error_lookup)) {
-    Handle<JSObject> holder = internal_error_lookup.GetHolder<JSObject>();
-    name = JSReceiver::GetConstructorName(holder);
-  } else {
-    ASSIGN_RETURN_ON_EXCEPTION(
-        isolate, name,
-        GetStringifiedProperty(isolate, &name_lookup,
-                               isolate->factory()->Error_string()),
-        String);
-  }
-
-  LookupIterator message_lookup(
-      error, message_string, LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
-  if (internal_error_lookup.IsFound() &&
-      !ShadowsInternalError(isolate, &message_lookup, &internal_error_lookup)) {
-    message = isolate->factory()->empty_string();
-  } else {
-    ASSIGN_RETURN_ON_EXCEPTION(
-        isolate, message,
-        GetStringifiedProperty(isolate, &message_lookup,
-                               isolate->factory()->empty_string()),
-        String);
-  }
-
-  if (name->length() == 0) return message;
-  if (message->length() == 0) return name;
-  IncrementalStringBuilder builder(isolate);
-  builder.AppendString(name);
-  builder.AppendCString(": ");
-  builder.AppendString(message);
-  return builder.Finish();
-}
-
-
-bool ErrorToStringHelper::ShadowsInternalError(
-    Isolate* isolate, LookupIterator* property_lookup,
-    LookupIterator* internal_error_lookup) {
-  if (!property_lookup->IsFound()) return false;
-  Handle<JSObject> holder = property_lookup->GetHolder<JSObject>();
-  // It's fine if the property is defined on the error itself.
-  if (holder.is_identical_to(property_lookup->GetReceiver())) return true;
-  PrototypeIterator it(isolate, holder, PrototypeIterator::START_AT_RECEIVER);
-  while (true) {
-    if (it.IsAtEnd()) return false;
-    if (it.IsAtEnd(internal_error_lookup->GetHolder<JSObject>())) return true;
-    it.AdvanceIgnoringProxies();
-  }
-}
-
-
-MaybeHandle<String> ErrorToStringHelper::GetStringifiedProperty(
-    Isolate* isolate, LookupIterator* property_lookup,
-    Handle<String> default_value) {
-  if (!property_lookup->IsFound()) return default_value;
-  Handle<Object> obj;
-  ASSIGN_RETURN_ON_EXCEPTION(isolate, obj, Object::GetProperty(property_lookup),
-                             String);
-  if (obj->IsUndefined()) return default_value;
-  if (!obj->IsString()) {
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, obj, Object::ToString(isolate, obj),
-                               String);
-  }
-  return Handle<String>::cast(obj);
-}
-
 }  // namespace internal
 }  // namespace v8