Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange logging bug with new noticed 1.6.3 version #289

Closed
edk opened this issue May 24, 2023 · 1 comment
Closed

Strange logging bug with new noticed 1.6.3 version #289

edk opened this issue May 24, 2023 · 1 comment

Comments

@edk
Copy link

edk commented May 24, 2023

I updated JSP recently, but am running into an issue that I can't figure out. I narrowed it down to the new :logger change in Noticed 1.6.3, and when I downgrade to 1.6.2, my test failures go away. Note that I am also using rails_semantic_logger and silencer (I wish I didn't need silencer, but the filtering in semantic_logger isn't working for me). More on this later.

I started getting many failures, related to whenever a notification was generated:
Minitest::UnexpectedError: NoMethodError: undefined method tagged' for nil:NilClass`
Here's a gist of the test failures.

I eventually settled on a monkey-patch that "fixes" it:

# initializer
module Extensions
  module ActiveJob
    module Logging
      def tag_logger(*tags, &block)
        if logger.nil?
          @logger = ::ActiveJob::Base.logger
        end
        logger.tagged(*tags, &block)
      end
    end
  end
end

ActiveJob::Logging.prepend(Extensions::ActiveJob::Logging)

If I remove either Silencer (2.0.0) or rails_semantic_logger (4.12.0) and I don't have the monkeypatch enabled, it no longer fails. So it seems some sort of crazy interaction between the three libraries.

From what I have found, it seems by the time the tag_logger method is called, SemanticLogger has replaced the activejob version of tag_logger with its own.

I'm not sure if I'm missing something else here, have you seen this before? While my tests are passing again with a monkeypatch, I'm not sure if other problems might pop up in dev or production. And ideally, I'd like avoid a monkeypatch if at all possible.

I also tried adding logger: Rails.logger to the deliver_by in ApplicationNotification with no luck.

class ApplicationNotification < Noticed::Base
  # Delivery methods and helpers used by all notifications can be defined here.
  deliver_by :database, format: :to_database, logger: Rails.logger

I was wondering if you could explain the context of how the :logger option is normally set?

attr_reader :notification, :options, :params, :recipient, :record, :logger

What I see when looking at the source in pry:

From: /Users/edk/.rvm/gems/ruby-3.2.2/gems/activejob-7.0.4.3/lib/active_job/logging.rb:18 ActiveJob::Logging#perform_now:

    17: def perform_now
 => 18:   binding.pry
    19:   tag_logger(self.class.name, self.job_id) { super }
    20: end

[1] pry(#<Noticed::DeliveryMethods::Database>)> $ tag_logger

From: /Users/edk/.rvm/gems/ruby-3.2.2/gems/rails_semantic_logger-4.12.0/lib/rails_semantic_logger/extensions/active_job/logging.rb:11:
Owner: ActiveJob::Logging
Visibility: private
Signature: tag_logger(*tags, &block)
Number of lines: 3

def tag_logger(*tags, &block)
  logger.tagged(*tags, &block) # line 83, logger is nil!
end
[2] pry(#<Noticed::DeliveryMethods::Database>)>

For some reason logger on line 83 is nil, and I can't figure out how to set it without a monkeypatch.

Thanks!

@bvogel
Copy link

bvogel commented May 26, 2023

I can confirm I'm running in the same issue. We are also running rails_semantic_logger (4.12.0), but without silencer. The same tag_logger method is affected with NoMethodError: undefined method `tagged' for nil:NilClass

Repository owner locked and limited conversation to collaborators Oct 31, 2023
@Kentasmic Kentasmic converted this issue into discussion #326 Oct 31, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants