Merge pull request #113 from mozilla/error-results

Catch scan errors and show them alongside results

Author: Paul Theriault

client/img/throbber.png

@@ -50,7 +50,9 @@
       <ul class="nav nav-sidebar">
         <li id="scan-input-nav" ng-click="navShowInput()" class="active"><a>Input<span class="badge pull-right">{{inputFiles.length}}</span></a></li>
 	<li id="scan-output-nav">
-	  <a ng-click="navShowOutput()" >Output <span class="badge pull-right">{{results.length}}</span></a>
+	  <a ng-click="navShowOutput()">Output
+          <img ng-show="throb" src="img/throbber.png">
+          <span class="badge pull-right">{{results.length}}</span></a>
 	  <div id="scan-output-rules" class="hidden">
 	    <div class="bg-info">Rules</div>
 	    <ul>
@@ -108,7 +110,7 @@ <h4>Choose files to scan</h4>
 
     <div id="scan-results" class="hidden">
       <h4>Results <span class="badge pull-right">{{filteredResults.length}}</span></h4>
-      <div id="results-wrapper" ng-show="results<1">
+      <div id="results-wrapper" ng-show="((results.length == 0) && (errors.length == 0))">
         Scan results will be loaded here.
       </div>
       <div id="results-wrapper" ng-show="results">
@@ -123,13 +125,34 @@ <h4>Results <span class="badge pull-right">{{filteredResults.length}}</span></h4
           </thead>
           <tr ng-repeat='result in filteredResults| orderBy:predicate:reverse' title={{result.rule.rec}}>
             <td>{{result.filename}}</td>
-            <td><a ng-click="showResult(result.filename,result.line, result.col)">Line {{result.line}}</a>
+            <td><a ng-click="showResult(result.filename,result.line, result.col)">Line {{result.line}}</a></td>
             <td>{{result.rule.name }}</td>
            <!-- <td>{{result.rule.desc}}</a>-->
             </td>
           </tr>
         </table>
       </div>
+      <h4>Errors <span class="badge pull-right">{{errors.length}}</span></h4>
+      <div id="error-wrapper" ng-show="(errors.length > 0)">
+            <table id="error-table" class="table table-striped">
+                <thead>
+                <tr>
+                    <th><a href="" ng-click="predicate = 'filename'; reverse=!reverse">Filename</a></th>
+                    <th><a href="" ng-click="predicate = 'line'; reverse=!reverse">Line</a></th>
+                    <th><a href="" ng-click="predicate = 'name'; reverse=!reverse">Name</a></th>
+                    <th><a href="" ng-click="predicate = 'message'; reverse=!reverse">Message</a></th>
+                </tr>
+                </thead>
+                <tr ng-repeat='error in errors'>
+                    <td>{{ error.filename }}</td>
+                    <td><a ng-click="showResult(error.filename, error.loc.line, error.loc.col)">Line {{error.loc.line }}</a></td>
+                    <td>{{ error.name }}</td>
+                    <td>{{ error.message }}</td>
+                    </td>
+                </tr>
+            </table>
+      </div>
+
     </div>
 
     </div>

client/index.html

