SemanticLogger is not working

[rade-tomovic]

I want to be able to use any logger. Currently, SemanticLogger is not working for no obvious reason.

     TypeError:
       Parameter 'logger': Expected type T.nilable(T.any(ActiveSupport::BroadcastLogger, Logger, Vonage::Logger)), got type SemanticLogger::Logger with hash 2142317253973070942
       Caller: /home/rade/.rbenv/versions/3.3.6/lib/ruby/gems/3.3.0/gems/vonage-7.28.0/lib/vonage/config.rb:15
       Definition: /home/rade/.rbenv/versions/3.3.6/lib/ruby/gems/3.3.0/gems/vonage-7.28.0/lib/vonage/config.rb:139 (Vonage::Config#logger=)

Config:

def client
      @client ||= Vonage::Client.new(
        api_key: ENV["VONAGE_API_KEY"],
        api_secret: ENV["VONAGE_API_SECRET"],
        logger: nil,
      ).sms
    end

[superchilled]

Hi @rade-tomovic. Setting :logger to nil (as you have done in the Client options) should normally disable logging. Looking at the error, it seems like an issue with the Sorbet type signature for Config#logger, which is confusing because it should be nilable (see this line), so I'm not too sure what's happening here.

I'll need to investigate further. Will get back to you once I know more.

[rade-tomovic]

The only way to go around it is to monkey patch config with ugly overrides, but definitely not something I would like to use as solution for production - I would still like to use SemanticLogger as is.

# config/initalizers
# frozen_string_literal: true

module Vonage
  class SemanticLoggerAdapter < ::Logger
    def initialize(semantic_logger)
      @semantic_logger = semantic_logger
      super($stdout) # Parent needs an IO object but we won't use it
    end

    ::Logger::Severity.constants.each do |severity|
      define_method(severity.downcase) do |*args, &block|
        message = block_given? ? block.call : args.first
        @semantic_logger.send(severity.downcase, message)
      end
    end
  end

  class Config
    def logger=(logger)
      @logger = Logger.new(SemanticLoggerAdapter.new(logger))
    end
  end
end

[superchilled]

@rade-tomovic sorry, I've not had a chance to look at this in depth yet. TBH I'm not familiar with SemanticLogger. I'll try and carve out some time over the next week or two to dig into it.

[psbordjukov]

Hi, I just hit this as well -- if you have any logger configured apart from the default logger, the library fails to even initialize. This is due to the fact that the Vonage::Config#logger= enforces a specific implementation or nil, and Vonage::Config eagerly sets the logger (using Vonage::Config#logger=) to what the default Rails logger is.

I respect that types are enforced in this library, however the logger implementation being hardcoded (as opposed to an interface) is not good OOP design.

Having said that, I did not succeed in finding a way to alter the Vonage::Config class to check for the logger interface as opposed to an implementation. Not without altering Ruby's and SemanticLogger's class definitions to extend a common Sorbet interface. I take it this is a limitation of Sorbet.

My first suggestion here would be to make the logger= method unchecked.

My second suggestion would be to make the logger check if the default Rails logger is indeed a ::Logger upon initialization of the Vonage::Config class. This allows for a user to define a wrapper to their application's logger that inherits from Vonage::Logger. This is a compromise with usability and devex for the sake of retaining the type check.

I will open a PR that follows the second approach.

[superchilled]

Hi @psbordjukov. Sorry that this kind of fell off my radar a bit due to other priorities. Thanks for opening the PR. I'll take a look this week.

[psbordjukov]

@superchilled, yw! The entirety of the change amounts to replacing a nil check on Rails.logger with an is_a? check. The bulk of the PR are tests and docs.

[superchilled]

Fixed in v7.30.2