Merge remote-tracking branch 'origin/master' into update-log-error

umbrellio · Apr 18, 2023 · bcb5f8d · bcb5f8d
2 parents c1cf1d5 + ae2de00
commit bcb5f8d
Show file tree

Hide file tree

Showing 12 changed files with 118 additions and 70 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -20,7 +20,7 @@ jobs:
           - rails: 5.2
             ruby: 2.6
           - rails: 5.2
-            ruby: jruby
+            ruby: jruby-9.3
 
           - rails: "6.0"
             ruby: 2.6
@@ -35,19 +35,20 @@ jobs:
             ruby: jruby
 
           - rails: "7.0"
-            ruby: 3.1
-          - rails: "6.1"
-            ruby: jruby
-
+            ruby: "3.0"
           - rails: "7.0"
             ruby: 3.1
+          - rails: "7.0"
+            ruby: 3.2
+          - rails: "7.0"
+            ruby: jruby 
 
     env:
       BUNDLE_GEMFILE: gemfiles/rails_${{ matrix.rails }}.gemfile
       DISPLAY: ":99.0"
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: Set up Ruby
         uses: ruby/setup-ruby@v1
         with:

diff --git a/Appraisals b/Appraisals
@@ -28,6 +28,6 @@ appraise "rails_7.0" do
   gem "thread_safe", "~> 0.3.6"
 
   gem "rails", "~> 7.0.0"
-  gem "activerecord-jdbcsqlite3-adapter", "~> 61.0", platform: :jruby
+  gem "activerecord-jdbcsqlite3-adapter", "~> 70.0", platform: :jruby
   gem "sqlite3", "~> 1.4.0", platform: :ruby
 end
diff --git a/Gemfile b/Gemfile
@@ -11,11 +11,8 @@ gem "minitest"
 gem "minitest-rails"
 gem "rake"
 gem "sprockets", "< 4.0"
-gem "semantic_logger", github: "reidmorrison/semantic_logger"
 
-gem "rails", "~> 6.1.0"
+gem "rails", "~> 7.0.0"
 gem "sqlite3", "~> 1.4.0", platform: :ruby
 
-group :development do
-  gem "rubocop"
-end
+gem "rubocop"
diff --git a/gemfiles/rails_7.0.gemfile b/gemfiles/rails_7.0.gemfile
@@ -14,7 +14,7 @@ gem "semantic_logger", github: "reidmorrison/semantic_logger"
 gem "rails", "~> 7.0.0"
 gem "sqlite3", "~> 1.4.0", platform: :ruby
 gem "thread_safe", "~> 0.3.6"
-gem "activerecord-jdbcsqlite3-adapter", "~> 61.0", platform: :jruby
+gem "activerecord-jdbcsqlite3-adapter", "~> 70.0", platform: :jruby
 
 group :development do
   gem "rubocop"

diff --git a/lib/rails_semantic_logger/action_controller/log_subscriber.rb b/lib/rails_semantic_logger/action_controller/log_subscriber.rb
@@ -14,12 +14,22 @@ def process_action(event)
 
           # Unused, but needed for Devise 401 status code monkey patch to still work.
           ::ActionController::Base.log_process_action(payload)
+
+          params = payload[:params]
 
-          # According to PR https://github.com/reidmorrison/rails_semantic_logger/pull/37/files
-          # payload[:params] is not always a Hash.
-          payload[:params] = payload[:params].to_unsafe_h unless payload[:params].is_a?(Hash)
-          payload[:params] = payload[:params].except(*INTERNAL_PARAMS)
-          payload.delete(:params) if payload[:params].empty?
+          if params.kind_of?(Hash) || params.kind_of?(::ActionController::Parameters)
+            # According to PR https://github.com/reidmorrison/rails_semantic_logger/pull/37/files
+            # params is not always a Hash.
+            payload[:params] = params.to_unsafe_h unless params.is_a?(Hash)
+            payload[:params] = params.except(*INTERNAL_PARAMS)
+
+            if payload[:params].empty?
+              payload.delete(:params)
+            elsif params["file"]
+              # When logging to JSON the entire tempfile is logged, so convert it to a string.
+              payload[:params]["file"] = params["file"].inspect
+            end
+          end
 
           format           = payload[:format]
           payload[:format] = format.to_s.upcase if format.is_a?(Symbol)
@@ -48,12 +58,6 @@ def process_action(event)
           payload.delete(:request)
           payload.delete(:response)
 
