Ignore returnByValue when serializing caught value in promise rejections.

Since the same value is also returned in 'result' field it is still populated in accord with 'returnByValue' parameter. This behavior is consistent with 'evaluate'.

R=dgozman@chromium.org, lushnikov@chromium.org

Bug: v8:9509
Change-Id: I9f72682f87492ce5cd0759dce75ab3d75a5fe31c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1707331Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Yury Semikhatsky <yurys@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63134}

src/inspector/injected-script.cc

@@ -50,8 +50,8 @@
 namespace v8_inspector {
 
 namespace {
-static const char kGlobalHandleLabel[] = "DevTools console";
-static bool isResolvableNumberLike(String16 query) {
+const char kGlobalHandleLabel[] = "DevTools console";
+bool isResolvableNumberLike(String16 query) {
   return query == "Infinity" || query == "-Infinity" || query == "NaN";
 }
 }  // namespace
@@ -220,8 +220,13 @@ class InjectedScript::ProtocolPromiseHandler {
                                                       : 0)
             .setColumnNumber(
                 stack && !stack->isEmpty() ? stack->topColumnNumber() : 0)
-            .setException(wrappedValue->clone())
             .build();
+    response = scope.injectedScript()->addExceptionToDetails(
+        result, exceptionDetails.get(), m_objectGroup);
+    if (!response.isSuccess()) {
+      callback->sendFailure(response);
+      return;
+    }
     if (stack)
       exceptionDetails->setStackTrace(
           stack->buildInspectorObjectImpl(m_inspector->debugger()));
@@ -289,8 +294,7 @@ Response InjectedScript::getProperties(
   PropertyAccumulator accumulator(&mirrors);
   if (!ValueMirror::getProperties(context, object, ownProperties,
                                   accessorPropertiesOnly, &accumulator)) {
-    return createExceptionDetails(tryCatch, groupName, wrapMode,
-                                  exceptionDetails);
+    return createExceptionDetails(tryCatch, groupName, exceptionDetails);
   }
   for (const PropertyMirror& mirror : mirrors) {
     std::unique_ptr<PropertyDescriptor> descriptor =
@@ -632,9 +636,25 @@ Response InjectedScript::resolveCallArgument(
   return Response::OK();
 }
 
+Response InjectedScript::addExceptionToDetails(
+    v8::Local<v8::Value> exception,
+    protocol::Runtime::ExceptionDetails* exceptionDetails,
+    const String16& objectGroup) {
+  if (exception.IsEmpty()) return Response::OK();
+  std::unique_ptr<protocol::Runtime::RemoteObject> wrapped;
+  Response response =
+      wrapObject(exception, objectGroup,
+                 exception->IsNativeError() ? WrapMode::kNoPreview
+                                            : WrapMode::kWithPreview,
+                 &wrapped);
+  if (!response.isSuccess()) return response;
+  exceptionDetails->setException(std::move(wrapped));
+  return Response::OK();
+}
+
 Response InjectedScript::createExceptionDetails(
     const v8::TryCatch& tryCatch, const String16& objectGroup,
-    WrapMode wrapMode, Maybe<protocol::Runtime::ExceptionDetails>* result) {
+    Maybe<protocol::Runtime::ExceptionDetails>* result) {
   if (!tryCatch.HasCaught()) return Response::InternalError();
   v8::Local<v8::Message> message = tryCatch.Message();
   v8::Local<v8::Value> exception = tryCatch.Exception();
@@ -667,16 +687,9 @@ Response InjectedScript::createExceptionDetails(
               ->createStackTrace(stackTrace)
               ->buildInspectorObjectImpl(m_context->inspector()->debugger()));
   }
-  if (!exception.IsEmpty()) {
-    std::unique_ptr<protocol::Runtime::RemoteObject> wrapped;
-    Response response =
-        wrapObject(exception, objectGroup,
-                   exception->IsNativeError() ? WrapMode::kNoPreview
-                                              : WrapMode::kWithPreview,
-                   &wrapped);
-    if (!response.isSuccess()) return response;
-    exceptionDetails->setException(std::move(wrapped));
-  }
+  Response response =
+      addExceptionToDetails(exception, exceptionDetails.get(), objectGroup);
+  if (!response.isSuccess()) return response;
   *result = std::move(exceptionDetails);
   return Response::OK();
 }
@@ -709,8 +722,7 @@ Response InjectedScript::wrapEvaluateResult(
     if (!response.isSuccess()) return response;
     // We send exception in result for compatibility reasons, even though it's
     // accessible through exceptionDetails.exception.
-    response = createExceptionDetails(tryCatch, objectGroup, wrapMode,
-                                      exceptionDetails);
+    response = createExceptionDetails(tryCatch, objectGroup, exceptionDetails);
     if (!response.isSuccess()) return response;
   }
   return Response::OK();

src/inspector/injected-script.h

@@ -117,7 +117,7 @@ class InjectedScript final {
                                v8::Local<v8::Value>* result);
 
   Response createExceptionDetails(
-      const v8::TryCatch&, const String16& groupName, WrapMode wrapMode,
+      const v8::TryCatch&, const String16& groupName,
       Maybe<protocol::Runtime::ExceptionDetails>* result);
   Response wrapEvaluateResult(
       v8::MaybeLocal<v8::Value> maybeResultValue, const v8::TryCatch&,
@@ -219,6 +219,10 @@ class InjectedScript final {
   void discardEvaluateCallbacks();
   std::unique_ptr<EvaluateCallback> takeEvaluateCallback(
       EvaluateCallback* callback);
+  Response addExceptionToDetails(
+      v8::Local<v8::Value> exception,
+      protocol::Runtime::ExceptionDetails* exceptionDetails,
+      const String16& objectGroup);
 
   InspectedContext* m_context;
   int m_sessionId;

src/inspector/v8-runtime-agent-impl.cc

@@ -490,7 +490,7 @@ Response V8RuntimeAgentImpl::compileScript(
   if (!isOk) {
     if (scope.tryCatch().HasCaught()) {
       response = scope.injectedScript()->createExceptionDetails(
-          scope.tryCatch(), String16(), WrapMode::kNoPreview, exceptionDetails);
+          scope.tryCatch(), String16(), exceptionDetails);
       if (!response.isSuccess()) return response;
       return Response::OK();
     } else {

test/inspector/runtime/await-promise-expected.txt

@@ -19,10 +19,22 @@ Running test: testRejectedPromise
         exceptionDetails : {
             columnNumber : 0
             exception : {
-                type : object
-                value : {
-                    a : 1
+                className : Object
+                description : Object
+                objectId : <objectId>
+                preview : {
+                    description : Object
+                    overflow : false
+                    properties : [
+                        [0] : {
+                            name : a
+                            type : number
+                            value : 1
+                        }
+                    ]
+                    type : object
                 }
+                type : object
             }
             exceptionId : <exceptionId>
             lineNumber : 0

test/inspector/runtime/call-function-on-async-expected.txt

@@ -166,10 +166,22 @@ Running test: testFunctionReturnRejectedPromise
         exceptionDetails : {
             columnNumber : 0
             exception : {
-                type : object
-                value : {
-                    a : 3
+                className : Object
+                description : Object
+                objectId : <objectId>
+                preview : {
+                    description : Object
+                    overflow : false
+                    properties : [
+                        [0] : {
+                            name : a
+                            type : number
+                            value : 3
+                        }
+                    ]
+                    type : object
                 }
+                type : object
             }
             exceptionId : <exceptionId>
             lineNumber : 0
@@ -204,3 +216,75 @@ Running test: testPassingBothObjectIdAndExecutionContextId
     }
     id : <messageId>
 }
+
+Running test: testThrowNumber
+{
+    id : <messageId>
+    result : {
+        exceptionDetails : {
+            columnNumber : 10
+            exception : {
+                description : 100500
+                type : number
+                value : 100500
+            }
+            exceptionId : <exceptionId>
+            lineNumber : 0
+            scriptId : <scriptId>
+            stackTrace : {
+                callFrames : [
+                    [0] : {
+                        columnNumber : 10
+                        functionName : 
+                        lineNumber : 0
+                        scriptId : <scriptId>
+                        url : 
+                    }
+                ]
+            }
+            text : Uncaught
+        }
+        result : {
+            description : 100500
+            type : number
+            value : 100500
+        }
+    }
+}
+
+Running test: testAsyncFunctionWithUnknownReferenceReturnByValue
+{
+    id : <messageId>
+    result : {
+        exceptionDetails : {
+            columnNumber : 30
+            exception : {
+                className : ReferenceError
+                description : ReferenceError: does_not_exist is not defined     at <anonymous>:1:30
+                objectId : <objectId>
+                subtype : error
+                type : object
+            }
+            exceptionId : <exceptionId>
+            lineNumber : 1
+            scriptId : <scriptId>
+            stackTrace : {
+                callFrames : [
+                    [0] : {
+                        columnNumber : 29
+                        functionName : 
+                        lineNumber : 0
+                        scriptId : <scriptId>
+                        url : 
+                    }
+                ]
+            }
+            text : Uncaught (in promise) ReferenceError: does_not_exist is not defined
+        }
+        result : {
+            type : object
+            value : {
+            }
+        }
+    }
+}

test/inspector/runtime/call-function-on-async.js

@@ -146,6 +146,28 @@ let testSuite = [
       awaitPromise: false
     }));
   },
+
+  async function testThrowNumber() {
+    InspectorTest.logMessage(await callFunctionOn({
+      executionContextId,
+      functionDeclaration: '(() => { throw 100500; } )',
+      arguments: prepareArguments([]),
+      returnByValue: true,
+      generatePreview: false,
+      awaitPromise: true
+    }));
+  },
+
+  async function testAsyncFunctionWithUnknownReferenceReturnByValue() {
+    InspectorTest.logMessage(await callFunctionOn({
+      executionContextId,
+      functionDeclaration: '(async () => does_not_exist.click())',
+      arguments: prepareArguments([]),
+      returnByValue: true,
+      generatePreview: false,
+      awaitPromise: true
+    }));
+  },
 ];
 
 function prepareArguments(args) {

test/inspector/runtime/run-script-async-expected.txt

@@ -172,10 +172,22 @@ Running test: testAwaitRejectedPromise
         exceptionDetails : {
             columnNumber : 0
             exception : {
-                type : object
-                value : {
-                    a : 1
+                className : Object
+                description : Object
+                objectId : <objectId>
+                preview : {
+                    description : Object
+                    overflow : false
+                    properties : [
+                        [0] : {
+                            name : a
+                            type : number
+                            value : 1
+                        }
+                    ]
+                    type : object
                 }
+                type : object
             }
             exceptionId : <exceptionId>
             lineNumber : 0