Skip to content

Commit 2f19782

Browse files
byrootandrewn617dustinbrownman
committed
ErrorReporter#unexpected to report in production but raise in development
It's a common useful pattern for situation where something isn't supposed to happen, but if it does we can recover from it. So in such situation you don't want such issue to be hidden in development or test, as it's likely a bug, but do not want to fail a request if it happens in production. In other words, it behaves like `#record` in development and test environments, and like `raise` in production. Fix: rails#49638 Fix: rails#49339 Co-Authored-By: Andrew Novoselac <[email protected]> Co-Authored-By: Dustin Brown <[email protected]>
1 parent c2a3305 commit 2f19782

File tree

4 files changed

+96
-4
lines changed

4 files changed

+96
-4
lines changed

activesupport/CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
* Add `ErrorReported#unexpected` to report precondition violations.
2+
3+
For example:
4+
5+
```ruby
6+
def edit
7+
if published?
8+
Rails.error.unexpected("[BUG] Attempting to edit a published article, that shouldn't be possible")
9+
return false
10+
end
11+
# ...
12+
end
13+
```
14+
15+
The above will raise an error in development and test, but only report the error in production.
16+
17+
*Jean Boussier*
18+
119
* Make the order of read_multi and write_multi notifications for `Cache::Store#fetch_multi` operations match the order they are executed in.
220

321
*Adam Renberg Tamm*

activesupport/lib/active_support/error_reporter.rb

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,16 @@ module ActiveSupport
2626
class ErrorReporter
2727
SEVERITIES = %i(error warning info)
2828
DEFAULT_SOURCE = "application"
29+
DEFAULT_RESCUE = [StandardError].freeze
2930

30-
attr_accessor :logger
31+
attr_accessor :logger, :debug_mode
32+
33+
UnexpectedError = Class.new(Exception)
3134

3235
def initialize(*subscribers, logger: nil)
3336
@subscribers = subscribers.flatten
3437
@logger = logger
38+
@debug_mode = false
3539
end
3640

3741
# Evaluates the given block, reporting and swallowing any unhandled error.
@@ -72,7 +76,7 @@ def initialize(*subscribers, logger: nil)
7276
# source of the error. Subscribers can use this value to ignore certain
7377
# errors. Defaults to <tt>"application"</tt>.
7478
def handle(*error_classes, severity: :warning, context: {}, fallback: nil, source: DEFAULT_SOURCE)
75-
error_classes = [StandardError] if error_classes.blank?
79+
error_classes = DEFAULT_RESCUE if error_classes.empty?
7680
yield
7781
rescue *error_classes => error
7882
report(error, handled: true, severity: severity, context: context, source: source)
@@ -108,13 +112,47 @@ def handle(*error_classes, severity: :warning, context: {}, fallback: nil, sourc
108112
# source of the error. Subscribers can use this value to ignore certain
109113
# errors. Defaults to <tt>"application"</tt>.
110114
def record(*error_classes, severity: :error, context: {}, source: DEFAULT_SOURCE)
111-
error_classes = [StandardError] if error_classes.blank?
115+
error_classes = DEFAULT_RESCUE if error_classes.empty?
112116
yield
113117
rescue *error_classes => error
114118
report(error, handled: false, severity: severity, context: context, source: source)
115119
raise
116120
end
117121

122+
# Either report the given error when in production, or raise it when in development or test.
123+
#
124+
# When called in production, after the error is reported, this method will return
125+
# nil and execution will continue.
126+
#
127+
# When called in development, the original error is wrapped in a different error class to ensure
128+
# it's not being rescued higher in the stack and will be surfaced to the developer.
129+
#
130+
# This method is intended for reporting violated assertions about preconditions, or similar
131+
# cases that can and should be gracefully handled in production, but that aren't supposed to happen.
132+
#
133+
# The error can be either an exception instance or a String.
134+
#
135+
# example:
136+
#
137+
# def edit
138+
# if published?
139+
# Rails.error.unexpected("[BUG] Attempting to edit a published article, that shouldn't be possible")
140+
# return false
141+
# end
142+
# # ...
143+
# end
144+
#
145+
def unexpected(error, severity: :warning, context: {}, source: DEFAULT_SOURCE)
146+
error = RuntimeError.new(error) if error.is_a?(String)
147+
error.set_backtrace(caller(1)) if error.backtrace.nil?
148+
149+
if @debug_mode
150+
raise UnexpectedError, "#{error.class.name}: #{error.message}", error.backtrace, cause: error
151+
else
152+
report(error, handled: true, severity: severity, context: context, source: source)
153+
end
154+
end
155+
118156
# Register a new error subscriber. The subscriber must respond to
119157
#
120158
# report(Exception, handled: Boolean, severity: (:error OR :warning OR :info), context: Hash, source: String)

activesupport/test/error_reporter_test.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,38 @@ class ErrorReporterTest < ActiveSupport::TestCase
168168
assert_equal 4, result
169169
end
170170

171+
test "#unexpected swallows errors by default" do
172+
error = RuntimeError.new("Oops")
173+
assert_nil @reporter.unexpected(error)
174+
assert_equal [[error, true, :warning, "application", {}]], @subscriber.events
175+
assert_not_predicate error.backtrace, :empty?
176+
end
177+
178+
test "#unexpected accepts an error message" do
179+
assert_nil @reporter.unexpected("Oops")
180+
assert_equal 1, @subscriber.events.size
181+
182+
error, *event_details = @subscriber.events.first
183+
assert_equal [true, :warning, "application", {}], event_details
184+
185+
assert_equal "Oops", error.message
186+
assert_equal RuntimeError, error.class
187+
assert_not_predicate error.backtrace, :empty?
188+
end
189+
190+
test "#unexpected re-raise errors in development and test" do
191+
@reporter.debug_mode = true
192+
error = RuntimeError.new("Oops")
193+
raise_line = __LINE__ + 2
194+
raised_error = assert_raises ActiveSupport::ErrorReporter::UnexpectedError do
195+
@reporter.unexpected(error)
196+
end
197+
assert_includes raised_error.message, "RuntimeError: Oops"
198+
assert_not_nil raised_error.cause
199+
assert_same error, raised_error.cause
200+
assert_includes raised_error.backtrace.first, "#{__FILE__}:#{raise_line}"
201+
end
202+
171203
test "can have multiple subscribers" do
172204
second_subscriber = ErrorSubscriber.new
173205
@reporter.subscribe(second_subscriber)

railties/lib/rails/application/bootstrap.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,12 @@ module Bootstrap
6161
broadcast_logger.formatter = Rails.logger.formatter
6262
Rails.logger = broadcast_logger
6363
end
64+
end
6465

65-
unless config.consider_all_requests_local
66+
initializer :initialize_error_reporter, group: :all do
67+
if config.consider_all_requests_local
68+
Rails.error.debug_mode = true
69+
else
6670
Rails.error.logger = Rails.logger
6771
end
6872
end

0 commit comments

Comments
 (0)