src: refactor JSError stringifier

Essentially move all the logic related to how JSError stack traces are
accessible to llv8 files, breaking this logic into smaller functions.
Also moved the chunk of code related to printing JSObject properties to
its own function, to allow reuse between JSError and JSObject
stringifiers.

PR-URL: #256
Reviewed-By: Joyee Cheung <[email protected]>

Author: mmarchini

src/llv8-inl.h

@@ -722,6 +722,18 @@ inline bool JSArrayBuffer::WasNeutered(Error& err) {
   return field != 0;
 }
 
+inline std::string JSError::stack_trace_property() {
+  // TODO (mmarchini): once we have Symbol support we'll need to search for
+  // <unnamed symbol>, since the stack symbol doesn't have an external name.
+  // In the future we can add postmortem metadata on V8 regarding existing
+  // symbols, but for now we'll use an heuristic to find the stack in the
+  // error object.
+  return v8()->types()->kSymbolType != -1 ? "Symbol()" : "<non-string>";
+}
+
+inline int StackTrace::GetFrameCount() { return len_; }
+
+
 #undef ACCESSOR
 
 }  // namespace v8

src/llv8.cc

@@ -1202,5 +1202,111 @@ v8::Value JSObject::GetArrayElement(int64_t pos, Error& err) {
 }
 
 
+bool JSError::HasStackTrace(Error& err) {
+  StackTrace stack_trace = GetStackTrace(err);
+
+  return stack_trace.GetFrameCount() > -1;
+}
+
+
+JSArray JSError::GetFrameArray(Error& err) {
+  v8::Value maybe_stack = GetProperty(stack_trace_property(), err);
+
+  if (err.Fail() || maybe_stack.raw() == -1) {
+    Error::PrintInDebugMode(
+        "Couldn't find a symbol property in the Error object.");
+    return JSArray();
+  }
+
+  int64_t type = v8::HeapObject(maybe_stack).GetType(err);
+
+  if (err.Fail()) {
+    Error::PrintInDebugMode("Symbol property references an invalid object.");
+    return JSArray();
+  }
+
+  // NOTE (mmarchini): The stack is stored as a JSArray
+  if (type != v8()->types()->kJSArrayType) {
+    Error::PrintInDebugMode("Symbol property doesn't have the right type.");
+    return JSArray();
+  }
+
+  v8::JSArray arr(maybe_stack);
+
+  return arr;
+}
+
+
+StackTrace::Iterator StackTrace::begin() {
+  Error err;
+  return StackTrace::Iterator(this, err);
+}
+
+StackTrace::Iterator StackTrace::end() {
+  Error err;
+  return StackTrace::Iterator(this, GetFrameCount(), err);
+}
+
+StackTrace::StackTrace(JSArray frame_array, Error& err)
+    : frame_array_(frame_array) {
+  v8::Value maybe_stack_len = frame_array.GetArrayElement(0, err);
+
+  if (err.Fail()) {
+    Error::PrintInDebugMode(
+        "Couldn't get the first element from the stack array");
+    return;
+  }
+
+  len_ = v8::Smi(maybe_stack_len).GetValue();
+
+  multiplier_ = 5;
+  // On Node.js v8.x, the first array element is the stack size, and each
+  // stack frame use 5 elements.
+  if ((len_ * multiplier_ + 1) != frame_array_.GetArrayLength(err)) {
+    // On Node.js v6.x, the first array element is zero, and each stack frame
+    // use 4 element.
+    multiplier_ = 4;
+    if ((len_ != 0) ||
+        ((frame_array_.GetArrayLength(err) - 1) % multiplier_ != 0)) {
+      Error::PrintInDebugMode(
+          "JSArray doesn't look like a Stack Frames array. stack_len: %lld "
+          "array_len: %lld",
+          len_, frame_array_.GetArrayLength(err));
+      len_ = -1;
+      multiplier_ = -1;
+      return;
+    }
+    len_ = (frame_array_.GetArrayLength(err) - 1) / multiplier_;
+  }
+}
+
+
+StackTrace JSError::GetStackTrace(Error& err) {
+  return StackTrace(GetFrameArray(err), err);
+}
+
+
+StackFrame StackTrace::GetFrame(uint32_t index, Error& err) {
+  return StackFrame(this, index);
+}
+
+StackFrame::StackFrame(StackTrace* stack_trace, int index)
+    : stack_trace_(stack_trace), index_(index) {}
+
+JSFunction StackFrame::GetFunction(Error& err) {
+  JSArray frame_array = stack_trace_->frame_array_;
+  v8::Value maybe_fn = frame_array.GetArrayElement(frame_array_index(), err);
+  if (err.Fail()) return JSFunction();
+  return JSFunction(maybe_fn);
+}
+
+int StackFrame::frame_array_index() {
+  int multiplier = stack_trace_->multiplier_;
+  const int js_function_pos = 1;
+  const int begin_offset = 1;
+  return begin_offset + js_function_pos + (index_ * multiplier);
+}
+
+
 }  // namespace v8
 }  // namespace llnode

