Warn on debug logpoints

Matt Loring · Matt Loring · commit 1db03bbc12ec · 2016-04-21T14:49:11.000-07:00
diff --git a/lib/apiclasses.js b/lib/apiclasses.js
@@ -32,6 +32,8 @@ function StatusMessage(refersTo, description, isError) {
   this.isError = isError;
 }
 
+// These status messages come from a proto definition.
+// New status messages cannot be added here.
 /** @const */ StatusMessage.UNSPECIFIED = 'UNSPECIFIED';
 /** @const */ StatusMessage.BREAKPOINT_SOURCE_LOCATION =
                 'BREAKPOINT_SOURCE_LOCATION';
diff --git a/lib/debuglet.js b/lib/debuglet.js
@@ -32,6 +32,8 @@ var assert = require('assert');
 
 var NODE_VERSION_MESSAGE = 'Node.js version not supported. Node.js 5.2.0 and ' +
   ' versions older than 0.12 are not supported.';
+var BREAKPOINT_ACTION_MESSAGE = 'The only currently supported breakpoint action' +
+  ' is CAPTURE.';
 
 module.exports = Debuglet;
 
@@ -215,7 +217,18 @@ Debuglet.prototype.scheduleBreakpointFetch_ = function(seconds) {
 
         default:
           that.logger_.info('\t' + response.statusCode + ' completed.');
-          that.updateActiveBreakpoints_(body.breakpoints || []);
+          var bps = (body.breakpoints || []).filter(function(bp) {
+            var action = bp.action || 'CAPTURE';
+            if (action !== 'CAPTURE') {
+              that.logger_.warn('Found breakpoint with invalid action:', action);
+              bp.status = new StatusMessage(StatusMessage.UNSPECIFIED,
+                BREAKPOINT_ACTION_MESSAGE, true);
+              that.rejectBreakpoint_(bp);
+              return false;
+            }
+            return true;
+          });
+          that.updateActiveBreakpoints_(bps);
           if (Object.keys(that.activeBreakpointMap_).length) {
             that.logger_.breakpoints(Logger.INFO, 'Active Breakpoints:',
               that.activeBreakpointMap_);
@@ -353,7 +366,20 @@ Debuglet.prototype.completeBreakpoint_ = function(breakpoint) {
   });
 };
 
+/**
+ * Update the server that the breakpoint cannot be handled.
+ * @param {Breakpoint} breakpoint
+ * @private
+ */
+Debuglet.prototype.rejectBreakpoint_ = function(breakpoint) {
+  var that = this;
 
+  that.debugletApi_.updateBreakpoint(breakpoint, function(err/*, body*/) {
+    if (err) {
+      that.logger_.error('Unable to complete breakpoint on server', err);
+    }
+  });
+};
 
 /**
  * This schedules a delayed operation that will delete the breakpoint from the
diff --git a/test/standalone/test-debuglet.js b/test/standalone/test-debuglet.js
@@ -32,6 +32,12 @@ nock.disableNetConnect();
 var debuglet;
 var bp = {
   id: 'test',
+  action: 'CAPTURE',
+  location: { path: 'fixtures/foo.js', line: 2 }
+};
+var logBp = {
+  id: 'testLog',
+  action: 'LOG',
   location: { path: 'fixtures/foo.js', line: 2 }
 };
 
@@ -158,13 +164,20 @@ describe(__filename, function(){
       .reply(409)
       .get(BPS_PATH)
       .reply(200, {
-        breakpoints: [bp]
-      });
+        breakpoints: [bp, logBp]
+      })
+      .put(BPS_PATH + '/' + logBp.id, function(body) {
+        var status = body.breakpoint.status;
+        return status.isError &&
+          status.description.format.indexOf('action is CAPTURE') !== -1;
+      })
+      .reply(200);
 
     debuglet.once('registered', function reg(id) {
       assert(id === DEBUGGEE_ID);
       setTimeout(function() {
         assert.deepEqual(debuglet.activeBreakpointMap_.test, bp);
+        assert(!debuglet.activeBreakpointMap_.testLog);
         scope.done();
         done();
       }, 1000);
@@ -178,6 +191,7 @@ describe(__filename, function(){
   it('should report error on breakpoint set', function(done) {
     var errorBp = {
       id: 'test',
+      action: 'CAPTURE',
       location: { path: 'fixtures/foo', line: 2 }
     };
 

Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,8 @@ function StatusMessage(refersTo, description, isError) {`
`32`	`32`	`this.isError = isError;`
`33`	`33`	`}`
`34`	`34`
	`35`	`+// These status messages come from a proto definition.`
	`36`	`+// New status messages cannot be added here.`
`35`	`37`	`/** @const */ StatusMessage.UNSPECIFIED = 'UNSPECIFIED';`
`36`	`38`	`/** @const */ StatusMessage.BREAKPOINT_SOURCE_LOCATION =`
`37`	`39`	`'BREAKPOINT_SOURCE_LOCATION';`