Skip to content

Commit 891ce71

Browse files
daipomashie
andcommitted
parser_json: fix wrong LoadError warning
If Oj is not installed, LoadError with the empty message is raised. So, the current condition `/\boj\z/.match?(ex.message)` does not work and the following meaningless warning is displayed. {datetime} [warn]: #x {id} LoadError After this fix, the log message will be: {datetime} [info]: #x {id} Oj is not installed, and failing back to Yajl for json parser Refactor "rescue" logic because this falling back feature is currently only for "oj" (LoadError can not occur for "json" and "yajl"). Signed-off-by: Daijiro Fukuda <[email protected]> Co-authored-by: Takuro Ashie <[email protected]>
1 parent 94185f4 commit 891ce71

File tree

2 files changed

+35
-12
lines changed

2 files changed

+35
-12
lines changed

lib/fluent/plugin/parser_json.rb

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,15 @@ def configure(conf)
5050
def configure_json_parser(name)
5151
case name
5252
when :oj
53-
raise LoadError unless Fluent::OjOptions.available?
54-
[Oj.method(:load), Oj::ParseError]
53+
return [Oj.method(:load), Oj::ParseError] if Fluent::OjOptions.available?
54+
55+
log&.info "Oj is not installed, and failing back to Yajl for json parser"
56+
configure_json_parser(:yajl)
5557
when :json then [JSON.method(:load), JSON::ParserError]
5658
when :yajl then [Yajl.method(:load), Yajl::ParseError]
5759
else
5860
raise "BUG: unknown json parser specified: #{name}"
5961
end
60-
rescue LoadError => ex
61-
name = :yajl
62-
if log
63-
if /\boj\z/.match?(ex.message)
64-
log.info "Oj is not installed, and failing back to Yajl for json parser"
65-
else
66-
log.warn ex.message
67-
end
68-
end
69-
retry
7062
end
7163

7264
def parse(text)

test/plugin/test_parser_json.rb

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,37 @@ def setup
88
@parser = Fluent::Test::Driver::Parser.new(Fluent::Plugin::JSONParser)
99
end
1010

11+
sub_test_case "configure_json_parser" do
12+
data("oj", [:oj, [Oj.method(:load), Oj::ParseError]])
13+
data("json", [:json, [JSON.method(:load), JSON::ParserError]])
14+
data("yajl", [:yajl, [Yajl.method(:load), Yajl::ParseError]])
15+
def test_return_each_loader((input, expected_return))
16+
result = @parser.instance.configure_json_parser(input)
17+
assert_equal expected_return, result
18+
end
19+
20+
def test_raise_exception_for_unknown_input
21+
assert_raise RuntimeError do
22+
@parser.instance.configure_json_parser(:unknown)
23+
end
24+
end
25+
26+
def test_fall_back_oj_to_yajl_if_oj_not_available
27+
stub(Fluent::OjOptions).available? { false }
28+
29+
result = @parser.instance.configure_json_parser(:oj)
30+
31+
assert_equal [Yajl.method(:load), Yajl::ParseError], result
32+
logs = @parser.logs.collect do |log|
33+
log.gsub(/\A\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} [-+]\d{4} /, "")
34+
end
35+
assert_equal(
36+
["[info]: Oj is not installed, and failing back to Yajl for json parser\n"],
37+
logs
38+
)
39+
end
40+
end
41+
1142
data('oj' => 'oj', 'yajl' => 'yajl')
1243
def test_parse(data)
1344
@parser.configure('json_parser' => data)

0 commit comments

Comments
 (0)