Skip to content

Commit b99284c

Browse files
committed
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.
1 parent 5c3c53d commit b99284c

File tree

2 files changed

+61
-0
lines changed

2 files changed

+61
-0
lines changed

sentry-ruby/lib/sentry/log_event_buffer.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ def size
6969

7070
def send_events
7171
@client.send_logs(@pending_events)
72+
@pending_events.clear
73+
rescue => e
74+
log_debug("[LogEventBuffer] Failed to send logs: #{e.message}")
75+
7276
@pending_events.clear
7377
end
7478
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)