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

Commit b89e31c

Browse files
DominicKramerofrobots
authored andcommitted
Evaluated expressions respect capture.maxProperties (#174)
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.
1 parent 2a131c2 commit b89e31c

File tree

2 files changed

+71
-101
lines changed

2 files changed

+71
-101
lines changed

lib/state.js

Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,6 @@ StateResolver.prototype.capture_ = function() {
149149
var that = this;
150150

151151
// Evaluate the watch expressions
152-
var evalIndexSet = new Set();
153152
if (that.expressions_) {
154153
that.expressions_.forEach(function(expression, index) {
155154
var result = evaluate(expression, that.state_.frame(0));
@@ -162,11 +161,7 @@ StateResolver.prototype.capture_ = function() {
162161
result.error, true)
163162
};
164163
} else {
165-
evaluated = that.resolveVariable_(expression, result.mirror, true);
166-
var varTableIdx = evaluated.varTableIndex;
167-
if (typeof varTableIdx !== 'undefined'){
168-
evalIndexSet.add(varTableIdx);
169-
}
164+
evaluated = that.resolveVariable_(expression, result.mirror);
170165
}
171166
that.evaluatedExpressions_[index] = evaluated;
172167
});
@@ -183,9 +178,8 @@ StateResolver.prototype.capture_ = function() {
183178
while (index < that.rawVariableTable_.length && // NOTE: length changes in loop
184179
(that.totalSize_ < that.config_.capture.maxDataSize || noLimit)) {
185180
assert(!that.resolvedVariableTable_[index]); // shouldn't have it resolved yet
186-
var isEvaluated = evalIndexSet.has(index);
187181
that.resolvedVariableTable_[index] =
188-
that.resolveMirror_(that.rawVariableTable_[index], isEvaluated);
182+
that.resolveMirror_(that.rawVariableTable_[index]);
189183
index++;
190184
}
191185

@@ -365,7 +359,7 @@ StateResolver.prototype.extractArgumentsList_ = function (frame) {
365359
StateResolver.prototype.resolveArgumentList_ = function(args) {
366360
var resolveVariable = this.resolveVariable_.bind(this);
367361
return args.map(function (arg){
368-
return resolveVariable(arg.name, arg.value, false);
362+
return resolveVariable(arg.name, arg.value);
369363
});
370364
};
371365

@@ -429,12 +423,12 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
429423
// locals list.
430424
remove(args, {name: name});
431425
usedNames[name] = true;
432-
locals.push(self.resolveVariable_(name, trg, false));
426+
locals.push(self.resolveVariable_(name, trg));
433427
} else if (!usedNames[name]) {
434428
// It's a valid variable that belongs in the locals list and wasn't
435429
// discovered at a lower-scope
436430
usedNames[name] = true;
437-
locals.push(self.resolveVariable_(name, trg, false));
431+
locals.push(self.resolveVariable_(name, trg));
438432
} // otherwise another same-named variable occured at a lower scope
439433
return locals;
440434
},
@@ -448,7 +442,7 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
448442
// under the name 'context' which is used by the Chrome DevTools.
449443
var ctx = frame.details().receiver();
450444
if (ctx) {
451-
return [self.resolveVariable_('context', makeMirror(ctx), false)];
445+
return [self.resolveVariable_('context', makeMirror(ctx))];
452446
}
453447
return [];
454448
}()));
@@ -461,10 +455,8 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
461455
*
462456
* @param {String} name The name of the variable.
463457
* @param {Object} value A v8 debugger representation of a variable value.
464-
* @param {boolean} isEvaluated Specifies if the variable is from a watched
465-
* expression.
466458
*/
467-
StateResolver.prototype.resolveVariable_ = function(name, value, isEvaluated) {
459+
StateResolver.prototype.resolveVariable_ = function(name, value) {
468460
var size = name.length;
469461

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

491-
if (!isEvaluated && maxProps && maxProps < numKeys) {
483+
if (maxProps && maxProps < numKeys) {
492484
data.status = MESSAGE_TABLE[OBJECT_LIMIT_MESSAGE_INDEX].status;
493485
}
494486
} else {
@@ -529,16 +521,16 @@ StateResolver.prototype.storeObjectToVariableTable_ = function(obj) {
529521
*
530522
* See https://github.com/iojs/io.js/issues/1190.
531523
*/
532-
StateResolver.prototype.resolveMirror_ = function(mirror, isEvaluated) {
524+
StateResolver.prototype.resolveMirror_ = function(mirror) {
533525
if (semver.satisfies(process.version, '<1.6')) {
534-
return this.resolveMirrorSlow_(mirror, isEvaluated);
526+
return this.resolveMirrorSlow_(mirror);
535527
} else {
536-
return this.resolveMirrorFast_(mirror, isEvaluated);
528+
return this.resolveMirrorFast_(mirror);
537529
}
538530
};
539531

540532
// A slower implementation of resolveMirror_ which is safe for all node versions
541-
StateResolver.prototype.resolveMirrorSlow_ = function(mirror, isEvaluated) {
533+
StateResolver.prototype.resolveMirrorSlow_ = function(mirror) {
542534
// Instead, let's use Object.keys. This will only get the enumerable
543535
// properties. The other alternative would be Object.getOwnPropertyNames, but
544536
// I'm going with the former as that's what util.inspect does.
@@ -547,11 +539,11 @@ StateResolver.prototype.resolveMirrorSlow_ = function(mirror, isEvaluated) {
547539
var keys = Object.keys(mirror.value());
548540
var maxProps = that.config_.capture.maxProperties;
549541

550-
if (!isEvaluated && maxProps) {
542+
if (maxProps) {
551543
keys = keys.slice(0, maxProps);
552544
}
553545
var members = keys.map(function(prop) {
554-
return that.resolveMirrorProperty_(mirror.property(prop), isEvaluated);
546+
return that.resolveMirrorProperty_(mirror.property(prop));
555547
});
556548

557549
var mirrorVal = mirror.value();
@@ -566,31 +558,22 @@ StateResolver.prototype.resolveMirrorSlow_ = function(mirror, isEvaluated) {
566558
// A faster implementation of resolveMirror_ which segfaults in node <1.6
567559
//
568560
// See https://github.com/iojs/io.js/issues/1190.
569-
StateResolver.prototype.resolveMirrorFast_ = function(mirror, isEvaluated) {
570-
var resMirrorProp = this.resolveMirrorProperty_.bind(this);
571-
var members = this.getMirrorProperties_(mirror, isEvaluated).map(
572-
function(mirror) {
573-
return resMirrorProp(mirror, isEvaluated);
574-
}
575-
);
561+
StateResolver.prototype.resolveMirrorFast_ = function(mirror) {
562+
var members = this.getMirrorProperties_(mirror).map(
563+
this.resolveMirrorProperty_.bind(this));
576564
return {
577565
value: mirror.toText(),
578566
members: members
579567
};
580568
};
581569

582-
StateResolver.prototype.getMirrorProperties_ = function(mirror, isEvaluated) {
583-
var maxProperties = this.config_.capture.maxProperties;
570+
StateResolver.prototype.getMirrorProperties_ = function(mirror) {
571+
var numProperties = this.config_.capture.maxProperties;
584572
var properties = mirror.properties();
585-
586-
if (!isEvaluated && maxProperties) {
587-
return properties.slice(0, maxProperties);
588-
}
589-
590-
return properties;
573+
return numProperties ? properties.slice(0, numProperties) : properties;
591574
};
592575

593-
StateResolver.prototype.resolveMirrorProperty_ = function(property, isEvaluated) {
576+
StateResolver.prototype.resolveMirrorProperty_ = function(property) {
594577
var name = String(property.name());
595578
// Array length must be special cased as it is a native property that
596579
// we know to be safe to evaluate which is not generally true.
@@ -607,5 +590,5 @@ StateResolver.prototype.resolveMirrorProperty_ = function(property, isEvaluated)
607590
varTableIndex: GETTER_MESSAGE_INDEX
608591
};
609592
}
610-
return this.resolveVariable_(name, property.value(), isEvaluated);
593+
return this.resolveVariable_(name, property.value());
611594
};

test/test-v8debugapi.js

Lines changed: 49 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -743,42 +743,58 @@ describe('v8debugapi', function() {
743743
});
744744
});
745745

746-
it('should not limit the length of an evaluated array based on maxProperties',
747-
function(done) {
748-
var bp = {
749-
id: 'fake-id-124',
750-
location: { path: 'test-v8debugapi.js', line: 5 },
751-
expressions: ['A']
752-
};
753-
var oldMaxProps = config.capture.maxProperties;
754-
var oldMaxData = config.capture.maxDataSize;
755-
config.capture.maxProperties = 1;
756-
config.capture.maxDataSize = 20000;
757-
api.set(bp, function(err) {
746+
it('should limit array length', function(done) {
747+
var bp = {
748+
id: 'fake-id-124',
749+
location: { path: 'test-v8debugapi.js', line: 5 },
750+
expressions: ['A']
751+
};
752+
var oldMax = config.capture.maxProperties;
753+
config.capture.maxProperties = 1;
754+
api.set(bp, function(err) {
755+
assert.ifError(err);
756+
api.wait(bp, function(err) {
758757
assert.ifError(err);
759-
api.wait(bp, function(err) {
760-
assert.ifError(err);
761-
var foo = bp.evaluatedExpressions[0];
762-
var fooVal = bp.variableTable[foo.varTableIndex];
763-
if (semver.satisfies(process.version, '<1.6')) {
764-
// In v0.12 there are members for the attributes
765-
// '1', '2', and '3'
766-
assert.equal(fooVal.members.length, 3);
767-
}
768-
else {
769-
// After v0.12 there are members for the attributes
770-
// '1', '2', '3', and 'length'
771-
assert.equal(fooVal.members.length, 4);
772-
}
773-
assert.strictEqual(foo.status, undefined);
758+
var foo = bp.evaluatedExpressions[0];
759+
var fooVal = bp.variableTable[foo.varTableIndex];
760+
assert.equal(fooVal.members.length, 1);
761+
assert(foo.status.description.format.indexOf(
762+
'Only first') !== -1);
763+
assert(!foo.status.isError);
774764

775-
api.clear(bp);
776-
config.capture.maxDataSize = oldMaxData;
777-
config.capture.maxProperties = oldMaxProps;
778-
done();
779-
});
780-
process.nextTick(function() {foo(2);});
765+
api.clear(bp);
766+
config.capture.maxProperties = oldMax;
767+
done();
768+
});
769+
process.nextTick(function() {foo(2);});
770+
});
771+
});
772+
773+
it('should limit object length', function(done) {
774+
var bp = {
775+
id: 'fake-id-124',
776+
location: { path: 'test-v8debugapi.js', line: 5 },
777+
expressions: ['B']
778+
};
779+
var oldMax = config.capture.maxProperties;
780+
config.capture.maxProperties = 1;
781+
api.set(bp, function(err) {
782+
assert.ifError(err);
783+
api.wait(bp, function(err) {
784+
assert.ifError(err);
785+
var foo = bp.evaluatedExpressions[0];
786+
var fooVal = bp.variableTable[foo.varTableIndex];
787+
assert.equal(fooVal.members.length, 1);
788+
assert(foo.status.description.format.indexOf(
789+
'Only first') !== -1);
790+
assert(!foo.status.isError);
791+
792+
api.clear(bp);
793+
config.capture.maxProperties = oldMax;
794+
done();
781795
});
796+
process.nextTick(function() {foo(2);});
797+
});
782798
});
783799

784800
it('should display an error for an evaluated array beyond maxDataSize',
@@ -811,35 +827,6 @@ describe('v8debugapi', function() {
811827
});
812828
});
813829

814-
it('should not limit the length of an evaluated object based on maxProperties',
815-
function(done) {
816-
var bp = {
817-
id: 'fake-id-124',
818-
location: { path: 'test-v8debugapi.js', line: 5 },
819-
expressions: ['B']
820-
};
821-
var oldMaxProps = config.capture.maxProperties;
822-
var oldMaxData = config.capture.maxDataSize;
823-
config.capture.maxProperties = 1;
824-
config.capture.maxDataSize = 20000;
825-
api.set(bp, function(err) {
826-
assert.ifError(err);
827-
api.wait(bp, function(err) {
828-
assert.ifError(err);
829-
var foo = bp.evaluatedExpressions[0];
830-
var fooVal = bp.variableTable[foo.varTableIndex];
831-
assert.equal(fooVal.members.length, 3);
832-
assert.strictEqual(foo.status, undefined);
833-
834-
api.clear(bp);
835-
config.capture.maxDataSize = oldMaxData;
836-
config.capture.maxProperties = oldMaxProps;
837-
done();
838-
});
839-
process.nextTick(function() {foo(2);});
840-
});
841-
});
842-
843830
it('should display an error for an evaluated object beyond maxDataSize',
844831
function(done) {
845832
var bp = {

0 commit comments

Comments
 (0)