Rails 7.1's new BroadcastLogger causes TypeError in Vonage::Config

ohbarye commented 10 months ago

Problem

Rails 7.1 introduced ActiveSupport::BroadcastLogger and it violates vonage's type definition.

ref https://github.com/rails/rails/pull/48615

Reproduce procedure

Call Vonage::Client.new with rails 7.1.0 and vonage 7.16.0.

Expected Behavior

No error happens.

Actual Behavior

It raises TypeError.

     TypeError:
       Parameter 'logger': Expected type T.nilable(T.any(Logger, Vonage::Logger)), got type ActiveSupport::BroadcastLogger with hash -2750353119102352789
       Caller: /usr/local/bundle/ruby/3.2.0/gems/vonage-7.16.0/lib/vonage/config.rb:15
       Definition: /usr/local/bundle/ruby/3.2.0/gems/vonage-7.16.0/lib/vonage/config.rb:134
     # /usr/local/bundle/ruby/3.2.0/gems/sorbet-runtime-0.5.11039/lib/types/configuration.rb:296:in `call_validation_error_handler_default'

Cause

That's because vonage's type definition does not allow a class that does not inherits Logger unlike ActiveSupport::Logger .

ref https://github.com/Vonage/vonage-ruby-sdk/blob/b3122cbf4d5953eeb4be84fec27914394628206e/lib/vonage/config.rb#L133-L135

Other information

Here is the difference of Rails.logger between 7.0.8 and 7.1.0.

# 7.0.8
$ bin/rails c
> ::Rails.logger.class
=> ActiveSupport::Logger

> ActiveSupport::Logger.ancestors
=> 
[ActiveSupport::Logger,
 ActiveSupport::LoggerThreadSafeLevel,
 ActiveSupport::LoggerSilence,
 Logger,
 AwesomePrint::Logger,
 Logger::Severity,
 ActiveSupport::Dependencies::RequireDependency,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Object,
 PP::ObjectMixin,
 ActiveSupport::Tryable,
 JSON::Ext::Generator::GeneratorMethods::Object,
 DEBUGGER__::TrapInterceptor,
 Kernel,
 BasicObject]

# 7.1.0
$ bin/rails c
> Rails.logger.class
=> ActiveSupport::BroadcastLogger

> ActiveSupport::BroadcastLogger.ancestors
=> 
[ActiveSupport::BroadcastLogger,                                                   
 ActiveSupport::LoggerThreadSafeLevel,                                             
 ActiveSupport::LoggerSilence,                                                     
 ActiveSupport::Dependencies::RequireDependency,                                   
 ActiveSupport::ToJsonWithActiveSupportEncoder,                                    
 Object,                                                                           
 PP::ObjectMixin,                                                                  
 ActiveSupport::Tryable,                                                           
 JSON::Ext::Generator::GeneratorMethods::Object,                                   
 DEBUGGER__::TrapInterceptor,                                                      
 Kernel,                                                                           
 BasicObject]

ohbarye commented 10 months ago

Although I'm unfamiliar with Sorbet, I can send an easy patch to change the type definition like the one below. I don't know if it's valid and appropriate.

 sig { params(logger: T.nilable(
    defined?(ActiveSupport::BroadcastLogger) ? 
      T.any(::Logger, Vonage::Logger, ActiveSupport::BroadcastLogger) # rails 7.1.0 introduced 
    : T.any(::Logger, Vonage::Logger)
  )).returns(T.nilable(Vonage::Logger)) } 
 def logger=(logger) 
 end

ohbarye commented 10 months ago

Just to let you know, I found a runtime check behavior configuration of Sorbet.

https://sorbet.org/docs/tconfiguration#errors-from-invalid-method-calls

After executing the following code, my code stopped raising a TypeError. I feel weird about writing a config for Sorbet even though I don't use Sorbet as a user. 😓

T::Configuration.call_validation_error_handler = lambda do |signature, opts|
  puts opts[:message]
end

Or, SORBET_RUNTIME_DEFAULT_CHECKED_LEVEL=tests environment variable seems to disable raising an error.

https://sorbet.org/docs/tconfiguration#sorbet_runtime_default_checked_level