Errors shipping logs to sentry can crash main app thread

[robertjlooby]

Issue Description

We had a case where a networking issue that happened while attempting to ship logs to Sentry crashed the main application thread. Stacktrace:

ERROR Exception: Sentry::ExternalError: Failed to open TCP connection to sentry.xxx.com:443 (execution expired)'
Backtrace:
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/transport/http_transport.rb:64:in `rescue in send_data'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/transport/http_transport.rb:31:in `send_data'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/transport.rb:62:in `send_envelope'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/client.rb:319:in `send_envelope'
...
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/client.rb:308:in `send_logs'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/log_event_buffer.rb:71:in `send_events'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/log_event_buffer.rb:54:in `block in add_event'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/log_event_buffer.rb:52:in `synchronize'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/log_event_buffer.rb:52:in `add_event'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/client.rb:100:in `buffer_log_event'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/hub.rb:227:in `capture_log_event'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry-ruby.rb:499:in `capture_log'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/structured_logger.rb:132:in `log'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/structured_logger.rb:74:in `debug'
.../bundle/ruby/3.2.0/gems/sentry-ruby-6.1.0/lib/sentry/std_lib_logger.rb:40:in `add'
.../bundle/ruby/3.2.0/gems/logger-1.7.0/lib/logger.rb:715:in `debug'
.../bundle/ruby/3.2.0/gems/activesupport-7.2.2/lib/active_support/broadcast_logger.rb:122:in `block in debug'
.../bundle/ruby/3.2.0/gems/activesupport-7.2.2/lib/active_support/broadcast_logger.rb:231:in `block in dispatch'
.../bundle/ruby/3.2.0/gems/activesupport-7.2.2/lib/active_support/broadcast_logger.rb:231:in `each'
.../bundle/ruby/3.2.0/gems/activesupport-7.2.2/lib/active_support/broadcast_logger.rb:231:in `dispatch'
.../bundle/ruby/3.2.0/gems/activesupport-7.2.2/lib/active_support/broadcast_logger.rb:122:in `debug'

From looking through the gem code, I think there are 2 separate issues with the current implementation of LogEventBuffer.

1. If there are greater than max_log_events in the buffer, then logs are flushed to Sentry on the main thread without any error handling. This looks to be what happened in our case.
2. If this had happened in the background ThreadedPeriodicWorker thread of LogEventBuffer, then I don't think it would have crashed our app, but it would have killed that thread and then from that point forward all log shipping would have taken place on the main thread. I don't see any additional calls to ensure_thread that would recover from an error in the LogEventBuffer background thread.

Reproduction Steps

Force an error somewhere in the send_events/send_logs/send_envelope/send_data stack while flushing log events.

Expected Behavior

I expect errors in Sentry to never crash the application code (particularly log shipping).

Actual Behavior

An error pushing logs to Sentry can crash application code.

Ruby Version

3.2.1

SDK Version

24.8.0

Integration and Its Version

sentry-ruby (6.1.0), sentry-rails (6.1.0)

Sentry Config

Sentry.init do |config|
  config.breadcrumbs_logger = [:active_support_logger]
  config.enable_logs = true
  config.enabled_patches << :faraday
  config.enabled_patches << :logger
end

[linear]

RUBY-118

[solnic]

Thanks for such a detailed report! I'll deal with this tomorrow and we'll push a bug-fix release.