appsignal/appsignal-ruby

Broadcasting logs to Appsignal fails to record exception

oliverguenther opened this issue · 5 comments

Describe the bug

If you broadcast Rails logs to Appsignal, it will fail to record an exception, even though that is a valid argument to Logger#add: https://docs.ruby-lang.org/en/master/Logger.html#class-Logger-label-Message

To Reproduce

Steps to reproduce the behavior:

  • Using AppSignal for Ruby gem version 3.0

  • With this initializer

appsignal_logger = Appsignal::Logger.new("rails")
Rails.logger.broadcast_to(appsignal_logger)
  • Run this code
Rails.logger.error ArgumentError.new("Test!")

Expected behavior

The exception is logged, probably as exception.inspect as the ruby documentation states.

Actual behavior

Another exception is raised: wrong argument type ArgumentError (expected String)

Hi @oliverguenther, thanks for the report! We'll fix the error.
If you're using this logger feature to report errors, I would suggest using our Appsignal.set_error or Appsignal.send_error instead, read more on our exception handling docs page.
Our error reporting works much better that way.

Hi @oliverguenther, thanks for the report! We'll fix the error. If you're using this logger feature to report errors, I would suggest using our Appsignal.set_error or Appsignal.send_error instead, read more on our exception handling docs page. Our error reporting works much better that way.

Thanks for the reply. Good point on setting the error directly. I wonder if you could do that as a side effect in case someone logs an exception - which appears to be a valid use-case anyway according to the Logger#add documentation.

Thanks for the reply. Good point on setting the error directly. I wonder if you could do that as a side effect in case someone logs an exception - which appears to be a valid use-case anyway according to the Logger#add documentation.

It could, but we have decided not to do this. Our integration only support reporting one error per transaction (web request, background job, rake tasks, etc.). If the Logger.error method call would also set the error on the transaction, it might overwrite whatever error was already set on it, losing that error report.
Since Logger.error is called manually, we want to leave that decision up to whoever is writing that, and not do it automatically. We'll support multiple errors in one transaction in the future. For this, this seems like the best way.

I've created PR #1096 to fix the error.

Thanks for getting this fixed so quickly! 🫶

@oliverguenther I've released the fix in Ruby gem 3.8.0.