From 8688cbaea1f7bd8b7868a7809d5fabd6b1464afd Mon Sep 17 00:00:00 2001
From: Daijiro Fukuda <fukuda@clear-code.com>
Date: Wed, 12 Jun 2024 11:56:43 +0900
Subject: [PATCH] parser_json: fix wrong LoadError warning

Backported from 891ce71120198fa168232a796d4267a3cdb2c91f.

---

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 <fukuda@clear-code.com>
Co-authored-by: Takuro Ashie <ashie@clear-code.com>
---
 lib/fluent/plugin/parser_json.rb | 16 ++++------------
 test/plugin/test_parser_json.rb  | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/lib/fluent/plugin/parser_json.rb b/lib/fluent/plugin/parser_json.rb
index 829aa4b72a..79fea5abe9 100644
--- a/lib/fluent/plugin/parser_json.rb
+++ b/lib/fluent/plugin/parser_json.rb
@@ -50,23 +50,15 @@ def configure(conf)
       def configure_json_parser(name)
         case name
         when :oj
-          raise LoadError unless Fluent::OjOptions.available?
-          [Oj.method(:load), Oj::ParseError]
+          return [Oj.method(:load), Oj::ParseError] if Fluent::OjOptions.available?
+
+          log&.info "Oj is not installed, and failing back to Yajl for json parser"
+          configure_json_parser(:yajl)
         when :json then [JSON.method(:load), JSON::ParserError]
         when :yajl then [Yajl.method(:load), Yajl::ParseError]
         else
           raise "BUG: unknown json parser specified: #{name}"
         end
-      rescue LoadError => ex
-        name = :yajl
-        if log
-          if /\boj\z/.match?(ex.message)
-            log.info "Oj is not installed, and failing back to Yajl for json parser"
-          else
-            log.warn ex.message
-          end
-        end
-        retry
       end
 
       def parse(text)
diff --git a/test/plugin/test_parser_json.rb b/test/plugin/test_parser_json.rb
index 19c45402d1..e60784628e 100644
--- a/test/plugin/test_parser_json.rb
+++ b/test/plugin/test_parser_json.rb
@@ -8,6 +8,37 @@ def setup
     @parser = Fluent::Test::Driver::Parser.new(Fluent::Plugin::JSONParser)
   end
 
+  sub_test_case "configure_json_parser" do
+    data("oj", [:oj, [Oj.method(:load), Oj::ParseError]])
+    data("json", [:json, [JSON.method(:load), JSON::ParserError]])
+    data("yajl", [:yajl, [Yajl.method(:load), Yajl::ParseError]])
+    def test_return_each_loader((input, expected_return))
+      result = @parser.instance.configure_json_parser(input)
+      assert_equal expected_return, result
+    end
+
+    def test_raise_exception_for_unknown_input
+      assert_raise RuntimeError do
+        @parser.instance.configure_json_parser(:unknown)
+      end
+    end
+
+    def test_fall_back_oj_to_yajl_if_oj_not_available
+      stub(Fluent::OjOptions).available? { false }
+
+      result = @parser.instance.configure_json_parser(:oj)
+
+      assert_equal [Yajl.method(:load), Yajl::ParseError], result
+      logs = @parser.logs.collect do |log|
+        log.gsub(/\A\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} [-+]\d{4} /, "")
+      end
+      assert_equal(
+        ["[info]: Oj is not installed, and failing back to Yajl for json parser\n"],
+        logs
+      )
+    end
+  end
+
   data('oj' => 'oj', 'yajl' => 'yajl')
   def test_parse(data)
     @parser.configure('json_parser' => data)