Prioritize capturing expressions (#162)

DominicKramer · web-flow · commit d370e20e1e85 · 2016-11-10T14:13:10.000-08:00
Now the collection of the contents of watched expressions is given priority over other variables such as locals.  Further, the contents of watched expressions are shown completely provided that this can be done within the limits of `config.capture.maxDataSize`.  Otherwise, a message is displayed describing why the entire contents of the watched expression cannot be displayed.

For example, if a breakpoint is hit with a watched expression that is an array (even if that array contains more than `config.capture.maxProperties` elements), the entire contents of the array will be collected provided that the limit `config.capture.maxDataSize` is not reached.  Then, other variables such as locals will be collected (provided that there is enough space to collect that information as specified by `config.capture.maxDataSize`).
diff --git a/lib/state.js b/lib/state.js
@@ -41,6 +41,7 @@ var GETTER_MESSAGE_INDEX = 2;
 var ARG_LOCAL_LIMIT_MESSAGE_INDEX = 3;
 var OBJECT_LIMIT_MESSAGE_INDEX = 4;
 var STRING_LIMIT_MESSAGE_INDEX = 5;
+var DATA_LIMIT_MESSAGE_INDEX = 6;
 
 var MESSAGE_TABLE = [];
 MESSAGE_TABLE[BUFFER_FULL_MESSAGE_INDEX] =
@@ -67,6 +68,11 @@ MESSAGE_TABLE[STRING_LIMIT_MESSAGE_INDEX] =
                               'Only first `config.capture.maxStringLength` chars' +
                               ' were captured.',
                                 false) };
+MESSAGE_TABLE[DATA_LIMIT_MESSAGE_INDEX] =
+  { status: new StatusMessage(StatusMessage.VARIABLE_VALUE,
+                              'Truncating the results because the cap of ' +
+                              '`config.capture.maxDataSize` has been reached.',
+                                false) };
 
 /**
  * Captures the stack and current execution state.
@@ -140,11 +146,10 @@ function StateResolver(execState, expressions, config, v8) {
  *         evaluatedExpressions fields
  */
 StateResolver.prototype.capture_ = function() {
-  // Gather the stack frames first
   var that = this;
-  var frames = that.resolveFrames_();
 
   // Evaluate the watch expressions
+  var evalIndexSet = new Set();
   if (that.expressions_) {
     that.expressions_.forEach(function(expression, index) {
       var result = evaluate(expression, that.state_.frame(0));
@@ -157,20 +162,30 @@ StateResolver.prototype.capture_ = function() {
                                     result.error, true)
         };
       } else {
-        evaluated = that.resolveVariable_(expression, result.mirror);
+        evaluated = that.resolveVariable_(expression, result.mirror, true);
+        var varTableIdx = evaluated.varTableIndex;
+        if (typeof varTableIdx !== 'undefined'){
+          evalIndexSet.add(varTableIdx);
+        }
       }
       that.evaluatedExpressions_[index] = evaluated;
     });
   }
 
+  // The frames are resolved after the evaluated expressions so that 
+  // evaluated expressions can be evaluated as much as possible within 
+  // the max data size limits
+  var frames = that.resolveFrames_();
+
   // Now resolve the variables
   var index = MESSAGE_TABLE.length; // skip the sentinel values
   var noLimit = that.config_.capture.maxDataSize === 0;
   while (index < that.rawVariableTable_.length && // NOTE: length changes in loop
          (that.totalSize_ < that.config_.capture.maxDataSize || noLimit)) {
     assert(!that.resolvedVariableTable_[index]); // shouldn't have it resolved yet
+    var isEvaluated = evalIndexSet.has(index);
     that.resolvedVariableTable_[index] =
-      that.resolveMirror_(that.rawVariableTable_[index]);
+      that.resolveMirror_(that.rawVariableTable_[index], isEvaluated);
     index++;
   }
 
@@ -349,7 +364,7 @@ StateResolver.prototype.extractArgumentsList_ = function (frame) {
 StateResolver.prototype.resolveArgumentList_ = function(args) {
   var resolveVariable = this.resolveVariable_.bind(this);
   return args.map(function (arg){
-    return resolveVariable(arg.name, arg.value);
+    return resolveVariable(arg.name, arg.value, false);
   });
 };
 
@@ -413,12 +428,12 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
             // locals list.
             remove(args, {name: name});
             usedNames[name] = true;
-            locals.push(self.resolveVariable_(name, trg));
+            locals.push(self.resolveVariable_(name, trg, false));
           } else if (!usedNames[name]) {
             // It's a valid variable that belongs in the locals list and wasn't
             // discovered at a lower-scope
             usedNames[name] = true;
-            locals.push(self.resolveVariable_(name, trg));
+            locals.push(self.resolveVariable_(name, trg, false));
           } // otherwise another same-named variable occured at a lower scope
           return locals;
         },
@@ -432,7 +447,7 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
     // under the name 'context' which is used by the Chrome DevTools.
     var ctx = frame.details().receiver();
     if (ctx) {
-      return [self.resolveVariable_('context', makeMirror(ctx))];
+      return [self.resolveVariable_('context', makeMirror(ctx), false)];
     }
     return [];
   }()));
@@ -445,8 +460,10 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
  *
  * @param {String} name The name of the variable.
  * @param {Object} value A v8 debugger representation of a variable value.
+ * @param {boolean} isEvaluated Specifies if the variable is from a watched
+ *   expression.
  */
