[inspector] provide more usefull error message for non serializable value

src/inspector/InjectedScript.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 127 matching lines @@
   v8::TryCatch tryCatch(m_context->isolate());
   v8::Local<v8::Value> resultValue = function.callWithoutExceptionHandling();
   if (tryCatch.HasCaught()) {
     *exceptionDetails = createExceptionDetails(errorString, tryCatch, groupName,
                                                generatePreview);
     // FIXME: make properties optional
     *properties = Array<PropertyDescriptor>::create();
     return;
   }
 
   std::unique_ptr<protocol::Value> protocolValue =

[dgozman, 2016/09/19 17:35:05]

Looks like resultValue could be empty here?

[kozy, 2016/09/20 15:34:27]

Added check. I think that it should be not empty if exception wasn't thrown.

-      toProtocolValue(context, resultValue);
-  if (hasInternalError(errorString, !protocolValue)) return;
+      toProtocolValue(errorString, context, resultValue);
+  if (!protocolValue) return;
   protocol::ErrorSupport errors(errorString);
   std::unique_ptr<Array<PropertyDescriptor>> result =
       Array<PropertyDescriptor>::parse(protocolValue.get(), &errors);
   if (!hasInternalError(errorString, errors.hasErrors()))
     *properties = std::move(result);
 }
 
 void InjectedScript::releaseObject(const String16& objectId) {
   std::unique_ptr<protocol::Value> parsedObjectId =
       protocol::parseJSON(objectId);
@@ skipping 12 matching lines @@
     bool generatePreview) const {
   v8::HandleScope handles(m_context->isolate());
   v8::Local<v8::Value> wrappedObject;
   v8::Local<v8::Context> context = m_context->context();
   if (!wrapValue(errorString, value, groupName, forceValueType, generatePreview)
            .ToLocal(&wrappedObject))
     return nullptr;
   protocol::ErrorSupport errors;
   std::unique_ptr<protocol::Runtime::RemoteObject> remoteObject =
       protocol::Runtime::RemoteObject::parse(
-          toProtocolValue(context, wrappedObject).get(), &errors);
-  if (!remoteObject) *errorString = "Object has too long reference chain";
+          toProtocolValue(errorString, context, wrappedObject).get(), &errors);

[dgozman, 2016/09/19 17:35:05]

Should check return value of toProtocolValue and use it's errorString.

[kozy, 2016/09/20 15:34:27]

Done.

   return remoteObject;
 }
 
 bool InjectedScript::wrapObjectProperty(ErrorString* errorString,
                                         v8::Local<v8::Object> object,
                                         v8::Local<v8::Name> key,
                                         const String16& groupName,
                                         bool forceValueType,
                                         bool generatePreview) const {
   v8::Local<v8::Value> property;
@@ skipping 69 matching lines @@
   v8::HandleScope handles(m_context->isolate());
   v8::Local<v8::Context> context = m_context->context();
   V8FunctionCall function(m_context->inspector(), context, v8Value(),
                           "wrapTable");
   function.appendArgument(table);
   if (columns.IsEmpty())
     function.appendArgument(false);
   else
     function.appendArgument(columns);
   bool hadException = false;
   v8::Local<v8::Value> r = function.call(hadException);

[dgozman, 2016/09/19 17:35:05]

Looks like r could be empty here?

[kozy, 2016/09/20 15:34:27]

Done.

   if (hadException) return nullptr;
   protocol::ErrorSupport errors;
+  protocol::ErrorString errorString;
   return protocol::Runtime::RemoteObject::parse(
-      toProtocolValue(context, r).get(), &errors);
+      toProtocolValue(&errorString, context, r).get(), &errors);

[dgozman, 2016/09/19 17:35:05]

Should check return value of toProtocolValue and use it's errorString.

[kozy, 2016/09/20 15:34:27]

Done.

 }
 
 bool InjectedScript::findObject(ErrorString* errorString,
                                 const RemoteObjectId& objectId,
                                 v8::Local<v8::Value>* outObject) const {
   *outObject = m_native->objectForId(objectId.id());
   if (outObject->IsEmpty())
     *errorString = "Could not find object with given id";
   return !outObject->IsEmpty();
 }
@@ skipping 273 matching lines @@
 void InjectedScript::CallFrameScope::findInjectedScript(
     V8InspectorSessionImpl* session) {
   std::unique_ptr<RemoteCallFrameId> remoteId =
       RemoteCallFrameId::parse(m_errorString, m_remoteCallFrameId);
   if (!remoteId) return;
   m_frameOrdinal = static_cast<size_t>(remoteId->frameOrdinal());
   m_injectedScript = session->findInjectedScript(m_errorString, remoteId.get());
 }
 
 }  // namespace v8_inspector