Skip to content

Commit c11a499

Browse files
authored
fix: prevent SDK crash when SDK logging fails (#2817)
* Add mise.toml to .gitignore * fix: prevent SDK crash when SDK logging fails custom SDK loggers can crash the SDK, we guard against that by returning the exception to stderr * Rubo👮❤️ * Update CHANGELOG.md * use exception message instead of original message which might still contain e.g. encoding errors * dry & add sanitized original msg in stderr output & simplify test case from GitHub issue #2805 * more Rubo👮❤️ * guard against any error in stderr logging io errors, re-assigment * Add stacktrace to stderr logging * 🧽 🧽 🧽
1 parent 854f4ca commit c11a499

File tree

4 files changed

+142
-4
lines changed

4 files changed

+142
-4
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ node_modules
2121
.devcontainer/.env
2222
vendor/gems
2323
sentry-rails/Gemfile-*.lock
24+
mise.toml

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
# This now works too and the nested hash is dumped to JSON string
4040
Sentry.logger.info("Hello World", extra: { today: Date.today, user_id: user.id })
4141
```
42+
- Prevent SDK crash when SDK logging fails ([#2817](https://github.com/getsentry/sentry-ruby/pull/2817))
4243

4344
## 6.2.0
4445

sentry-ruby/lib/sentry/utils/logging_helper.rb

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,26 +6,40 @@ module LoggingHelper
66
# @!visibility private
77
def log_error(message, exception, debug: false)
88
message = "#{message}: #{exception.message}"
9-
message += "\n#{exception.backtrace.join("\n")}" if debug
9+
message += "\n#{exception.backtrace.join("\n")}" if debug && exception.backtrace
1010

11-
sdk_logger&.error(LOGGER_PROGNAME) do
12-
message
13-
end
11+
sdk_logger&.error(LOGGER_PROGNAME) { message }
12+
rescue StandardError => e
13+
log_to_stderr(e, message)
1414
end
1515

1616
# @!visibility private
1717
def log_debug(message)
1818
sdk_logger&.debug(LOGGER_PROGNAME) { message }
19+
rescue StandardError => e
20+
log_to_stderr(e, message)
1921
end
2022

2123
# @!visibility private
2224
def log_warn(message)
2325
sdk_logger&.warn(LOGGER_PROGNAME) { message }
26+
rescue StandardError => e
27+
log_to_stderr(e, message)
2428
end
2529

2630
# @!visibility private
2731
def sdk_logger
2832
@sdk_logger ||= Sentry.sdk_logger
2933
end
34+
35+
# @!visibility private
36+
def log_to_stderr(error, message)
37+
error_msg = "Sentry SDK logging failed (#{error.class}: #{error.message}): #{message}".scrub(%q(<?>))
38+
error_msg += "\n#{error.backtrace.map { |line| line.scrub(%q(<?>)) }.join("\n")}" if error.backtrace
39+
40+
$stderr.puts(error_msg)
41+
rescue StandardError
42+
# swallow everything – logging must never crash the app
43+
end
3044
end
3145
end
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe Sentry::LoggingHelper do
4+
let(:string_io) { StringIO.new }
5+
let(:logger) { Logger.new(string_io) }
6+
7+
let(:helper_class) do
8+
Class.new do
9+
include Sentry::LoggingHelper
10+
attr_accessor :sdk_logger
11+
12+
def initialize(sdk_logger)
13+
@sdk_logger = sdk_logger
14+
end
15+
end
16+
end
17+
18+
let(:logger_helper) { helper_class.new(logger) }
19+
20+
describe "#log_error" do
21+
it "logs exception message with description" do
22+
exception = StandardError.new("Something went wrong")
23+
logger_helper.log_error("test_error", exception)
24+
25+
expect(string_io.string).to include("test_error: Something went wrong")
26+
end
27+
28+
it "includes backtrace when debug is true" do
29+
exception = StandardError.new("Error")
30+
exception.set_backtrace(["it_broke.rb:1"])
31+
32+
logger_helper.log_error("test_error", exception, debug: true)
33+
34+
expect(string_io.string).to include("it_broke.rb:1")
35+
end
36+
end
37+
38+
describe "stderr fallback when logger fails" do
39+
shared_examples "falls back to stderr" do |method_name, *args|
40+
it "outputs to stderr with error class and message" do
41+
broken_logger = Class.new do
42+
def debug(*); raise IOError, "oops"; end
43+
def warn(*); raise IOError, "oops"; end
44+
end.new
45+
46+
helper = helper_class.new(broken_logger)
47+
48+
expect($stderr).to receive(:puts).with(/Sentry SDK logging failed \(IOError:/)
49+
expect { helper.public_send(method_name, *args) }.not_to raise_error
50+
end
51+
end
52+
53+
context "#log_debug" do
54+
include_examples "falls back to stderr", :log_debug, "Debug message"
55+
end
56+
57+
context "#log_warn" do
58+
include_examples "falls back to stderr", :log_warn, "Warning message"
59+
end
60+
61+
it "includes backtrace in stderr output" do
62+
broken_logger = Class.new do
63+
def error(*)
64+
error = IOError.new("oops")
65+
error.set_backtrace([
66+
"logger.rb:42:in `write'",
67+
"logger.rb:10:in `error'"
68+
])
69+
70+
raise error
71+
end
72+
end.new
73+
74+
helper = helper_class.new(broken_logger)
75+
76+
stderr_output = nil
77+
expect($stderr).to receive(:puts) { |msg| stderr_output = msg }
78+
79+
helper.log_error("Test message", StandardError.new("Error"))
80+
81+
expect(stderr_output).to include("IOError: oops")
82+
expect(stderr_output).to include("logger.rb:42:in `write'")
83+
expect(stderr_output).to include("logger.rb:10:in `error'")
84+
end
85+
end
86+
87+
describe "custom JSON logger with encoding errors" do
88+
# Custom logger from GitHub issue #2805
89+
let(:json_logger) do
90+
Class.new(::Logger) do
91+
class JsonFormatter
92+
def call(level, _, _, m)
93+
{ severity: level, message: m }.to_json << "\n"
94+
end
95+
end
96+
97+
def initialize(*)
98+
super
99+
self.formatter = JsonFormatter.new
100+
end
101+
end.new(StringIO.new)
102+
end
103+
104+
let(:logger_helper) { helper_class.new(json_logger) }
105+
106+
it "scrubs invalid UTF-8 in stderr output when JSON logger fails on encoding" do
107+
helper = helper_class.new(json_logger)
108+
109+
invalid_message = "a\x92b"
110+
exception = StandardError.new("oops")
111+
112+
stderr_message = nil
113+
expect($stderr).to receive(:puts) { |msg| stderr_message = msg }
114+
115+
expect { helper.log_error(invalid_message, exception) }.not_to raise_error
116+
117+
expect(stderr_message).to include("JSON::GeneratorError")
118+
expect(stderr_message).to include("a<?>b")
119+
expect(stderr_message).to include("oops")
120+
end
121+
end
122+
end

0 commit comments

Comments
 (0)