Skip to content

Commit fac9cc3

Browse files
authored
fix(logging): prevent log errors from crashing main thread (#2795)
* fix(logging): prevent log errors from crashing main thread Fixes #2793 When log events are shipped to Sentry (either synchronously when the buffer is full, or via the background thread), network errors could propagate to the main application thread and crash the application. * Update CHANGELOG
1 parent 5c3c53d commit fac9cc3

File tree

3 files changed

+69
-1
lines changed

3 files changed

+69
-1
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
- Include otel as custom sampling context ([2683](https://github.com/getsentry/sentry-ruby/pull/2683))
66

7+
### Fixes
8+
9+
- Prevent logging from crashing main thread ([2795](https://github.com/getsentry/sentry-ruby/pull/2795))
10+
711
## 6.1.2
812

913
### Fixes

sentry-ruby/lib/sentry/log_event_buffer.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,18 @@ def size
6565
@pending_events.size
6666
end
6767

68+
def clear!
69+
@pending_events.clear
70+
end
71+
6872
private
6973

7074
def send_events
7175
@client.send_logs(@pending_events)
72-
@pending_events.clear
76+
rescue => e
77+
log_debug("[LogEventBuffer] Failed to send logs: #{e.message}")
78+
ensure
79+
clear!
7380
end
7481
end
7582
end

sentry-ruby/spec/sentry/log_event_buffer_spec.rb

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,61 @@
7474
expect(log_event_buffer).to be_empty
7575
end
7676
end
77+
78+
describe "error handling" do
79+
let(:max_log_events) { 3 }
80+
81+
let(:error) { Errno::ECONNREFUSED.new("Connection refused") }
82+
83+
context "when send_logs raises an exception" do
84+
before do
85+
allow(client).to receive(:send_logs).and_raise(error)
86+
end
87+
88+
it "does not propagate exception from add_event when buffer is full" do
89+
expect {
90+
3.times { log_event_buffer.add_event(log_event) }
91+
}.not_to raise_error
92+
end
93+
94+
it "does not propagate exception from flush" do
95+
2.times { log_event_buffer.add_event(log_event) }
96+
97+
expect {
98+
log_event_buffer.flush
99+
}.not_to raise_error
100+
end
101+
102+
it "logs the error to sdk_logger" do
103+
3.times { log_event_buffer.add_event(log_event) }
104+
105+
expect(string_io.string).to include("Failed to send logs")
106+
end
107+
108+
it "clears the buffer after a failed send to avoid memory buildup" do
109+
3.times { log_event_buffer.add_event(log_event) }
110+
111+
expect(log_event_buffer).to be_empty
112+
end
113+
end
114+
115+
context "when background thread encounters an error" do
116+
let(:max_log_events) { 100 }
117+
118+
before do
119+
allow(client).to receive(:send_logs).and_raise(error)
120+
end
121+
122+
it "keeps the background thread alive after an error" do
123+
log_event_buffer.add_event(log_event)
124+
log_event_buffer.start
125+
126+
thread = log_event_buffer.instance_variable_get(:@thread)
127+
128+
expect(thread).to be_alive
129+
expect { log_event_buffer.flush }.not_to raise_error
130+
expect(thread).to be_alive
131+
end
132+
end
133+
end
77134
end

0 commit comments

Comments
 (0)