src/llv8.h

@@ -59,7 +59,7 @@ class Value {
 
  protected:
   LLV8* v8_;
-  int64_t raw_;
+  int64_t raw_ = 0x0;
 };
 
 class Smi : public Value {
@@ -267,18 +267,88 @@ class JSObject : public HeapObject {
   Value GetDescriptorProperty(std::string key_name, Map map, Error& err);
 };
 
+class StackTrace;
+class StackFrame;
+class JSFunction;
+class JSArray : public JSObject {
+ public:
+  V8_VALUE_DEFAULT_METHODS(JSArray, JSObject);
+
+  inline Smi Length(Error& err);
+};
+
 class JSError : public JSObject {
  public:
   V8_VALUE_DEFAULT_METHODS(JSError, JSObject);
+
+  bool HasStackTrace(Error& err);
+  StackTrace GetStackTrace(Error& err);
+
+ private:
+  JSArray GetFrameArray(Error& err);
+
+  inline std::string stack_trace_property();
 };
 
-class JSArray : public JSObject {
+class StackFrame {
  public:
-  V8_VALUE_DEFAULT_METHODS(JSArray, JSObject);
+  JSFunction GetFunction(Error& err);
 
-  inline Smi Length(Error& err);
+ private:
+  friend StackTrace;
+  StackFrame() = delete;
+  StackFrame(StackTrace*, int);
+
+  StackTrace* stack_trace_;
+  int index_ = -1;
+
+  int frame_array_index();
+};
+
+
+class StackTrace {
+ public:
+  class Iterator {
+   public:
+    StackFrame operator*() { return stack_trace_->GetFrame(index_, err_); };
+    inline const StackTrace::Iterator operator++() {
+      return StackTrace::Iterator(stack_trace_, ++index_, err_);
+    };
+    bool operator!=(StackTrace::Iterator that) {
+      return index() != that.index();
+    };
+
+    inline Iterator(StackTrace* stack_trace, Error& err)
+        : stack_trace_(stack_trace), err_(err){};
+
+    inline Iterator(StackTrace* stack_trace, int index, Error& err)
+        : stack_trace_(stack_trace), index_(index), err_(err){};
+
+    int index() { return index_; };
+
+   private:
+    StackTrace* stack_trace_;
+    int index_ = 0;
+    Error err_;
+  };
+  Iterator begin();
+  Iterator end();
+
+  StackTrace() = delete;
+  StackTrace(JSArray frame_array, Error& err);
+
+  StackFrame GetFrame(uint32_t index, Error& err);
+
+  inline int GetFrameCount();
+
+ private:
+  friend StackFrame;
+  JSArray frame_array_;
+  int multiplier_ = -1;
+  int len_ = -1;
 };
 
+
 class JSFunction : public JSObject {
  public:
   V8_VALUE_DEFAULT_METHODS(JSFunction, JSObject)

src/printer.cc

@@ -547,99 +547,25 @@ std::string Printer::Stringify(v8::JSError js_error, Error& err) {
 
   // Print properties in detailed mode
   if (options_.detailed) {
-    output << rang::fg::reset << " " << StringifyProperties(js_error, err);
-    if (err.Fail()) return std::string();
-
-    std::string fields = StringifyInternalFields(js_error, err);
-    if (err.Fail()) return std::string();
-
-    if (!fields.empty()) {
-      output << std::endl << rang::fg::magenta << "  internal fields"
-          << rang::fg::reset << " {" << std::endl << fields << "}";
-    }
-  }
-
-  if (options_.detailed) {
-    PrinterOptions simple;
-
-    // TODO (mmarchini): once we have Symbol support we'll need to search for
-    // <unnamed symbol>, since the stack symbol doesn't have an external name.
-    // In the future we can add postmortem metadata on V8 regarding existing
-    // symbols, but for now we'll use an heuristic to find the stack in the
-    // error object.
-    std::string stack_property = llv8_->types()->kSymbolType != -1 ? "Symbol()" : "<non-string>";
-    v8::Value maybe_stack = js_error.GetProperty(stack_property, err);
-
-    if (err.Fail() || maybe_stack.raw() == -1) {
-      Error::PrintInDebugMode(
-          "Couldn't find a symbol property in the Error object.");
-      output << rang::fg::yellow << ">" << rang::fg::reset;
-      return output.str();
-    }
-
-    int64_t type = v8::HeapObject(maybe_stack).GetType(err);
+    output << StringifyJSObjectFields(js_error, err);
 
-    if (err.Fail()) {
-      Error::PrintInDebugMode("Symbol property references an invalid object.");
-      output << rang::fg::yellow << ">" << rang::fg::reset;
-      return output.str();
-    }
-
-    // NOTE (mmarchini): The stack is stored as a JSArray
-    if (type != llv8_->types()->kJSArrayType) {
-      Error::PrintInDebugMode("Symbol property doesn't have the right type.");
-      output << rang::fg::yellow << ">" << rang::fg::reset;
-      return output.str();
-    }
-
-    v8::JSArray arr(maybe_stack);
-
-    v8::Value maybe_stack_len = arr.GetArrayElement(0, err);
-
-    if (err.Fail()) {
-      Error::PrintInDebugMode(
-          "Couldn't get the first element from the stack array");
-      output << rang::fg::yellow << ">" << rang::fg::reset;
-      return output.str();
-    }
-
-    int64_t stack_len = v8::Smi(maybe_stack_len).GetValue();
-
-    int multiplier = 5;
-    // On Node.js v8.x, the first array element is the stack size, and each
-    // stack frame use 5 elements.
-    if ((stack_len * multiplier + 1) != arr.GetArrayLength(err)) {
-      // On Node.js v6.x, the first array element is zero, and each stack frame
-      // use 4 element.
-      multiplier = 4;
-      if ((stack_len != 0) ||
-          ((arr.GetArrayLength(err) - 1) % multiplier != 0)) {
-        Error::PrintInDebugMode(
-            "JSArray doesn't look like a Stack Frames array. stack_len: %lld "
-            "array_len: %lld",
-            stack_len, arr.GetArrayLength(err));
-        output << rang::fg::yellow << ">" << rang::fg::reset;
-        return output.str();
-      }
-      stack_len = (arr.GetArrayLength(err) - 1) / multiplier;
-    }
+    v8::StackTrace stack_trace = js_error.GetStackTrace(err);
 
     std::stringstream error_stack;
     error_stack << std::endl
                 << rang::fg::red << "  error stack" << rang::fg::reset << " {"
                 << std::endl;
 
-    // TODO (mmarchini): Refactor: create an StackIterator which returns
-    // StackFrame objects
-    for (int64_t i = 0; i < stack_len; i++) {
-      v8::Value maybe_fn = arr.GetArrayElement(2 + (i * multiplier), err);
+    Printer printer(llv8_);
+    for (v8::StackFrame frame : stack_trace) {
+      v8::JSFunction js_function = frame.GetFunction(err);
       if (err.Fail()) {
         error_stack << rang::fg::gray << "    <unknown>" << std::endl;
         continue;
       }
 
-      Printer printer(llv8_);
-      error_stack << "    " << printer.Stringify(v8::HeapObject(maybe_fn), err)
+      error_stack << "    "
+                  << printer.Stringify<v8::HeapObject>(js_function, err)
                   << std::endl;
     }
     error_stack << "  }";
@@ -661,19 +587,29 @@ std::string Printer::Stringify(v8::JSObject js_object, Error& err) {
   output << rang::fg::yellow << "<Object: " << name;
 
   // Print properties in detailed mode
-  if (options_.detailed) {
-    output << rang::fg::reset << " " << StringifyProperties(js_object, err);
-    if (err.Fail()) return std::string();
+  if (options_.detailed) output << StringifyJSObjectFields(js_object, err);
 
-    std::string fields = StringifyInternalFields(js_object, err);
-    if (err.Fail()) return std::string();
+  output << rang::fg::yellow << ">" << rang::fg::reset;
 
-    if (!fields.empty()) {
-      output << std::endl << rang::fg::magenta << "  internal fields"
-          << rang::fg::reset << " {" << std::endl << fields << "}";
-    }
+  return output.str();
+}
+
+std::string Printer::StringifyJSObjectFields(v8::JSObject js_object,
+                                             Error& err) {
+  std::stringstream output;
+
+  output << rang::fg::reset << " " << StringifyProperties(js_object, err);
+  if (err.Fail()) return std::string();
+
+  std::string fields = StringifyInternalFields(js_object, err);
+  if (err.Fail()) return std::string();
+
+  if (!fields.empty()) {
+    output << std::endl
+           << rang::fg::magenta << "  internal fields" << rang::fg::reset
+           << " {" << std::endl
+           << fields << "}";
   }
-  output << rang::fg::yellow << ">" << rang::fg::reset;
 
   return output.str();
 }

src/printer.h

@@ -55,6 +55,7 @@ class Printer {
   std::string StringifyDescriptors(v8::JSObject js_obj, v8::Map map,
                                    Error& err);
 
+  std::string StringifyJSObjectFields(v8::JSObject js_obj, Error& err);
 
   // FixedArray Specific Methods
   std::string StringifyContents(v8::FixedArray fixed_array, int length,