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

Commit 36d9a7b

Browse files
ofrobotsofrobots
authored
improve UX for truncated objects properties (#175)
Change how we indicate that an object with too many properties has been truncated on our state captures. Instead of a non-error StatusMessage add an extra member with no value / varTableIndex. This matches how the Java agent deals with this situation.
1 parent 9b6961d commit 36d9a7b

File tree

3 files changed

+37
-48
lines changed

3 files changed

+37
-48
lines changed

‎lib/state.js‎

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,8 @@ var BUFFER_FULL_MESSAGE_INDEX = 0;
3939
var NATIVE_PROPERTY_MESSAGE_INDEX = 1;
4040
var GETTER_MESSAGE_INDEX = 2;
4141
var ARG_LOCAL_LIMIT_MESSAGE_INDEX = 3;
42-
var OBJECT_LIMIT_MESSAGE_INDEX = 4;
43-
var STRING_LIMIT_MESSAGE_INDEX = 5;
44-
var DATA_LIMIT_MESSAGE_INDEX = 6;
42+
var STRING_LIMIT_MESSAGE_INDEX = 4;
43+
var DATA_LIMIT_MESSAGE_INDEX = 5;
4544

4645
var MESSAGE_TABLE = [];
4746
MESSAGE_TABLE[BUFFER_FULL_MESSAGE_INDEX] =
@@ -58,11 +57,6 @@ MESSAGE_TABLE[ARG_LOCAL_LIMIT_MESSAGE_INDEX] =
5857
'Locals and arguments are only displayed for the ' +
5958
'top `config.capture.maxExpandFrames` stack frames.',
6059
true) };
61-
MESSAGE_TABLE[OBJECT_LIMIT_MESSAGE_INDEX] =
62-
{ status: new StatusMessage(StatusMessage.VARIABLE_VALUE,
63-
'Only first `config.capture.maxProperties` elements' +
64-
' were captured.',
65-
false) };
6660
MESSAGE_TABLE[STRING_LIMIT_MESSAGE_INDEX] =
6761
{ status: new StatusMessage(StatusMessage.VARIABLE_VALUE,
6862
'Only first `config.capture.maxStringLength` chars' +
@@ -474,15 +468,8 @@ StateResolver.prototype.resolveVariable_ = function(name, value) {
474468

475469
} else if (value.isFunction()) {
476470
data.value = 'function ' + this.resolveFunctionName_(value) + '()';
477-
478471
} else if (value.isObject()) {
479472
data.varTableIndex = this.getVariableIndex_(value);
480-
var maxProps = this.config_.capture.maxProperties;
481-
var numKeys = Object.keys(value.value()).length;
482-
483-
if (maxProps && maxProps < numKeys) {
484-
data.status = MESSAGE_TABLE[OBJECT_LIMIT_MESSAGE_INDEX].status;
485-
}
486473
} else {
487474
// PropertyMirror, InternalPropertyMirror, FrameMirror, ScriptMirror
488475
data.value = 'unknown mirror type';
@@ -538,18 +525,22 @@ StateResolver.prototype.resolveMirrorSlow_ = function(mirror) {
538525

539526
var keys = Object.keys(mirror.value());
540527
var maxProps = that.config_.capture.maxProperties;
541-
542-
if (maxProps) {
528+
var truncate = maxProps && keys.length > maxProps;
529+
if (truncate) {
543530
keys = keys.slice(0, maxProps);
544531
}
545532
var members = keys.map(function(prop) {
546533
return that.resolveMirrorProperty_(mirror.property(prop));
547534
});
535+
if (truncate) {
536+
members.push({name: 'Only first `config.capture.maxProperties` ' +
537+
'properties were captured'});
538+
}
548539

549540
var mirrorVal = mirror.value();
550541
var len = mirrorVal && mirrorVal.length;
551542
return {
552-
value: mirror.toText() +
543+
value: mirror.toText() +
553544
((typeof len === 'undefined') ? '' : ' of length ' + len),
554545
members: members
555546
};
@@ -559,20 +550,22 @@ StateResolver.prototype.resolveMirrorSlow_ = function(mirror) {
559550
//
560551
// See https://github.com/iojs/io.js/issues/1190.
561552
StateResolver.prototype.resolveMirrorFast_ = function(mirror) {
562-
var members = this.getMirrorProperties_(mirror).map(
563-
this.resolveMirrorProperty_.bind(this));
553+
var properties = mirror.properties();
554+
var maxProps = this.config_.capture.maxProperties;
555+
var truncate = maxProps && properties.length > maxProps;
556+
if (truncate) {
557+
properties = properties.slice(0, maxProps);
558+
}
559+
var members = properties.map(this.resolveMirrorProperty_.bind(this));
560+
if (truncate) {
561+
members.push({name: 'Only first maxProperties properties were captured'});
562+
}
564563
return {
565564
value: mirror.toText(),
566565
members: members
567566
};
568567
};
569568

570-
StateResolver.prototype.getMirrorProperties_ = function(mirror) {
571-
var numProperties = this.config_.capture.maxProperties;
572-
var properties = mirror.properties();
573-
return numProperties ? properties.slice(0, numProperties) : properties;
574-
};
575-
576569
StateResolver.prototype.resolveMirrorProperty_ = function(property) {
577570
var name = String(property.name());
578571
// Array length must be special cased as it is a native property that

‎test/standalone/test-try-catch.js‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,13 @@ describe('v8debugapi', function() {
8989
{name: 'e', value: 'undefined'}
9090
);
9191
} else {
92-
assert.deepEqual(
93-
locals[0],
94-
{name: 'e', varTableIndex: 7}
95-
);
92+
var e = locals[0];
93+
assert(e.name === 'e');
94+
assert(Number.isInteger(e.varTableIndex));
9695
}
97-
98-
assert.deepEqual(
99-
args[0],
100-
{name: 'arguments_not_available', varTableIndex: 3}
101-
);
96+
var arg0 = args[0];
97+
assert(arg0.name === 'arguments_not_available');
98+
assert(Number.isInteger(arg0.varTableIndex));
10299
api.clear(brk);
103100
done();
104101
});
@@ -130,10 +127,9 @@ describe('v8debugapi', function() {
130127
locals[0],
131128
{name: 'e', value: '2'}
132129
);
133-
assert.deepEqual(
134-
args[0],
135-
{name: 'arguments_not_available', varTableIndex: 3}
136-
);
130+
var arg0 = args[0];
131+
assert(arg0.name === 'arguments_not_available');
132+
assert(Number.isInteger(arg0.varTableIndex));
137133
}
138134
api.clear(brk);
139135
done();

‎test/test-v8debugapi.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,9 @@ describe('v8debugapi', function() {
660660
assert.equal(procEnv.name, 'process.env');
661661
var envVal = bp.variableTable[procEnv.varTableIndex];
662662
envVal.members.forEach(function(member) {
663-
assert(bp.variableTable[member.varTableIndex].status.isError);
663+
if (member.hasOwnProperty('varTableIndex')) {
664+
assert(bp.variableTable[member.varTableIndex].status.isError);
665+
}
664666
});
665667
var hasGetter = bp.evaluatedExpressions[1];
666668
var getterVal = bp.variableTable[hasGetter.varTableIndex];
@@ -757,10 +759,9 @@ describe('v8debugapi', function() {
757759
assert.ifError(err);
758760
var foo = bp.evaluatedExpressions[0];
759761
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);
762+
// should have 1 element + truncation message.
763+
assert.equal(fooVal.members.length, 2);
764+
assert(fooVal.members[1].name.indexOf('Only first') !== -1);
764765

765766
api.clear(bp);
766767
config.capture.maxProperties = oldMax;
@@ -784,10 +785,9 @@ describe('v8debugapi', function() {
784785
assert.ifError(err);
785786
var foo = bp.evaluatedExpressions[0];
786787
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);
788+
// should have 1 element + truncation message
789+
assert.equal(fooVal.members.length, 2);
790+
assert(fooVal.members[1].name.indexOf('Only first') !== -1);
791791

792792
api.clear(bp);
793793
config.capture.maxProperties = oldMax;

0 commit comments

Comments
 (0)