Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Commit

Permalink
Evaluated expressions respect capture.maxProperties (#174)
Browse files Browse the repository at this point in the history 
Now whether or not a captured variable is an evaluated
expression, the `config.capture.maxProperties` configuration will
be respected when displaying the contents of the variable.
  • Loading branch information
@DominicKramer @ofrobots
DominicKramer authored and ofrobots committed Nov 13, 2016
1 parent 2a131c2 commit b89e31c
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 101 deletions.
61 changes: 22 additions & 39 deletions lib/state.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ StateResolver.prototype.capture_ = function() {
var that = this;

// 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));
Expand All @@ -162,11 +161,7 @@ StateResolver.prototype.capture_ = function() {
result.error, true)
};
} else {
evaluated = that.resolveVariable_(expression, result.mirror, true);
var varTableIdx = evaluated.varTableIndex;
if (typeof varTableIdx !== 'undefined'){
evalIndexSet.add(varTableIdx);
}
evaluated = that.resolveVariable_(expression, result.mirror);
}
that.evaluatedExpressions_[index] = evaluated;
});
Expand All @@ -183,9 +178,8 @@ StateResolver.prototype.capture_ = function() {
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], isEvaluated);
that.resolveMirror_(that.rawVariableTable_[index]);
index++;
}

Expand Down Expand Up @@ -365,7 +359,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, false);
return resolveVariable(arg.name, arg.value);
});
};

