From 53562ccc89e2fabfb9189ac0716ccf4f3dfee506 Mon Sep 17 00:00:00 2001 From: Matthew Loring Date: Tue, 6 Jun 2017 15:19:25 -0700 Subject: [PATCH] Update truncated object message (#269) This change additionally avoids truncating evaluated expressions and makes behavior more consistent with other language debug agents. PR-URL: https://github.com/GoogleCloudPlatform/cloud-debug-nodejs/pull/269 --- src/agent/state.js | 48 ++++++++------- test/test-v8debugapi.js | 132 ++++++++++++++++++++++++++++++++++------ 2 files changed, 141 insertions(+), 39 deletions(-) diff --git a/src/agent/state.js b/src/agent/state.js index 9ddc3376..6dcfab3f 100644 --- a/src/agent/state.js +++ b/src/agent/state.js @@ -36,7 +36,6 @@ var BUFFER_FULL_MESSAGE_INDEX = 0; var NATIVE_PROPERTY_MESSAGE_INDEX = 1; var GETTER_MESSAGE_INDEX = 2; var ARG_LOCAL_LIMIT_MESSAGE_INDEX = 3; -var STRING_LIMIT_MESSAGE_INDEX = 4; /** * Captures the stack and current execution state. @@ -115,12 +114,6 @@ function StateResolver(execState, expressions, config, v8) { config.capture.maxExpandFrames + '` stack frames.', true) }; - this.messageTable_[STRING_LIMIT_MESSAGE_INDEX] = - { status: new StatusMessage(StatusMessage.VARIABLE_VALUE, - 'Only first `config.capture.maxStringLength=' + - config.capture.maxStringLength + - '` chars were captured.', - false) }; this.resolvedVariableTable_ = util._extend([], this.messageTable_); this.rawVariableTable_ = this.messageTable_.map(function() { return null; }); @@ -137,6 +130,7 @@ 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)); @@ -149,7 +143,11 @@ 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; }); @@ -166,8 +164,9 @@ 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]); + that.resolveMirror_(that.rawVariableTable_[index], isEvaluated); index++; } @@ -380,7 +379,7 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) { // 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; }, @@ -394,7 +393,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 []; }())); @@ -407,8 +406,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 = { @@ -419,9 +420,13 @@ StateResolver.prototype.resolveVariable_ = function(name, value) { // primitives: undefined, null, boolean, number, string, symbol data.value = value.toText(); var maxLength = this.config_.capture.maxStringLength; - if (maxLength && maxLength < data.value.length) { + if (!isEvaluated && maxLength && maxLength < data.value.length) { + data.status = new StatusMessage(StatusMessage.VARIABLE_VALUE, + 'Only first `config.capture.maxStringLength=' + + this.config_.capture.maxStringLength + + '` chars were captured for string of length ' + data.value.length + + '. Use in an expression to see the full string.', false); data.value = data.value.substring(0, maxLength) + '...'; - data.status = this.messageTable_[STRING_LIMIT_MESSAGE_INDEX].status; } } else if (value.isFunction()) { @@ -462,18 +467,19 @@ StateResolver.prototype.storeObjectToVariableTable_ = function(obj) { * Responsible for recursively resolving the properties on a * provided object mirror. */ -StateResolver.prototype.resolveMirror_ = function(mirror) { +StateResolver.prototype.resolveMirror_ = function(mirror, isEvaluated) { var properties = mirror.properties(); var maxProps = this.config_.capture.maxProperties; var truncate = maxProps && properties.length > maxProps; - if (truncate) { + if (!isEvaluated && truncate) { properties = properties.slice(0, maxProps); } - var members = properties.map(this.resolveMirrorProperty_.bind(this)); - if (truncate) { + var members = properties.map(this.resolveMirrorProperty_.bind(this, isEvaluated)); + if (!isEvaluated && truncate) { members.push({name: 'Only first `config.capture.maxProperties=' + this.config_.capture.maxProperties + - '` properties were captured'}); + '` properties were captured. Use in an expression' + + ' to see all properties.'}); } return { value: mirror.toText(), @@ -481,7 +487,7 @@ StateResolver.prototype.resolveMirror_ = function(mirror) { }; }; -StateResolver.prototype.resolveMirrorProperty_ = function(property) { +StateResolver.prototype.resolveMirrorProperty_ = function(isEvaluated, 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. @@ -498,5 +504,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/test-v8debugapi.js b/test/test-v8debugapi.js index a40837e7..ae5eff6e 100644 --- a/test/test-v8debugapi.js +++ b/test/test-v8debugapi.js @@ -710,8 +710,7 @@ describe('v8debugapi', function() { it('should limit string length', function(done) { var bp = { id: 'fake-id-124', - location: { path: 'test-v8debugapi.js', line: 9 }, - expressions: ['hasGetter'] + location: { path: 'test-v8debugapi.js', line: 9 } }; var oldMaxLength = config.capture.maxStringLength; var oldMaxData = config.capture.maxDataSize; @@ -721,8 +720,10 @@ describe('v8debugapi', function() { assert.ifError(err); api.wait(bp, function(err) { assert.ifError(err); - var hasGetter = bp.evaluatedExpressions[0]; - var getterVal = bp.variableTable[hasGetter.varTableIndex]; + var hasGetter = bp.stackFrames[0].locals.filter(function(value) { + return value.name === 'hasGetter'; + }); + var getterVal = bp.variableTable[hasGetter[0].varTableIndex]; var stringItems = getterVal.members.filter(function(m) { return m.value === 'hel...'; }); @@ -732,6 +733,7 @@ describe('v8debugapi', function() { assert(item.status.description.format.indexOf('Only first') !== -1); assert(item.status.description.format.indexOf( 'config.capture.maxStringLength=3') !== -1); + assert(item.status.description.format.indexOf('of length 11.') !== -1); api.clear(bp); config.capture.maxDataSize = oldMaxData; @@ -745,8 +747,7 @@ describe('v8debugapi', function() { it('should limit array length', function(done) { var bp = { id: 'fake-id-124', - location: { path: 'test-v8debugapi.js', line: 5 }, - expressions: ['A'] + location: { path: 'test-v8debugapi.js', line: 5 } }; var oldMax = config.capture.maxProperties; config.capture.maxProperties = 1; @@ -754,12 +755,14 @@ describe('v8debugapi', function() { assert.ifError(err); api.wait(bp, function(err) { assert.ifError(err); - var foo = bp.evaluatedExpressions[0]; - var fooVal = bp.variableTable[foo.varTableIndex]; + var aResults = bp.stackFrames[0].locals.filter(function(value) { + return value.name === 'A'; + }); + var aVal = bp.variableTable[aResults[0].varTableIndex]; // should have 1 element + truncation message. - assert.equal(fooVal.members.length, 2); - assert(fooVal.members[1].name.indexOf('Only first') !== -1); - assert(fooVal.members[1].name.indexOf( + assert.equal(aVal.members.length, 2); + assert(aVal.members[1].name.indexOf('Only first') !== -1); + assert(aVal.members[1].name.indexOf( 'config.capture.maxProperties=1') !== -1); api.clear(bp); @@ -773,8 +776,7 @@ describe('v8debugapi', function() { it('should limit object length', function(done) { var bp = { id: 'fake-id-124', - location: { path: 'test-v8debugapi.js', line: 5 }, - expressions: ['B'] + location: { path: 'test-v8debugapi.js', line: 5 } }; var oldMax = config.capture.maxProperties; config.capture.maxProperties = 1; @@ -782,12 +784,14 @@ describe('v8debugapi', function() { assert.ifError(err); api.wait(bp, function(err) { assert.ifError(err); - var foo = bp.evaluatedExpressions[0]; - var fooVal = bp.variableTable[foo.varTableIndex]; + var bResults = bp.stackFrames[0].locals.filter(function(value) { + return value.name === 'B'; + }); + var bVal = bp.variableTable[bResults[0].varTableIndex]; // should have 1 element + truncation message - assert.equal(fooVal.members.length, 2); - assert(fooVal.members[1].name.indexOf('Only first') !== -1); - assert(fooVal.members[1].name.indexOf( + assert.equal(bVal.members.length, 2); + assert(bVal.members[1].name.indexOf('Only first') !== -1); + assert(bVal.members[1].name.indexOf( 'config.capture.maxProperties=1') !== -1); api.clear(bp); @@ -798,6 +802,98 @@ describe('v8debugapi', function() { }); }); + it('should not limit the length of an evaluated string based on maxStringLength', + function(done) { + var bp = { + id: 'fake-id-124', + location: { path: 'test-v8debugapi.js', line: 9 }, + expressions: ['hasGetter'] + }; + var oldMaxLength = config.capture.maxStringLength; + var oldMaxData = config.capture.maxDataSize; + config.capture.maxStringLength = 3; + config.capture.maxDataSize = 20000; + api.set(bp, function(err) { + assert.ifError(err); + api.wait(bp, function(err) { + assert.ifError(err); + var hasGetter = bp.evaluatedExpressions[0]; + var getterVal = bp.variableTable[hasGetter.varTableIndex]; + var stringItems = getterVal.members.filter(function(m) { + return m.value === 'hello world'; + }); + // The property would have value 'hel...' if truncation occured + // resulting in stringItems.length being 0. + assert(stringItems.length === 1); + + api.clear(bp); + config.capture.maxDataSize = oldMaxData; + config.capture.maxStringLength = oldMaxLength; + done(); + }); + process.nextTick(function() {getterObject();}); + }); + }); + + 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) { + assert.ifError(err); + api.wait(bp, function(err) { + assert.ifError(err); + var foo = bp.evaluatedExpressions[0]; + var fooVal = bp.variableTable[foo.varTableIndex]; + // '1', '2', '3', and 'length' + assert.equal(fooVal.members.length, 4); + assert.strictEqual(foo.status, undefined); + + api.clear(bp); + config.capture.maxDataSize = oldMaxData; + config.capture.maxProperties = oldMaxProps; + done(); + }); + process.nextTick(function() {foo(2);}); + }); + }); + + 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 array beyond maxDataSize', function(done) { var bp = {