include variables from outer scopes (#271)

ofrobots · web-flow · commit 37d7745a6755 · 2017-06-06T19:23:39.000-07:00
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.
diff --git a/src/agent/state.js b/src/agent/state.js
@@ -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_();
 
@@ -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)) {
@@ -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
@@ -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) {
@@ -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.
diff --git a/test/fixtures/ts/async.js b/test/fixtures/ts/async.js
@@ -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;
diff --git a/test/fixtures/ts/async.ts b/test/fixtures/ts/async.ts
@@ -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;
diff --git a/test/test-max-data-size.js b/test/test-max-data-size.js
@@ -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();
diff --git a/test/test-v8debugapi.js b/test/test-v8debugapi.js
@@ -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');