-StateResolver.prototype.resolveVariable_ = function(name, value) {
+StateResolver.prototype.resolveVariable_ = function(name, value, isEvaluated) {
   var size = name.length;
 
   var data = {
@@ -468,10 +485,11 @@ StateResolver.prototype.resolveVariable_ = function(name, value) {
   } else if (value.isObject()) {
     data.varTableIndex = this.getVariableIndex_(value);
     var maxProps = this.config_.capture.maxProperties;
-    if (maxProps && maxProps < Object.keys(value.value()).length) {
+    var numKeys = Object.keys(value.value()).length;
+
+    if (!isEvaluated && maxProps && maxProps < numKeys) {
       data.status = MESSAGE_TABLE[OBJECT_LIMIT_MESSAGE_INDEX].status;
     }
-
   } else {
     // PropertyMirror, InternalPropertyMirror, FrameMirror, ScriptMirror
     data.value = 'unknown mirror type';
@@ -510,53 +528,68 @@ StateResolver.prototype.storeObjectToVariableTable_ = function(obj) {
  *
  * See https://github.com/iojs/io.js/issues/1190.
  */
-StateResolver.prototype.resolveMirror_ = function(mirror) {
+StateResolver.prototype.resolveMirror_ = function(mirror, isEvaluated) {
   if (semver.satisfies(process.version, '<1.6')) {
-    return this.resolveMirrorSlow_(mirror);
+    return this.resolveMirrorSlow_(mirror, isEvaluated);
   } else {
-    return this.resolveMirrorFast_(mirror);
+    return this.resolveMirrorFast_(mirror, isEvaluated);
   }
 };
 
 // A slower implementation of resolveMirror_ which is safe for all node versions
-StateResolver.prototype.resolveMirrorSlow_ = function(mirror) {
+StateResolver.prototype.resolveMirrorSlow_ = function(mirror, isEvaluated) {
   // Instead, let's use Object.keys. This will only get the enumerable
   // properties. The other alternative would be Object.getOwnPropertyNames, but
   // I'm going with the former as that's what util.inspect does.
   var that = this;
+
   var keys = Object.keys(mirror.value());
-  if (that.config_.capture.maxProperties) {
-    keys = keys.slice(0, that.config_.capture.maxProperties);
+  var maxProps = that.config_.capture.maxProperties;
+
+  if (!isEvaluated && maxProps) {
+    keys = keys.slice(0, maxProps);
   }
   var members = keys.map(function(prop) {
-    return that.resolveMirrorProperty_(mirror.property(prop));
+    return that.resolveMirrorProperty_(mirror.property(prop), isEvaluated);
   });
 
+  var mirrorVal = mirror.value();
+  var len = mirrorVal && mirrorVal.length;
   return {
-    value: mirror.toText(),
+    value: mirror.toText() + 
+      ((typeof len === 'undefined') ? '' : ' of length ' + len),
     members: members
   };
 };
 
 // A faster implementation of resolveMirror_ which segfaults in node <1.6
 //
 // See https://github.com/iojs/io.js/issues/1190.
-StateResolver.prototype.resolveMirrorFast_ = function(mirror) {
-  var members = this.getMirrorProperties_(mirror).map(
-      this.resolveMirrorProperty_.bind(this));
+StateResolver.prototype.resolveMirrorFast_ = function(mirror, isEvaluated) {
+  var resMirrorProp = this.resolveMirrorProperty_.bind(this);
+  var members = this.getMirrorProperties_(mirror, isEvaluated).map(
+        function(mirror) {
+          return resMirrorProp(mirror, isEvaluated);
+        }
+      );
   return {
     value: mirror.toText(),
     members: members
   };
 };
 
-StateResolver.prototype.getMirrorProperties_ = function(mirror) {
-  var numProperties = this.config_.capture.maxProperties;
+StateResolver.prototype.getMirrorProperties_ = function(mirror, isEvaluated) {
+  var maxProperties = this.config_.capture.maxProperties;
   var properties = mirror.properties();
-  return numProperties ? properties.slice(0, numProperties) : properties;
+
+  if (!isEvaluated && maxProperties) {
+    return properties.slice(0, maxProperties);
+  }
+
+  return properties;
 };
 
-StateResolver.prototype.resolveMirrorProperty_ = function(property) {
+StateResolver.prototype.resolveMirrorProperty_ = function(property, isEvaluated) {
   var name = String(property.name());
   // Array length must be special cased as it is a native property that
   // we know to be safe to evaluate which is not generally true.
@@ -573,5 +606,5 @@ StateResolver.prototype.resolveMirrorProperty_ = function(property) {
       varTableIndex: GETTER_MESSAGE_INDEX
     };
   }
-  return this.resolveVariable_(name, property.value());
+  return this.resolveVariable_(name, property.value(), isEvaluated);
 };
diff --git a/test/standalone/test-try-catch.js b/test/standalone/test-try-catch.js
@@ -91,7 +91,7 @@ describe('v8debugapi', function() {
         } else {
           assert.deepEqual(
             locals[0],
-            {name: 'e', varTableIndex: 6}
+            {name: 'e', varTableIndex: 7}
           );
         }
         
diff --git a/test/test-v8debugapi.js b/test/test-v8debugapi.js

Original file line number	Diff line number	Diff line change
`@@ -91,7 +91,7 @@ describe('v8debugapi', function() {`
`91`	`91`	`} else {`
`92`	`92`	`assert.deepEqual(`
`93`	`93`	`locals[0],`
`94`		`- {name: 'e', varTableIndex: 6}`
	`94`	`+ {name: 'e', varTableIndex: 7}`
`95`	`95`	`);`
`96`	`96`	`}`
`97`	`97`