Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

Commit 86723c6

Browse files
committed
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.
1 parent 90b5a4e commit 86723c6

File tree

8 files changed

+101
-23
lines changed

8 files changed

+101
-23
lines changed

‎lib/cc/engine/analyzers/analyzer_base.rb‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,27 @@
1+
require "cc/engine/analyzers/parser_error"
2+
13
module CC
24
module Engine
35
module Analyzers
46
class Base
7+
RESCUABLE_ERRORS = [
8+
::CC::Engine::Analyzers::ParserError,
9+
::Racc::ParseError,
10+
::Timeout::Error,
11+
]
12+
513
def initialize(engine_config:)
614
@engine_config = engine_config
715
end
816

917
def run(file)
1018
process_file(file)
19+
rescue *RESCUABLE_ERRORS => ex
20+
$stderr.puts("Skipping file #{file} due to exception:")
21+
$stderr.puts("(#{ex.class}) #{ex.message} #{ex.backtrace.join("\n")}")
1122
rescue => ex
12-
$stderr.puts"Skipping file #{file} due to exception"
13-
$stderr.puts"(#{ex.class}) #{ex.message}#{ex.backtrace.join("\n")}"
23+
$stderr.puts("#{ex.class} error occurred processing file #{file}: aborting.")
24+
raiseex
1425
end
1526

1627
def files

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
require 'timeout'
1+
require "open3"
2+
require "timeout"
3+
24

35
module CC
46
module Engine
@@ -55,14 +57,23 @@ def initialize(command, delegate)
5557

5658
def run(input, timeout = DEFAULT_TIMEOUT)
5759
Timeout.timeout(timeout) do
58-
IO.popen command, 'r+' do |io|
59-
io.puts input
60-
io.close_write
60+
Open3.popen3 command, "r+" do |stdin, stdout, stderr, wait_thr|
61+
stdin.puts input
62+
stdin.close
63+
64+
exit_code = wait_thr.value
65+
66+
output = stdout.gets
67+
stdout.close
6168

62-
output = io.gets
63-
io.close
69+
err_output = stderr.gets
70+
stderr.close
6471

65-
yield output if $?.to_i == 0
72+
if 0 == exit_code
73+
yield output
74+
else
75+
raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"
76+
end
6677
end
6778
end
6879
end
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module CC
2+
module Engine
3+
module Analyzers
4+
ParserError = Class.new(StandardError)
5+
end
6+
end
7+
end

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,23 @@ def initialize(command)
4848

4949
def run(input, timeout = DEFAULT_TIMEOUT)
5050
Timeout.timeout(timeout) do
51-
IO.popen command, 'r+' do |io|
52-
io.puts input
53-
io.close_write
51+
Open3.popen3 command, "r+" do |stdin,stdout,stderr,wait_thr|
52+
stdin.puts input
53+
stdin.close
5454

55-
output = io.gets
56-
io.close
55+
exit_code = wait_thr.value
5756

58-
yield output if $?.to_i == 0
57+
output = stdout.gets
58+
stdout.close
59+
60+
err_output = stderr.gets
61+
stderr.close
62+
63+
if 0 == exit_code
64+
yield output
65+
else
66+
raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"
67+
end
5968
end
6069
end
6170
end

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

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
require 'timeout'
2-
require 'json'
1+
require "timeout"
2+
require "json"
3+
require "open3"
34

45
module CC
56
module Engine
@@ -41,14 +42,23 @@ def initialize(command, delegate)
4142

4243
def run(input, timeout = DEFAULT_TIMEOUT)
4344
Timeout.timeout(timeout) do
44-
IO.popen command, "r+" do |io|
45-
io.puts input
46-
io.close_write
45+
Open3.popen3 command, "r+" do |stdin,stdout,stderr,wait_thr|
46+
stdin.puts input
47+
stdin.close
4748

48-
output = io.gets
49-
io.close
49+
exit_code = wait_thr.value
5050

51-
yield output if $?.to_i == 0
51+
output = stdout.gets
52+
stdout.close
53+
54+
err_output = stderr.gets
55+
stderr.close
56+
57+
if 0 == exit_code
58+
yield output
59+
else
60+
raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"
61+
end
5262
end
5363
end
5464
end

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@
3636
expect(json["content"]["body"]).to match /This issue has a mass of `99`/
3737
expect(json["fingerprint"]).to eq("55ae5d0990647ef496e9e0d315f9727d")
3838
end
39+
40+
it "skips unparsable files" do
41+
create_source_file("foo.js", <<-EOJS)
42+
function () { do(); // missing closing brace
43+
EOJS
44+
45+
expect {
46+
expect(run_engine(engine_conf)).to eq("")
47+
}.to output(/Skipping file/).to_stderr
48+
end
3949
end
4050

4151
it "does not flag duplicate comments" do

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,16 @@
4848
expect(json["content"]["body"]).to match /This issue has a mass of `44`/
4949
expect(json["fingerprint"]).to eq("667da0e2bab866aa2fe9d014a65d57d9")
5050
end
51+
52+
it "skips unparsable files" do
53+
create_source_file("foo.php", <<-EOPHP)
54+
<?php blorb &; "fee
55+
EOPHP
56+
57+
expect {
58+
expect(run_engine(engine_conf)).to eq("")
59+
}.to output(/Skipping file/).to_stderr
60+
end
5161
end
5262

5363
def printed_issue

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@
3636
expect(json["content"]["body"]).to match /This issue has a mass of `54`/
3737
expect(json["fingerprint"]).to eq("42b832387c997f54a2012efb2159aefc")
3838
end
39+
40+
it "skips unparsable files" do
41+
create_source_file("foo.py", <<-EOPY)
42+
---
43+
EOPY
44+
45+
expect {
46+
expect(run_engine(engine_conf)).to eq("")
47+
}.to output(/Skipping file/).to_stderr
48+
end
3949
end
4050

4151
def engine_conf

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /