[json] Extend JSON#stringify error message for circular structures

This CL extends the kCircularStructure error message to include the
constructors and keys involved in the circle:

const a = {};
a.arr = [];
a.arr[0] = a;
JSON.stringify(a);

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'arr' -> object with constructor 'Array'
    --- index 0 closes the circle

R=gsathya@chromium.org, yangguo@chromium.org

Bug: v8:6513, v8:8696
Change-Id: I393aa3ce47d8bfd03734fccac63445006940ef7a
Reviewed-on: https://chromium-review.googlesource.com/c/1433776Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59152}

src/json-stringifier.cc

@@ -109,6 +109,15 @@ class JsonStringifier {
   Result StackPush(Handle<Object> object, Handle<Object> key);
   void StackPop();
 
+  // Uses the current stack_ to provide a detailed error message of
+  // the objects involved in the circular structure.
+  Handle<String> ConstructCircularStructureErrorMessage(Handle<Object> last_key,
+                                                        size_t start_index);
+  // The prefix and postfix count do NOT include the starting and
+  // closing lines of the error message.
+  static const int kCircularErrorMessagePrefixCount = 2;
+  static const int kCircularErrorMessagePostfixCount = 1;
+
   Factory* factory() { return isolate_->factory(); }
 
   Isolate* isolate_;
@@ -373,11 +382,13 @@ JsonStringifier::Result JsonStringifier::StackPush(Handle<Object> object,
 
   {
     DisallowHeapAllocation no_allocation;
-    for (const KeyObject& key_object : stack_) {
-      if (*key_object.second == *object) {
+    for (size_t i = 0; i < stack_.size(); ++i) {
+      if (*stack_[i].second == *object) {
         AllowHeapAllocation allow_to_return_error;
-        Handle<Object> error =
-            factory()->NewTypeError(MessageTemplate::kCircularStructure);
+        Handle<String> circle_description =
+            ConstructCircularStructureErrorMessage(key, i);
+        Handle<Object> error = factory()->NewTypeError(
+            MessageTemplate::kCircularStructure, circle_description);
         isolate_->Throw(*error);
         return EXCEPTION;
       }
@@ -389,6 +400,117 @@ JsonStringifier::Result JsonStringifier::StackPush(Handle<Object> object,
 
 void JsonStringifier::StackPop() { stack_.pop_back(); }
 
+class CircularStructureMessageBuilder {
+ public:
+  explicit CircularStructureMessageBuilder(Isolate* isolate)
+      : builder_(isolate) {}
+
+  void AppendStartLine(Handle<Object> start_object) {
+    builder_.AppendCString(kStartPrefix);
+    builder_.AppendCString("starting at object with constructor ");
+    AppendConstructorName(start_object);
+  }
+
+  void AppendNormalLine(Handle<Object> key, Handle<Object> object) {
+    builder_.AppendCString(kLinePrefix);
+    AppendKey(key);
+    builder_.AppendCString(" -> object with constructor ");
+    AppendConstructorName(object);
+  }
+
+  void AppendClosingLine(Handle<Object> closing_key) {
+    builder_.AppendCString(kEndPrefix);
+    AppendKey(closing_key);
+    builder_.AppendCString(" closes the circle");
+  }
+
+  void AppendEllipsis() {
+    builder_.AppendCString(kLinePrefix);
+    builder_.AppendCString("...");
+  }
+
+  MaybeHandle<String> Finish() { return builder_.Finish(); }
+
+ private:
+  void AppendConstructorName(Handle<Object> object) {
+    builder_.AppendCharacter('\'');
+    Handle<String> constructor_name =
+        JSReceiver::GetConstructorName(Handle<JSReceiver>::cast(object));
+    builder_.AppendString(constructor_name);
+    builder_.AppendCharacter('\'');
+  }
+
+  // A key can either be a string, the empty string or a Smi.
+  void AppendKey(Handle<Object> key) {
+    if (key->IsSmi()) {
+      builder_.AppendCString("index ");
+      AppendSmi(Smi::cast(*key));
+      return;
+    }
+
+    CHECK(key->IsString());
+    Handle<String> key_as_string = Handle<String>::cast(key);
+    if (key_as_string->length() == 0) {
+      builder_.AppendCString("<anonymous>");
+    } else {
+      builder_.AppendCString("property '");
+      builder_.AppendString(key_as_string);
+      builder_.AppendCharacter('\'');
+    }
+  }
+
+  void AppendSmi(Smi smi) {
+    static const int kBufferSize = 100;
+    char chars[kBufferSize];
+    Vector<char> buffer(chars, kBufferSize);
+    builder_.AppendCString(IntToCString(smi->value(), buffer));
+  }
+
+  IncrementalStringBuilder builder_;
+  static constexpr const char* kStartPrefix = "\n    --> ";
+  static constexpr const char* kEndPrefix = "\n    --- ";
+  static constexpr const char* kLinePrefix = "\n    |     ";
+};
+
+Handle<String> JsonStringifier::ConstructCircularStructureErrorMessage(
+    Handle<Object> last_key, size_t start_index) {
+  DCHECK(start_index < stack_.size());
+  CircularStructureMessageBuilder builder(isolate_);
+
+  // We track the index to be printed next for better readability.
+  size_t index = start_index;
+  const size_t stack_size = stack_.size();
+
+  builder.AppendStartLine(stack_[index++].second);
+
+  // Append a maximum of kCircularErrorMessagePrefixCount normal lines.
+  const size_t prefix_end =
+      std::min(stack_size, index + kCircularErrorMessagePrefixCount);
+  for (; index < prefix_end; ++index) {
+    builder.AppendNormalLine(stack_[index].first, stack_[index].second);
+  }
+
+  // If the circle consists of too many objects, we skip them and just
+  // print an ellipsis.
+  if (stack_size > index + kCircularErrorMessagePostfixCount) {
+    builder.AppendEllipsis();
+  }
+
+  // Since we calculate the postfix lines from the back of the stack,
+  // we have to ensure that lines are not printed twice.
+  index = std::max(index, stack_size - kCircularErrorMessagePostfixCount);
+  for (; index < stack_size; ++index) {
+    builder.AppendNormalLine(stack_[index].first, stack_[index].second);
+  }
+
+  builder.AppendClosingLine(last_key);
+
+  Handle<String> result;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate_, result, builder.Finish(),
+                                   factory()->empty_string());
+  return result;
+}
+
 template <bool deferred_string_key>
 JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object,
                                                     bool comma,

src/message-template.h

@@ -61,7 +61,7 @@ namespace internal {
   T(CannotFreezeArrayBufferView,                                               \
     "Cannot freeze array buffer views with elements")                          \
   T(CannotSeal, "Cannot seal")                                                 \
-  T(CircularStructure, "Converting circular structure to JSON")                \
+  T(CircularStructure, "Converting circular structure to JSON%")               \
   T(ConstructAbstractClass, "Abstract class % not directly constructable")     \
   T(ConstAssign, "Assignment to constant variable.")                           \
   T(ConstructorClassField, "Classes may not have a field named 'constructor'") \

test/message/fail/json-stringify-circular-ellipsis.js

+// Copyright 2019 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.
+
+class Outer {
+    constructor(o) { this.x = o; }
+}
+
+class Inner {
+    constructor(o) { this.y = o; }
+}
+
+class ArrayHolder {
+    constructor(o) {
+        this.array = [];
+        this.array[1] = o;
+    }
+}
+
+const root = {};
+root.first = new Outer(
+    new ArrayHolder(
+        new Inner(root)
+    )
+);
+
+JSON.stringify(root);

test/message/fail/json-stringify-circular-ellipsis.out

+*%(basename)s:27: TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    |     property 'first' -> object with constructor 'Outer'
+    |     property 'x' -> object with constructor 'ArrayHolder'
+    |     ...
+    |     index 1 -> object with constructor 'Inner'
+    --- property 'y' closes the circle
+JSON.stringify(root);
+     ^
+TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    |     property 'first' -> object with constructor 'Outer'
+    |     property 'x' -> object with constructor 'ArrayHolder'
+    |     ...
+    |     index 1 -> object with constructor 'Inner'
+    --- property 'y' closes the circle
+    at JSON.stringify (<anonymous>)
+    at *%(basename)s:27:6
+

test/message/fail/json-stringify-circular-max-display-depth.js

+// Copyright 2019 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.
+
+class Outer {
+    constructor(o) { this.x = o; }
+}
+
+class ArrayHolder {
+    constructor(o) {
+        this.array = [];
+        this.array[1] = o;
+    }
+}
+
+const root = {};
+root.first = new Outer(
+    new ArrayHolder(root)
+);
+
+JSON.stringify(root);

test/message/fail/json-stringify-circular-max-display-depth.out

+*%(basename)s:21: TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    |     property 'first' -> object with constructor 'Outer'
+    |     property 'x' -> object with constructor 'ArrayHolder'
+    |     property 'array' -> object with constructor 'Array'
+    --- index 1 closes the circle
+JSON.stringify(root);
+     ^
+TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    |     property 'first' -> object with constructor 'Outer'
+    |     property 'x' -> object with constructor 'ArrayHolder'
+    |     property 'array' -> object with constructor 'Array'
+    --- index 1 closes the circle
+    at JSON.stringify (<anonymous>)
+    at *%(basename)s:21:6
+

test/message/fail/json-stringify-circular-proxy.js

+// Copyright 2019 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.
+
+class Outer {
+    constructor(o) { this.x = o; }
+}
+
+class Inner {
+    constructor(o) { this.y = o; }
+}
+
+class ArrayHolder {
+    constructor(o) {
+        this.array = [];
+        this.array[1] = o;
+    }
+}
+
+const root = {};
+const outer = new Outer(
+    new ArrayHolder(
+        new Inner(root)
+    )
+);
+root.first = new Proxy(outer, outer);
+
+JSON.stringify(root);

test/message/fail/json-stringify-circular-proxy.out

+*%(basename)s:28: TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    |     property 'first' -> object with constructor 'Object'
+    |     property 'x' -> object with constructor 'ArrayHolder'
+    |     ...
+    |     index 1 -> object with constructor 'Inner'
+    --- property 'y' closes the circle
+JSON.stringify(root);
+     ^
+TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    |     property 'first' -> object with constructor 'Object'
+    |     property 'x' -> object with constructor 'ArrayHolder'
+    |     ...
+    |     index 1 -> object with constructor 'Inner'
+    --- property 'y' closes the circle
+    at JSON.stringify (<anonymous>)
+    at *%(basename)s:28:6
+

test/message/fail/json-stringify-circular-substructure.js

+// Copyright 2019 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.
+
+const object = {};
+object.substructure = {};
+object.substructure.key = object.substructure;
+
+JSON.stringify(object);

test/message/fail/json-stringify-circular-substructure.out

+*%(basename)s:9: TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    --- property 'key' closes the circle
+JSON.stringify(object);
+     ^
+TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    --- property 'key' closes the circle
+    at JSON.stringify (<anonymous>)
+    at *%(basename)s:9:6
+

test/message/fail/json-stringify-circular.js

+// Copyright 2019 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.
+
+const object = {};
+object.key = object;
+
+JSON.stringify(object);

test/message/fail/json-stringify-circular.out

+*%(basename)s:8: TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    --- property 'key' closes the circle
+JSON.stringify(object);
+     ^
+TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    --- property 'key' closes the circle
+    at JSON.stringify (<anonymous>)
+    at *%(basename)s:8:6
+

test/mjsunit/messages.js

@@ -126,13 +126,6 @@ test(function() {
   [].join(o);
 }, "Cannot convert object to primitive value", TypeError);
 
-// kCircularStructure
-test(function() {
-  var o = {};
-  o.o = o;
-  JSON.stringify(o);
-}, "Converting circular structure to JSON", TypeError);
-
 // kConstructorNotFunction
 test(function() {
   Map();

test/webkit/JSON-stringify-replacer-expected.txt

@@ -40,10 +40,18 @@ PASS JSON.stringify(object, returnNullFor1) is '{"0":0,"1":null,"2":2}'
 PASS JSON.stringify(array, returnNullFor1) is '[0,null,2,null]'
 PASS JSON.stringify(object, returnStringForUndefined) is '{"0":0,"1":1,"2":2,"3":"undefined value"}'
 PASS JSON.stringify(array, returnStringForUndefined) is '[0,1,2,"undefined value"]'
-PASS JSON.stringify(object, returnCycleObjectFor1) threw exception TypeError: Converting circular structure to JSON.
-PASS JSON.stringify(array, returnCycleObjectFor1) threw exception TypeError: Converting circular structure to JSON.
-PASS JSON.stringify(object, returnCycleArrayFor1) threw exception TypeError: Converting circular structure to JSON.
-PASS JSON.stringify(array, returnCycleArrayFor1) threw exception TypeError: Converting circular structure to JSON.
+PASS JSON.stringify(object, returnCycleObjectFor1) threw exception TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    --- property '1' closes the circle.
+PASS JSON.stringify(array, returnCycleObjectFor1) threw exception TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    --- property '1' closes the circle.
+PASS JSON.stringify(object, returnCycleArrayFor1) threw exception TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Array'
+    --- index 1 closes the circle.
+PASS JSON.stringify(array, returnCycleArrayFor1) threw exception TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Array'
+    --- index 1 closes the circle.
 PASS successfullyParsed is true
 
 TEST COMPLETE

test/webkit/run-json-stringify-expected.txt

@@ -434,7 +434,9 @@ function (jsonObject){
         cycleTracker = "";
         return jsonObject.stringify(cyclicObject);
     }
-PASS tests[i](nativeJSON) threw exception TypeError: Converting circular structure to JSON.
+PASS tests[i](nativeJSON) threw exception TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Object'
+    --- property 'self' closes the circle.
 function (jsonObject){
         cycleTracker = "";
         try { jsonObject.stringify(cyclicObject); } catch(e) { cycleTracker += " -> exception" }
@@ -445,7 +447,9 @@ function (jsonObject){
         cycleTracker = "";
         return jsonObject.stringify(cyclicArray);
     }
-PASS tests[i](nativeJSON) threw exception TypeError: Converting circular structure to JSON.
+PASS tests[i](nativeJSON) threw exception TypeError: Converting circular structure to JSON
+    --> starting at object with constructor 'Array'
+    --- index 1 closes the circle.
 function (jsonObject){
         cycleTracker = "";
         try { jsonObject.stringify(cyclicArray); } catch { cycleTracker += " -> exception" }