LogStashLogger must not fail with an exception

[stefanchrobot]

The LogStashLogger can raise an exception when logging. This makes it unsuitable for error logging, as that would require error handling code for error handling code.

Note that this issue is not about the particular error (see https://github.com/dwbutler/logstash-logger/issues/158 for that), rather about the whole approach - logging must not raise exceptions.

Reproduction:

s = "x\xc3x".force_encoding('ASCII-8BIT')

l = Logger.new(STDOUT)
l.info(s)
# fine
# I, [2020-04-06T09:16:57.787645 #39066]  INFO -- : x�x

ll = LogStashLogger.new(type: :stdout)
ll.info(s)
# explodes
# Encoding::UndefinedConversionError: "\xC3" from ASCII-8BIT to UTF-8
# from /Users/stefan/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/logstash-event-1.2.02/lib/logstash/event.rb:169:in `to_json'

Expected result: Swallow any error. Optionally log a warning.

Actual result: Exception.

[marcoadkins]

@stefanchrobot Implemented a fix for this. You have any thoughts on the change? https://github.com/dwbutler/logstash-logger/pull/165

[stefanchrobot]

@marcoadkins Thanks for taking the time to fix this! The change looks good, but I'm wondering if it would make sense to add a catch-all clause here: https://github.com/dwbutler/logstash-logger/pull/165/files#diff-247c8156e87173ba822a64e0e41ec50cR8

rescue StandardError
  log_error(e)
  'failed to format log event'
end

This would protect from any future errors that might pop out. I think it would make sense to add it in the Base formatter to avoid repeating it in all the places.

[marcoadkins]

@stefanchrobot Thats a good idea. I could add that in. Can't add it to the base formatter because they all override the format_event method so it wouldn't propagate.

[marcoadkins]

@stefanchrobot I see what you were saying now. The recommended change has been made.