Clean up coffeescript error messages

Matt Loring · Matt Loring · commit 4de169b128a3 · 2016-01-11T15:00:41.000-08:00
If compilation of a coffeescript expression fails, we will report the associated error if the coffeescript compiler provides one. Otherwise, we will just report that the compilation failed. Fixes #2
diff --git a/lib/v8debugapi.js b/lib/v8debugapi.js
@@ -36,6 +36,7 @@
   V8_BREAKPOINT_ERROR: 'Unable to set breakpoint in v8',
   SYNTAX_ERROR_IN_CONDITION: 'Syntax error in condition: ',
   ERROR_EVALUATING_CONDITION: 'Error evaluating condition: ',
+  ERROR_COMPILING_CONDITION: 'Error compiling condition.',
   DISALLOWED_EXPRESSION: 'Expression not allowed',
   SOURCE_MAP_URL_NOT_FOUND: 'The source map url could not be found in the compiled file',
   SOURCE_MAP_READ_ERROR: 'The source map could not be read or was incorrectly formatted',
@@ -132,7 +133,15 @@ module.exports.create = function(logger_, config_, fileStats_) {
         };
         compile = getBreakpointCompiler(breakpoint);
         if (breakpoint.condition && compile) {
-          breakpoint.condition = compile(breakpoint.condition);
+          try {
+            breakpoint.condition = compile(breakpoint.condition);
+          } catch (e) {
+            logger.info('Unable to compile condition >> ' +
+              breakpoint.condition + ' <<');
+            return setErrorStatusAndCallback(cb, breakpoint,
+                StatusMessage.BREAKPOINT_CONDITION,
+                messages.ERROR_COMPILING_CONDITION);
+          }
         }
         // TODO: more robust file finding of compiled files
         scriptPath = scriptPath.substr(0, scriptPath.lastIndexOf('.')) + '.js';
@@ -323,16 +332,15 @@ module.exports.create = function(logger_, config_, fileStats_) {
     switch(path.normalize(breakpoint.location.path).split('.').pop()) {
       case 'coffee':
         return function(uncompiled) {
-          try {
-            var comp = require('coffee-script');
-            var compiled = comp.compile('0 || (' + uncompiled + ')');
-            // Strip out coffeescript scoping wrapper to get translated condition
-            var re = /\(function\(\) {\s*0 \|\| \((.*)\);\n\n\}\)\.call\(this\);/;
-            var match = re.exec(compiled)[1];
-            return match ? match.trim() : match;
-          } catch (err) {
-            logger.info('Unable to compile break or watch point >> ' +
-              uncompiled + ' <<', err);
+          var comp = require('coffee-script');
+          var compiled = comp.compile('0 || (' + uncompiled + ')');
+          // Strip out coffeescript scoping wrapper to get translated condition
+          var re = /\(function\(\) {\s*0 \|\| \((.*)\);\n\n\}\)\.call\(this\);/;
+          var match = re.exec(compiled);
+          if (match && match.length > 1) {
+            return match[1].trim();
+          } else {
+            throw new Error('Compilation Error for: ' + uncompiled);
           }
         };
       case 'es6':
@@ -467,16 +475,29 @@ module.exports.create = function(logger_, config_, fileStats_) {
   }
 
   function captureBreakpointData(breakpoint, execState) {
+    var expressionErrors = [];
     if (breakpoint.expressions && breakpoints[breakpoint.id].compile) {
       for (var i = 0; i < breakpoint.expressions.length; i++) {
-        breakpoint.expressions[i] =
-          breakpoints[breakpoint.id].compile(breakpoint.expressions[i]);
+        try {
+          breakpoint.expressions[i] =
+            breakpoints[breakpoint.id].compile(breakpoint.expressions[i]);
+        } catch (e) {
+          logger.info('Unable to compile watch expression >> ' +
+              breakpoint.expressions[i] + ' <<');
+          expressionErrors.push({
+            name: breakpoint.expressions[i],
+            status: new StatusMessage(StatusMessage.VARIABLE_VALUE,
+                                      'Error Compiling Expression', true)
+          });
+          breakpoint.expressions.splice(i, 1);
+        }
       }
     }
     var captured = state.capture(execState, breakpoint.expressions, config);
     breakpoint.stackFrames = captured.stackFrames;
     breakpoint.variableTable = captured.variableTable;
-    breakpoint.evaluatedExpressions = captured.evaluatedExpressions;
+    breakpoint.evaluatedExpressions =
+      expressionErrors.concat(captured.evaluatedExpressions);
   }
 
   /**
diff --git a/test/test-v8debugapi.js b/test/test-v8debugapi.js
@@ -414,6 +414,21 @@ describe('v8debugapi', function() {
         });
     });
 
+    it('should show error for invalid conditions in coffeescript',
+      function (done) {
+        var bp = {
+          id: 'coffee-id-1729',
+          location: { path: './test/fixtures/coffee/transpile.coffee',
+            line: 3 },
+          condition: 'process=false'
+        };
+        api.set(bp, function(err) {
+          assert(err);
+          assert.equal(err.message, 'Error compiling condition.');
+          done();
+        });
+    });
+
     it('should be possible to set conditional breakpoints with babel',
       function (done) {
         var bp = {
@@ -489,6 +504,16 @@ describe('v8debugapi', function() {
             for (var i in bp.evaluatedExpressions) {
               var expr = bp.evaluatedExpressions[i];
               assert(expr.status && expr.status.isError);
+              if (expr.name === ':)' ||
+                  expr.name === 'process=this' ||
+                  expr.name === 'return') {
+                assert.equal(expr.status.description.format,
+                  'Error Compiling Expression');
+              } else {
+                assert.notEqual(
+                  expr.status.description.format.indexOf('Unexpected token'),
+                  -1);
+              }
             }
 
             api.clear(bp);