@@ -3,22 +3,23 @@ function ScanCtrl($scope, ScanSvc) {
   $scope.codeMirrorManual = undefined;
   $scope.inputFiles = [];
   $scope.results=[];
+  $scope.errors=[];
   $scope.filteredResults=[];
   $scope.inputFilename="";
   $scope.issueList=[];
+  $scope.throb = false;
 
+  var pending = 0;
   var selectedFile = 0;
   var codeMirror_index = 0;
-  $scope.loadPossible = false;
-  localforage.length(function(len) {
-    $scope.loadPossible = (len == 3);
-  });
 
   $scope.run = function (source, filename) {
     //empty last scan
     $scope.results=[];
+    $scope.errors=[];
     $scope.inputFiles.forEach(function (scriptFile, i) {
       if (document.getElementById('doScan_'+i).checked) {
+        pending++; $scope.throb = true;
         ScanSvc.newScan(scriptFile.name,scriptFile.asText());
       }
     });
@@ -36,9 +37,12 @@ function ScanCtrl($scope, ScanSvc) {
 
   $scope.updateIssueList = function(){
     $scope.issueList = $scope.results.reduce(function(p, c) {
-      if (p.indexOf(c.rule.name) < 0)
-        p.push(c.rule.name);
-      return p;
+      if ((c.type == 'finding') && (typeof p !== "undefined")) {
+        if (p.indexOf(c.rule.name) < 0) {
+          p.push(c.rule.name);
+        }
+        return p;
+      }
     }, []);
   }
 
@@ -175,6 +179,9 @@ function ScanCtrl($scope, ScanSvc) {
     var serializedResults = JSON.stringify($scope.results, includedAttributes);
     localforage.setItem('results', serializedResults, function() { });
 
+    var serializedErrors = JSON.stringify($scope.errors);
+    localforage.setItem('errors', serializedErrors, function() { });
+
     var serializedInputFiles = $scope.inputFiles.map( function(el) { return {data: el.asText(), name: el.name }; });
     localforage.setItem("inputFiles", JSON.stringify(serializedInputFiles), function(r) { });
 
@@ -187,15 +194,19 @@ function ScanCtrl($scope, ScanSvc) {
   };
 
   //TODO loadstate isn't called anymore, need to make it work with new workflow
-  $scope.loadState = function() {
-    // restore results as is
+  //TODO -> call loadState() around in main.js, line 36 (using the scanCtrlScope) and expose "reset" button in the UI.
+  $scope.restoreState = function() {
+    var apply = false;
     localforage.getItem('results', function (results_storage) {
-      if(!results_storage){
-        alert('No previous scan found.')
-      }
       $scope.results = JSON.parse(results_storage);
-      $scope.$apply();
+      apply = true;
       });
+    localforage.getItem('errors', function (errors_storage) {
+      if (errors_storage) {
+        $scope.errors = JSON.parse(errors_storage);
+        apply = true;
+      }
+    });
     // restore files, by creating JSZip things :)
     localforage.getItem("inputFiles", function(inputFiles_storage) {
       // mimic behavior from handleFileUpload
@@ -215,8 +226,9 @@ function ScanCtrl($scope, ScanSvc) {
           document.getElementById("doScan_" + i).checked = checkboxes[i];
         }
       });
-      $scope.$apply();
+      apply = true;
     });
+    if (apply) { $scope.$apply(); }
   };
 
   $scope.selectAll = function () {
@@ -241,9 +253,11 @@ function ScanCtrl($scope, ScanSvc) {
     });
     var content=file.asText();
     return content.split('\n').splice(line,line+numLines).join('\n');
-  }
+  };
 
   $scope.$on('NewResults', function (event, result) {
+    pending--;
+    if (pending == 0) { $scope.throb = false; }
     if (Object.keys(result).length === 0) {
       $scope.error = "Empty result set (this can also be a good thing, if you test a simple file)";
       return
@@ -259,6 +273,11 @@ function ScanCtrl($scope, ScanSvc) {
   });
 
   $scope.$on('ScanError', function (event, exception) {
-    $scope.error = exception.name + " at Line " + exception.loc.line + ", Column " + exception.loc.column + ": " + exception.message;
+    pending--;
+    if (pending == 0) { $scope.throb = false; }
+    $scope.errors.push(exception);
+    $scope.updateIssueList();
+    $scope.$apply();
+    $scope.saveState();
   });
 }

client/js/scanctrl.js

@@ -19,12 +19,21 @@ scanjsModule.factory('ScanSvc', function($rootScope) {
   ScanService.scanWorker = new Worker("js/scanworker.js");
   ScanService.scanWorker.addEventListener("message", function (evt) {
     if (('findings' in evt.data) && ('filename' in evt.data)) {
+      if (evt.data.findings.length > 0) {
+        if (evt.data.findings[0].type == 'error') {
+          $rootScope.$broadcast('ScanError', evt.data.findings[0])
+          return;
+        }
+      }
       ScanService.addResults(evt.data);
     }
     else if ('error' in evt.data) {
+      // This is for errors in the worker, not in the scanning.
+      // Exceptions (like SyntaxErrors) when scanning files
+      // are in the findings.
       var exception = evt.data.error;
       if (e instanceof SyntaxError) {
-        $rootsScope.$broadcast('ScanError', {name: exception.name, loc: exception.loc, message: exception.message })
+        $rootScope.$broadcast('ScanError', {filename: evt.data.filename, name: exception.name, loc: exception.loc, message: exception.message })
       } else {
         throw e; // keep throwing unexpected things.
       }

client/js/scanservice.js

@@ -85,9 +85,24 @@
         });
       }
       catch(e) {
-        console.log("ERROR: Skipping " + filename +" (parsing failure)");
-        console.log('Exception: '+e+ "\n");
-        throw e;
+        if (e instanceof SyntaxError) {
+          scanresults.push({
+            //TODO decide whether to leave our or include this (increases results size)
+            type: 'error',
+            name: e.name,
+            pos: e.pos,
+            loc: { column: e.loc.column, line: e.loc.line},
+            message: e.message,
+            filename: filename
+          });
+          // Assuming that we can't get more than 1 SyntaxError per file
+          // from acorn and there's nothing left to do.
+          //TODO investigate this.
+          return scanresults;
+        } else {
+          console.log("uenxpected error!!", e);
+          throw e;
+        }
       }
 
       //run all the rules against content.
@@ -108,6 +123,8 @@
           var result = testFunc.call(this, node);
           if(result) {
             scanresults.push({
+              //TODO decide whether to leave our or include this (increases results size)
+              type: 'finding',
               rule : rule,
               filename : filename,
               line : node.loc.start.line,

common/scan.js

@@ -61,16 +61,11 @@ if( typeof process != 'undefined' && process.argv[2]) {
 
       if(ext == '.js') {
         var content = fs.readFileSync(fullpath, 'utf8');
-        try {
-          results[fullpath] = ScanJS.scan(content, signatures, fullpath);
-        } catch(e) {
-          if (e instanceof SyntaxError) { // e.g., parse failure
-            //XXX this might be easy to overlook when scanning a big folder
-            console.log("SKIPPING FILE: SyntaxError in "+ fullpath+", at Line "+ e.loc.line +", Column "+e.loc.column+ ": " + e.message);
-          } else {
-            throw e;
-          }
+        var scanresult = ScanJS.scan(content, signatures, fullpath);
+        if (scanresult.type == 'error') {
+          console.log("SKIPPING FILE: Error in "+ fullpath+", at Line "+ scanresult.error.loc.line +", Column "+scanresult.error.loc.column+ ": " + scanresult.error.message);
         }
+        results[fullpath] = scanresult;
       }
     });
     // Flatten report file to remove files with no findings and tests with no results (i.e. empty arr)