Be specific about what exceptions can be skipped

The Analyzer `#run` loop was rescuing *all* exceptions, which is
excessive. I don't think we should hard-fail on all exceptions, either:
the Ruby parser gem is definitely known to barf on some valid but
esoteric Ruby code, and I wouldn't be surprised if the other parsers had
similar edge cases.

So this changes behavior to only skip files for a known set of catchable
errors. The out-of-process parsers are a little tricky since all you can
get from them is an exit code and output streams: for now wrapping those
in our own exception class seems reasonable.

I'm still catching all exceptions, but only so that we can log a message
about which file is impacted before we re-raise. Since a raw exception
we'll likely abort on may not contain helpful information about the file
that triggered it, this would be helpful for debugging cases.

Author: wfleming

lib/cc/engine/analyzers/analyzer_base.rb

@@ -1,16 +1,27 @@
+require "cc/engine/analyzers/parser_error"
+
 module CC
   module Engine
     module Analyzers
       class Base
+        RESCUABLE_ERRORS = [
+          ::CC::Engine::Analyzers::ParserError,
+          ::Racc::ParseError,
+          ::Timeout::Error,
+        ]
+
         def initialize(engine_config:)
           @engine_config = engine_config
         end
 
         def run(file)
           process_file(file)
+        rescue *RESCUABLE_ERRORS => ex
+          $stderr.puts("Skipping file #{file} due to exception:")
+          $stderr.puts("(#{ex.class}) #{ex.message} #{ex.backtrace.join("\n")}")
         rescue => ex
-          $stderr.puts "Skipping file #{file} due to exception"
-          $stderr.puts "(#{ex.class}) #{ex.message} #{ex.backtrace.join("\n")}"
+          $stderr.puts("#{ex.class} error occurred processing file #{file}: aborting.")
+          raise ex
         end
 
         def files

lib/cc/engine/analyzers/javascript/parser.rb

@@ -1,4 +1,6 @@
-require 'timeout'
+require "open3"
+require "timeout"
+
 
 module CC
   module Engine
@@ -55,14 +57,23 @@ def initialize(command, delegate)
 
           def run(input, timeout = DEFAULT_TIMEOUT)
             Timeout.timeout(timeout) do
-              IO.popen command, 'r+' do |io|
-                io.puts input
-                io.close_write
+              Open3.popen3 command, "r+" do |stdin, stdout, stderr, wait_thr|
+                stdin.puts input
+                stdin.close
+
+                exit_code = wait_thr.value
+
+                output = stdout.gets
+                stdout.close
 
-                output = io.gets
-                io.close
+                err_output = stderr.gets
+                stderr.close
 
-                yield output if $?.to_i == 0
+                if 0 == exit_code
+                  yield output
+                else
+                  raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"
+                end
               end
             end
           end

lib/cc/engine/analyzers/parser_error.rb

@@ -0,0 +1,7 @@
+module CC
+  module Engine
+    module Analyzers
+      ParserError = Class.new(StandardError)
+    end
+  end
+end

lib/cc/engine/analyzers/php/parser.rb

@@ -48,14 +48,23 @@ def initialize(command)
 
           def run(input, timeout = DEFAULT_TIMEOUT)
             Timeout.timeout(timeout) do
-              IO.popen command, 'r+' do |io|
-                io.puts input
-                io.close_write
+              Open3.popen3 command, "r+" do |stdin, stdout, stderr, wait_thr|
+                stdin.puts input
+                stdin.close
 
-                output = io.gets
-                io.close
+                exit_code = wait_thr.value
 
-                yield output if $?.to_i == 0
+                output = stdout.gets
+                stdout.close
+
+                err_output = stderr.gets
+                stderr.close
+
+                if 0 == exit_code
+                  yield output
+                else
+                  raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"
+                end
               end
             end
           end

lib/cc/engine/analyzers/python/parser.rb

@@ -1,5 +1,6 @@
-require 'timeout'
-require 'json'
+require "timeout"
+require "json"
+require "open3"
 
 module CC
   module Engine
@@ -41,14 +42,23 @@ def initialize(command, delegate)
 
           def run(input, timeout = DEFAULT_TIMEOUT)
             Timeout.timeout(timeout) do
-              IO.popen command, "r+" do |io|
-                io.puts input
-                io.close_write
+              Open3.popen3 command, "r+" do |stdin, stdout, stderr, wait_thr|
+                stdin.puts input
+                stdin.close
 
-                output = io.gets
-                io.close
+                exit_code = wait_thr.value
 
-                yield output if $?.to_i == 0
+                output = stdout.gets
+                stdout.close
+
+                err_output = stderr.gets
+                stderr.close
+
+                if 0 == exit_code
+                  yield output
+                else
+                  raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"
+                end
               end
             end
           end

spec/cc/engine/analyzers/javascript/main_spec.rb

@@ -36,6 +36,16 @@
       expect(json["content"]["body"]).to match /This issue has a mass of `99`/
       expect(json["fingerprint"]).to eq("55ae5d0990647ef496e9e0d315f9727d")
     end
+
+    it "skips unparsable files" do
+      create_source_file("foo.js", <<-EOJS)
+        function () { do(); // missing closing brace
+      EOJS
+
+      expect {
+        expect(run_engine(engine_conf)).to eq("")
+      }.to output(/Skipping file/).to_stderr
+    end
   end
 
   it "does not flag duplicate comments" do

spec/cc/engine/analyzers/php/main_spec.rb

@@ -48,6 +48,16 @@
       expect(json["content"]["body"]).to match /This issue has a mass of `44`/
       expect(json["fingerprint"]).to eq("667da0e2bab866aa2fe9d014a65d57d9")
     end
+
+    it "skips unparsable files" do
+      create_source_file("foo.php", <<-EOPHP)
+        <?php blorb &; "fee
+      EOPHP
+
+      expect {
+        expect(run_engine(engine_conf)).to eq("")
+      }.to output(/Skipping file/).to_stderr
+    end
   end
 
   def printed_issue

spec/cc/engine/analyzers/python/main_spec.rb

@@ -36,6 +36,16 @@
       expect(json["content"]["body"]).to match /This issue has a mass of `54`/
       expect(json["fingerprint"]).to eq("42b832387c997f54a2012efb2159aefc")
     end
+
+    it "skips unparsable files" do
+      create_source_file("foo.py", <<-EOPY)
+        ---
+      EOPY
+
+      expect {
+        expect(run_engine(engine_conf)).to eq("")
+      }.to output(/Skipping file/).to_stderr
+    end
   end
 
   def engine_conf