Expand Down Expand Up @@ -429,12 +423,12 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
// locals list.
remove(args, {name: name});
usedNames[name] = true;
locals.push(self.resolveVariable_(name, trg, false));
locals.push(self.resolveVariable_(name, trg));
} 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, false));
locals.push(self.resolveVariable_(name, trg));
} // otherwise another same-named variable occured at a lower scope
return locals;
},
Expand All @@ -448,7 +442,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), false)];
return [self.resolveVariable_('context', makeMirror(ctx))];
}
return [];
}()));
Expand All @@ -461,10 +455,8 @@ 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, isEvaluated) {
StateResolver.prototype.resolveVariable_ = function(name, value) {
var size = name.length;

var data = {
Expand All @@ -488,7 +480,7 @@ StateResolver.prototype.resolveVariable_ = function(name, value, isEvaluated) {
var maxProps = this.config_.capture.maxProperties;
var numKeys = Object.keys(value.value()).length;

if (!isEvaluated && maxProps && maxProps < numKeys) {
if (maxProps && maxProps < numKeys) {
data.status = MESSAGE_TABLE[OBJECT_LIMIT_MESSAGE_INDEX].status;
}
} else {
Expand Down Expand Up @@ -529,16 +521,16 @@ StateResolver.prototype.storeObjectToVariableTable_ = function(obj) {
*
* See https://github.com/iojs/io.js/issues/1190.
*/
StateResolver.prototype.resolveMirror_ = function(mirror, isEvaluated) {
StateResolver.prototype.resolveMirror_ = function(mirror) {
if (semver.satisfies(process.version, '<1.6')) {
return this.resolveMirrorSlow_(mirror, isEvaluated);
return this.resolveMirrorSlow_(mirror);
} else {
return this.resolveMirrorFast_(mirror, isEvaluated);
return this.resolveMirrorFast_(mirror);
}
};

// A slower implementation of resolveMirror_ which is safe for all node versions
StateResolver.prototype.resolveMirrorSlow_ = function(mirror, isEvaluated) {
StateResolver.prototype.resolveMirrorSlow_ = function(mirror) {
// 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.
Expand All @@ -547,11 +539,11 @@ StateResolver.prototype.resolveMirrorSlow_ = function(mirror, isEvaluated) {
var keys = Object.keys(mirror.value());
var maxProps = that.config_.capture.maxProperties;

if (!isEvaluated && maxProps) {
if (maxProps) {
keys = keys.slice(0, maxProps);
}
var members = keys.map(function(prop) {
return that.resolveMirrorProperty_(mirror.property(prop), isEvaluated);
return that.resolveMirrorProperty_(mirror.property(prop));
});

var mirrorVal = mirror.value();
Expand All @@ -566,31 +558,22 @@ StateResolver.prototype.resolveMirrorSlow_ = function(mirror, isEvaluated) {
// 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, isEvaluated) {
var resMirrorProp = this.resolveMirrorProperty_.bind(this);
var members = this.getMirrorProperties_(mirror, isEvaluated).map(
function(mirror) {
return resMirrorProp(mirror, isEvaluated);
}
);
StateResolver.prototype.resolveMirrorFast_ = function(mirror) {
var members = this.getMirrorProperties_(mirror).map(
this.resolveMirrorProperty_.bind(this));
return {
value: mirror.toText(),
members: members
};
};

StateResolver.prototype.getMirrorProperties_ = function(mirror, isEvaluated) {
var maxProperties = this.config_.capture.maxProperties;
StateResolver.prototype.getMirrorProperties_ = function(mirror) {
var numProperties = this.config_.capture.maxProperties;
var properties = mirror.properties();

if (!isEvaluated && maxProperties) {
return properties.slice(0, maxProperties);
}

return properties;
return numProperties ? properties.slice(0, numProperties) : properties;
};

StateResolver.prototype.resolveMirrorProperty_ = function(property, isEvaluated) {
StateResolver.prototype.resolveMirrorProperty_ = function(property) {
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.
Expand All @@ -607,5 +590,5 @@ StateResolver.prototype.resolveMirrorProperty_ = function(property, isEvaluated)
varTableIndex: GETTER_MESSAGE_INDEX
};
}
return this.resolveVariable_(name, property.value(), isEvaluated);
return this.resolveVariable_(name, property.value());
};
111 changes: 49 additions & 62 deletions test/test-v8debugapi.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -743,42 +743,58 @@ describe('v8debugapi', function() {
});
});

it('should not limit the length of an evaluated array based on maxProperties',
function(done) {
var bp = {
id: 'fake-id-124',
location: { path: 'test-v8debugapi.js', line: 5 },
expressions: ['A']
};
var oldMaxProps = config.capture.maxProperties;
var oldMaxData = config.capture.maxDataSize;
config.capture.maxProperties = 1;
config.capture.maxDataSize = 20000;
api.set(bp, function(err) {
it('should limit array length', function(done) {
var bp = {
id: 'fake-id-124',
location: { path: 'test-v8debugapi.js', line: 5 },
expressions: ['A']
};
var oldMax = config.capture.maxProperties;
config.capture.maxProperties = 1;
api.set(bp, function(err) {
assert.ifError(err);
api.wait(bp, function(err) {
assert.ifError(err);
api.wait(bp, function(err) {
assert.ifError(err);
var foo = bp.evaluatedExpressions[0];
var fooVal = bp.variableTable[foo.varTableIndex];
if (semver.satisfies(process.version, '<1.6')) {
// In v0.12 there are members for the attributes
// '1', '2', and '3'
assert.equal(fooVal.members.length, 3);
}
else {
// After v0.12 there are members for the attributes
// '1', '2', '3', and 'length'
assert.equal(fooVal.members.length, 4);
}
assert.strictEqual(foo.status, undefined);
var foo = bp.evaluatedExpressions[0];
var fooVal = bp.variableTable[foo.varTableIndex];
assert.equal(fooVal.members.length, 1);
assert(foo.status.description.format.indexOf(
'Only first') !== -1);
assert(!foo.status.isError);

api.clear(bp);
config.capture.maxDataSize = oldMaxData;
config.capture.maxProperties = oldMaxProps;
done();
});
process.nextTick(function() {foo(2);});
api.clear(bp);
config.capture.maxProperties = oldMax;
done();
});
process.nextTick(function() {foo(2);});
});
});

it('should limit object length', function(done) {
var bp = {
id: 'fake-id-124',
location: { path: 'test-v8debugapi.js', line: 5 },
expressions: ['B']
};
var oldMax = config.capture.maxProperties;
config.capture.maxProperties = 1;
api.set(bp, function(err) {
assert.ifError(err);
api.wait(bp, function(err) {
assert.ifError(err);
var foo = bp.evaluatedExpressions[0];
var fooVal = bp.variableTable[foo.varTableIndex];
assert.equal(fooVal.members.length, 1);
assert(foo.status.description.format.indexOf(
'Only first') !== -1);
assert(!foo.status.isError);

api.clear(bp);
config.capture.maxProperties = oldMax;
done();
});
process.nextTick(function() {foo(2);});
});
});

it('should display an error for an evaluated array beyond maxDataSize',
Expand Down Expand Up @@ -811,35 +827,6 @@ describe('v8debugapi', function() {
});
});

it('should not limit the length of an evaluated object based on maxProperties',
function(done) {
var bp = {
id: 'fake-id-124',
location: { path: 'test-v8debugapi.js', line: 5 },
expressions: ['B']
};
var oldMaxProps = config.capture.maxProperties;
var oldMaxData = config.capture.maxDataSize;
config.capture.maxProperties = 1;
config.capture.maxDataSize = 20000;
api.set(bp, function(err) {
assert.ifError(err);
api.wait(bp, function(err) {
assert.ifError(err);
var foo = bp.evaluatedExpressions[0];
var fooVal = bp.variableTable[foo.varTableIndex];
assert.equal(fooVal.members.length, 3);
assert.strictEqual(foo.status, undefined);

api.clear(bp);
config.capture.maxDataSize = oldMaxData;
config.capture.maxProperties = oldMaxProps;
done();
});
process.nextTick(function() {foo(2);});
});
});

it('should display an error for an evaluated object beyond maxDataSize',
function(done) {
var bp = {
Expand Down

0 comments on commit b89e31c

Please sign in to comment.