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

Commit

Permalink
include variables from outer scopes (#271)
Browse files Browse the repository at this point in the history
We were previously omitting variables from closure scopes. This can be
problematic specifically in transpiled async functions as most actual
variables end up in a closure.
  • Loading branch information
ofrobots committed Jun 7, 2017
1 parent adcf3ae commit 37d7745
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 16 deletions.
42 changes: 27 additions & 15 deletions src/agent/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ StateResolver.prototype.capture_ = function() {
});
}

// The frames are resolved after the evaluated expressions so that
// evaluated expressions can be evaluated as much as possible within
// 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_();

Expand Down Expand Up @@ -220,7 +220,7 @@ StateResolver.prototype.resolveFrames_ = function() {
var frames = [];
var frameCount = Math.min(this.state_.frameCount(),
this.config_.capture.maxFrames);

for (var i = 0; i < frameCount; i++) {
var frame = this.state_.frame(i);
if (this.shouldFrameBeResolved_(frame)) {
Expand Down Expand Up @@ -343,7 +343,7 @@ StateResolver.prototype.extractArgumentsList_ = function (frame) {

/**
* Iterates and returns variable information for all scopes (excluding global)
* in a given frame. FrameMirrors should return their scope object list with
* in a given frame. FrameMirrors should return their scope object list with
* most deeply nested scope first so variables initially encountered will take
* precedence over subsequent instance with the same name - this is tracked in
* the usedNames map. The argument list given to this function may be
Expand All @@ -360,17 +360,29 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
var self = this;
var usedNames = {};
var makeMirror = this.ctx_.MakeMirror;
return flatten(frame.allScopes().map(
const allScopes = frame.allScopes();
const count = allScopes.length;

// There will always be at least 3 scopes.
// For top-level breakpoints: [local, script, global]
// Other: [..., closure (module IIFE), script, global]
assert(count >= 3);
assert.strictEqual(allScopes[count - 1].scopeType(), ScopeType.Global);
assert.strictEqual(allScopes[count - 2].scopeType(), ScopeType.Script);

// We find the top-level (module global) variable pollute the local variables
// we omit them by default, unless the breakpoint itself is top-level. The
// last two scopes are always omitted.
let scopes;
if (allScopes[count - 3].scopeType() === ScopeType.Closure) {
scopes = allScopes.slice(0, -3);
} else {
assert(allScopes[count - 3].scopeType() === ScopeType.Local);
scopes = allScopes.slice(0, -2);
}

return flatten(scopes.map(
function (scope) {
switch (scope.scopeType()) {
case ScopeType.Global:
// We do not capture globals in the debug client.
case ScopeType.Closure:
// The closure scope is contaminated by Node.JS's require IIFE pattern
// and, if traversed, will cause local variable pools to include what
// are considered node 'globals'. Therefore, avoid traversal.
return [];
}
return transform(
scope.details().object(),
function (locals, value, name) {
Expand All @@ -387,7 +399,7 @@ StateResolver.prototype.resolveLocalsList_ = function (frame, args) {
);
}
)).concat((function () {
// The frame receiver is the 'this' context that is present during
// The frame receiver is the 'this' context that is present during
// invocation. Check to see whether a receiver context is substantive,
// (invocations may be bound to null) if so: store in the locals list
// under the name 'context' which is used by the Chrome DevTools.
Expand Down
77 changes: 77 additions & 0 deletions test/fixtures/ts/async.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// compile with
// $ tsc --lib es6 --target es5 async.ts
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new (P || (P = Promise))(function (resolve, reject) {
function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
step((generator = generator.apply(thisArg, _arguments || [])).next());
});
};
var __generator = (this && this.__generator) || function (thisArg, body) {
var _ = { label: 0, sent: function() { if (t[0] & 1) throw t[1]; return t[1]; }, trys: [], ops: [] }, f, y, t, g;
return g = { next: verb(0), "throw": verb(1), "return": verb(2) }, typeof Symbol === "function" && (g[Symbol.iterator] = function() { return this; }), g;
function verb(n) { return function (v) { return step([n, v]); }; }
function step(op) {
if (f) throw new TypeError("Generator is already executing.");
while (_) try {
if (f = 1, y && (t = y[op[0] & 2 ? "return" : op[0] ? "throw" : "next"]) && !(t = t.call(y, op[1])).done) return t;
if (y = 0, t) op = [0, t.value];
switch (op[0]) {
case 0: case 1: t = op; break;
case 4: _.label++; return { value: op[1], done: false };
case 5: _.label++; y = op[1]; op = [0]; continue;
case 7: op = _.ops.pop(); _.trys.pop(); continue;
default:
if (!(t = _.trys, t = t.length > 0 && t[t.length - 1]) && (op[0] === 6 || op[0] === 2)) { _ = 0; continue; }
if (op[0] === 3 && (!t || (op[1] > t[0] && op[1] < t[3]))) { _.label = op[1]; break; }
if (op[0] === 6 && _.label < t[1]) { _.label = t[1]; t = op; break; }
if (t && _.label < t[2]) { _.label = t[2]; _.ops.push(op); break; }
if (t[2]) _.ops.pop();
_.trys.pop(); continue;
}
op = body.call(thisArg, _);
} catch (e) { op = [6, e]; y = 0; } finally { f = t = 0; }
if (op[0] & 5) throw op[1]; return { value: op[0] ? op[1] : void 0, done: true };
}
};
function delay(t) {
return new Promise(function (resolve, reject) {
setTimeout(resolve, t);
});
}
function get(path, handler) {
var _this = this;
delay(10).then(function () {
var req = {
name: 'fake request',
path: path
};
var res = {
name: 'fake response',
status: function (statusCode) {
_this.statusCode = statusCode;
return _this;
},
send: function (msg) {
console.log(msg);
}
};
handler(req, res);
});
}
function run() {
var _this = this;
get('/foo', function (req, res) { return __awaiter(_this, void 0, void 0, function () {
return __generator(this, function (_a) {
switch (_a.label) {
case 0: return [4 /*yield*/, delay(10)];
case 1:
_a.sent();
res.status(200);
return [2 /*return*/];
}
});
}); });
}
module.exports = run;
37 changes: 37 additions & 0 deletions test/fixtures/ts/async.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// compile with
// $ tsc --lib es6 --target es5 async.ts