-          params = payload[:params]
-          if params
-            # When logging to JSON the entire tempfile is logged, so convert it to a string.
-            params["file"] = params["file"].inspect if params["file"]
-          end
-
           {
             message:  "Completed ##{payload[:action]}",
             duration: event.duration,

diff --git a/lib/rails_semantic_logger/action_mailer/log_subscriber.rb b/lib/rails_semantic_logger/action_mailer/log_subscriber.rb
@@ -9,15 +9,15 @@ def deliver(event)
         message_id = event.payload[:message_id]
         duration = event.duration.round(1)
         if ex
-         log_with_formatter event: event, log_duration: true, level: :error do |fmt|
+          log_with_formatter event: event, log_duration: true, level: :error do |fmt|
             {
               message: "Error delivering mail #{message_id} (#{duration}ms)",
               exception: ex
             }
           end
         else
           message = begin
-          if event.payload[:perform_deliveries]
+            if event.payload[:perform_deliveries]
               "Delivered mail #{message_id} (#{duration}ms)"
             else
               "Skipped delivery of mail #{message_id} as `perform_deliveries` is false"

diff --git a/lib/rails_semantic_logger/engine.rb b/lib/rails_semantic_logger/engine.rb
@@ -1,7 +1,4 @@
 require "rails"
-require "action_controller/log_subscriber"
-require "action_view/log_subscriber"
-require "action_mailer/log_subscriber"
 require "rails_semantic_logger/options"
 
 module RailsSemanticLogger
@@ -111,7 +108,14 @@ class Engine < ::Rails::Engine
       Resque.logger        = SemanticLogger[Resque] if defined?(Resque) && Resque.respond_to?(:logger=)
 
       # Replace the Sidekiq logger
-      Sidekiq.logger       = SemanticLogger[Sidekiq] if defined?(Sidekiq) && Sidekiq.respond_to?(:logger=)
+      if defined?(Sidekiq)
+        if Sidekiq.respond_to?(:logger=)
+          Sidekiq.logger = SemanticLogger[Sidekiq]
+        elsif Sidekiq::VERSION[0..1] == '7.'
+          method = Sidekiq.server? ? :configure_server : :configure_client
+          Sidekiq.public_send(method) { |cfg| cfg.logger = SemanticLogger[Sidekiq] }
+        end
+      end
 
       # Replace the Sidetiq logger
       Sidetiq.logger       = SemanticLogger[Sidetiq] if defined?(Sidetiq) && Sidetiq.respond_to?(:logger=)
@@ -186,26 +190,38 @@ class Engine < ::Rails::Engine
         end
 
         # Action View
-        RailsSemanticLogger::ActionView::LogSubscriber.rendered_log_level = :info if config.rails_semantic_logger.rendered
-        RailsSemanticLogger.swap_subscriber(
-          ::ActionView::LogSubscriber,
-          RailsSemanticLogger::ActionView::LogSubscriber,
-          :action_view
-        )
+        if defined?(::ActionView)
+          require "action_view/log_subscriber"
+
+          RailsSemanticLogger::ActionView::LogSubscriber.rendered_log_level = :info if config.rails_semantic_logger.rendered
+          RailsSemanticLogger.swap_subscriber(
+            ::ActionView::LogSubscriber,
+            RailsSemanticLogger::ActionView::LogSubscriber,
+            :action_view
+          )
+        end
 
         # Action Controller
-        RailsSemanticLogger.swap_subscriber(
-          ::ActionController::LogSubscriber,
-          RailsSemanticLogger::ActionController::LogSubscriber,
-          :action_controller
-        )
+        if defined?(::ActionController)
+          require "action_controller/log_subscriber"
+
+          RailsSemanticLogger.swap_subscriber(
+            ::ActionController::LogSubscriber,
+            RailsSemanticLogger::ActionController::LogSubscriber,
+            :action_controller
+          )
+        end
 
         # Action Mailer
-        RailsSemanticLogger.swap_subscriber(
-          ::ActionMailer::LogSubscriber,
-          RailsSemanticLogger::ActionMailer::LogSubscriber,
-          :action_mailer
-        )
+        if defined?(::ActionMailer)
+          require "action_mailer/log_subscriber"
+
+          RailsSemanticLogger.swap_subscriber(
+            ::ActionMailer::LogSubscriber,
+            RailsSemanticLogger::ActionMailer::LogSubscriber,
+            :action_mailer
+          )
+        end
       end
 
       #

diff --git a/lib/rails_semantic_logger/rack/logger.rb b/lib/rails_semantic_logger/rack/logger.rb
@@ -35,16 +35,18 @@ def call(env)
       @started_request_log_level = :debug
 
       def call_app(request, env)
-        instrumenter = ActiveSupport::Notifications.instrumenter
-        instrumenter.start "request.action_dispatch", request: request
+        instrumenter        = ActiveSupport::Notifications.instrumenter
+        instrumenter_state  = instrumenter.start "request.action_dispatch", request: request
+        instrumenter_finish = -> () {
+          instrumenter.finish_with_state(instrumenter_state, "request.action_dispatch", request: request)
+        }
 
         logger.send(self.class.started_request_log_level) { started_request_message(request) }
-
         status, headers, body = @app.call(env)
-        body                  = ::Rack::BodyProxy.new(body) { finish(request) }
+        body                  = ::Rack::BodyProxy.new(body, &instrumenter_finish)
         [status, headers, body]
       rescue Exception
-        finish(request)
+        instrumenter_finish.call
         raise
       end
 
@@ -90,11 +92,6 @@ def compute_named_tags(request)
         tagged
       end
 
-      def finish(request)
-        instrumenter = ActiveSupport::Notifications.instrumenter
-        instrumenter.finish "request.action_dispatch", request: request
-      end
-
       def logger
         self.class.logger
       end

diff --git a/lib/rails_semantic_logger/version.rb b/lib/rails_semantic_logger/version.rb
@@ -1,3 +1,3 @@
 module RailsSemanticLogger
-  VERSION = "4.11.0".freeze
+  VERSION = "4.12.0".freeze
 end
diff --git a/rails_semantic_logger.gemspec b/rails_semantic_logger.gemspec
@@ -4,17 +4,23 @@ $LOAD_PATH.push File.expand_path("lib", __dir__)
 require "rails_semantic_logger/version"
 
 # Describe your gem and declare its dependencies:
-Gem::Specification.new do |spec|
-  spec.name                  = "rails_semantic_logger"
-  spec.version               = RailsSemanticLogger::VERSION
-  spec.platform              = Gem::Platform::RUBY
-  spec.authors               = ["Reid Morrison"]
-  spec.homepage              = "https://logger.rocketjob.io"
-  spec.summary               = "Feature rich logging framework that replaces the Rails logger."
-  spec.files                 = Dir["lib/**/*", "LICENSE.txt", "Rakefile", "README.md"]
-  spec.license               = "Apache-2.0"
-  spec.required_ruby_version = ">= 2.5"
-  spec.add_dependency "rack"
-  spec.add_dependency "railties", ">= 5.1"
-  spec.add_dependency "semantic_logger", "~> 4.9"
+Gem::Specification.new do |s|
+  s.name                  = "rails_semantic_logger"
+  s.version               = RailsSemanticLogger::VERSION
+  s.platform              = Gem::Platform::RUBY
+  s.authors               = ["Reid Morrison"]
+  s.homepage              = "https://logger.rocketjob.io"
+  s.summary               = "Feature rich logging framework that replaces the Rails logger."
+  s.files                 = Dir["lib/**/*", "LICENSE.txt", "Rakefile", "README.md"]
+  s.license               = "Apache-2.0"
+  s.required_ruby_version = ">= 2.5"
+  s.add_dependency "rack"
+  s.add_dependency "railties", ">= 5.1"
+  s.add_dependency "semantic_logger", "~> 4.13"
+  s.metadata = {
+    "bug_tracker_uri"       => "https://github.com/reidmorrison/rails_semantic_logger/issues",
+    "documentation_uri"     => "https://logger.rocketjob.io",
+    "source_code_uri"       => "https://github.com/reidmorrison/rails_semantic_logger/tree/#{RailsSemanticLogger::VERSION}",
+    "rubygems_mfa_required" => "true"
+  }
 end
diff --git a/test/action_controller_test.rb b/test/action_controller_test.rb
@@ -0,0 +1,27 @@
+require_relative "test_helper"
+
+class ActionControllerTest < Minitest::Test
+  describe "RailsSemanticLogger::ActionController::LogSubscriber" do
+    let(:subscriber) { RailsSemanticLogger::ActionController::LogSubscriber.new }
+
+    describe "#process_action" do
+      it "does not fail if params is not a Hash nor an instance of ActionController::Parameters" do
+        event = ActiveSupport::Notifications::Event.new(
+          "start_processing.action_controller",
+          5.seconds.ago,
+          Time.zone.now,
+          SecureRandom.uuid,
+          {
+            payload: "{}"
+          }
+        )
+
+        messages = semantic_logger_events do
+          subscriber.process_action(event)
+        end
+
+        assert_equal 1, messages.count, messages
+      end
+    end
+  end
+end
diff --git a/test/dummy/db/test.sqlite3 b/test/dummy/db/test.sqlite3