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
orAppsignal.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.