function delay(t) {
return new Promise((resolve, reject) => {
setTimeout(resolve, t);
});
}

function get(path, handler) {
delay(10).then(() => {
const req = {
name: 'fake request',
path: path
};
const res = {
name: 'fake response',
status: (statusCode) => {
this.statusCode = statusCode;
return this;
},
send: (msg) => {
console.log(msg);
}
}
handler(req, res);
});
}

function run() {
get('/foo', async (req, res) => {
await delay(10);
res.status(200);
});
}

module.exports = run;
4 changes: 3 additions & 1 deletion test/test-max-data-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ describe('maxDataSize', function() {
api.wait(bp, function(err) {
assert.ifError(err);
assert(bp.variableTable.reduce(function(acc, elem) {
return acc && elem.status.description.format !== 'Max data size reached';
return acc &&
(!elem.status ||
elem.status.description.format !== 'Max data size reached');
}), true);
api.clear(bp);
done();
Expand Down
26 changes: 26 additions & 0 deletions test/test-v8debugapi.js
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,32 @@ describe('v8debugapi', function() {
process.nextTick(function() {foo(1);});
});
});

it('should capture state in transpiled TS async functions', (done) => {
const bp = {
id: 'async-id-1',
location: {
path: path.join('.', 'test', 'fixtures', 'ts', 'async.js'),
line: 71
}
};

const run = require('./fixtures/ts/async.js');
api.set(bp, (err) => {
assert.ifError(err);
api.wait(bp, (err) => {
assert.ifError(err);
assert.ok(bp.stackFrames);

const topFrame = bp.stackFrames[0];
assert.ok(topFrame.locals.some((local) => (local.name === '_a')));
assert.ok(topFrame.locals.some((local) => (local.name === 'res')));
api.clear(bp);
done();
});
});
process.nextTick(run);
});
});

it('should be possible to set deferred breakpoints');
Expand Down

0 comments on commit 37d7745

Please sign in to comment.