Client errors without a stack property should try to include the message

This CL changes {descriptionForError} to not immediately return when a {stack}
is not found, but instead try to lookup and append the {message} as well.

The existing logic to build a description in a specific way when the class
of the exception does not match, is retained for backwards compatibility.

Bug: chromium:954017
Change-Id: I9fa1d2807e2877bd988f82b4b57cf329bcd9f61b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1738862
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63111}

src/inspector/DEPS

@@ -6,6 +6,7 @@ include_rules = [
   "+src/base/logging.h",
   "+src/base/macros.h",
   "+src/base/memory.h",
+  "+src/base/optional.h",
   "+src/base/platform/platform.h",
   "+src/base/platform/mutex.h",
   "+src/base/safe_conversions.h",

src/inspector/value-mirror.cc

@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <cmath>
 
+#include "src/base/optional.h"
 #include "src/debug/debug-interface.h"
 #include "src/inspector/v8-debugger.h"
 #include "src/inspector/v8-inspector-impl.h"
@@ -199,37 +200,57 @@ String16 descriptionForRegExp(v8::Isolate* isolate,
 
 enum class ErrorType { kNative, kClient };
 
+// Build a description from an exception using the following rules:
+//   * Usually return the stack trace found in the {stack} property.
+//   * If the stack trace does not start with the class name of the passed
+//     exception, try to build a description from the class name, the
+//     {message} property and the rest of the stack trace.
+//     (The stack trace is only used if {message} was also found in
+//     said stack trace).
 String16 descriptionForError(v8::Local<v8::Context> context,
                              v8::Local<v8::Object> object, ErrorType type) {
   v8::Isolate* isolate = context->GetIsolate();
   v8::TryCatch tryCatch(isolate);
   String16 className = toProtocolString(isolate, object->GetConstructorName());
-  v8::Local<v8::Value> stackValue;
-  if (!object->Get(context, toV8String(isolate, "stack"))
-           .ToLocal(&stackValue) ||
-      !stackValue->IsString()) {
-    return className;
-  }
-  String16 stack = toProtocolString(isolate, stackValue.As<v8::String>());
-  String16 description = stack;
-  if (type == ErrorType::kClient) {
-    if (stack.substring(0, className.length()) != className) {
-      v8::Local<v8::Value> messageValue;
-      if (!object->Get(context, toV8String(isolate, "message"))
-               .ToLocal(&messageValue) ||
-          !messageValue->IsString()) {
-        return stack;
-      }
-      String16 message = toProtocolStringWithTypeCheck(isolate, messageValue);
-      size_t index = stack.find(message);
-      String16 stackWithoutMessage =
-          index != String16::kNotFound
-              ? stack.substring(index + message.length())
-              : String16();
-      description = className + ": " + message + stackWithoutMessage;
+
+  v8::base::Optional<String16> stack;
+  {
+    v8::Local<v8::Value> stackValue;
+    if (object->Get(context, toV8String(isolate, "stack"))
+            .ToLocal(&stackValue) &&
+        stackValue->IsString()) {
+      stack = toProtocolString(isolate, stackValue.As<v8::String>());
     }
   }
-  return description;
+
+  if (type == ErrorType::kNative && stack) return *stack;
+
+  if (stack && stack->substring(0, className.length()) == className) {
+    return *stack;
+  }
+
+  v8::base::Optional<String16> message;
+  {
+    v8::Local<v8::Value> messageValue;
+    if (object->Get(context, toV8String(isolate, "message"))
+            .ToLocal(&messageValue) &&
+        messageValue->IsString()) {
+      String16 msg = toProtocolStringWithTypeCheck(isolate, messageValue);
+      if (!msg.isEmpty()) message = msg;
+    }
+  }
+
+  if (!message) return stack ? *stack : className;
+
+  String16 description = className + ": " + *message;
+  if (!stack) return description;
+
+  DCHECK(stack && message);
+  size_t index = stack->find(*message);
+  String16 stackWithoutMessage =
+      index != String16::kNotFound ? stack->substring(index + message->length())
+                                   : String16();
+  return description + stackWithoutMessage;
 }
 
 String16 descriptionForObject(v8::Isolate* isolate,

test/inspector/runtime/remote-object-expected.txt

@@ -1436,6 +1436,18 @@ Running test: testCustomError
     }
 }
 
+Running test: testCustomErrorWithMessage
+'class CustomMsgError extends Error {}; a = new CustomMsgError(); delete a.stack; a.message = 'foobar'; a', returnByValue: false, generatePreview: false
+{
+    result : {
+        className : CustomMsgError
+        description : CustomMsgError: foobar
+        objectId : <objectId>
+        subtype : error
+        type : object
+    }
+}
+
 Running test: testProxy
 'new Proxy({}, {})', returnByValue: false, generatePreview: false
 {

test/inspector/runtime/remote-object.js

@@ -419,6 +419,11 @@ InspectorTest.runAsyncTestSuite([
       expression: `class CustomError extends Error {}; a = new CustomError(); delete a.stack; a`
     })).result);
   },
+  async function testCustomErrorWithMessage() {
+    InspectorTest.logMessage((await evaluate( {
+      expression: `class CustomMsgError extends Error {}; a = new CustomMsgError(); delete a.stack; a.message = 'foobar'; a`
+    })).result);
+  },
   async function testProxy() {
     InspectorTest.logMessage((await evaluate({
       expression: 'new Proxy({}, {})'