Skip to content

Commit aca093f

Browse files
authored
Merge pull request rails#54330 from kmcphillips/main
Improve argument validation in `ActiveSupport::ErrorReporter#handle`
2 parents a274d4f + 747f8fa commit aca093f

File tree

2 files changed

+35
-1
lines changed

2 files changed

+35
-1
lines changed

activesupport/lib/active_support/error_reporter.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,15 @@ def set_context(...)
209209
#
210210
def report(error, handled: true, severity: handled ? :warning : :error, context: {}, source: DEFAULT_SOURCE)
211211
return if error.instance_variable_defined?(:@__rails_error_reported)
212+
raise ArgumentError, "Reported error must be an Exception, got: #{error.inspect}" unless error.is_a?(Exception)
213+
212214
ensure_backtrace(error)
213215

214216
unless SEVERITIES.include?(severity)
215217
raise ArgumentError, "severity must be one of #{SEVERITIES.map(&:inspect).join(", ")}, got: #{severity.inspect}"
216218
end
217219

218-
full_context = ActiveSupport::ExecutionContext.to_h.merge(context)
220+
full_context = ActiveSupport::ExecutionContext.to_h.merge(context || {})
219221
disabled_subscribers = ActiveSupport::IsolatedExecutionState[self]
220222
@subscribers.each do |subscriber|
221223
unless disabled_subscribers&.any? { |s| s === subscriber }

activesupport/test/error_reporter_test.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,38 @@ class ErrorReporterTest < ActiveSupport::TestCase
257257
assert_equal :error, @subscriber.events.dig(0, 2)
258258
end
259259

260+
test "errors be reported with valid severity" do
261+
ActiveSupport::ErrorReporter::SEVERITIES.each do |severity|
262+
@reporter.report(StandardError.new, severity: severity)
263+
assert_equal severity, @subscriber.events.last[2]
264+
end
265+
end
266+
267+
test "errors with invalid severity raise" do
268+
assert_raises ArgumentError do
269+
@reporter.report(@error, severity: :invalid)
270+
end
271+
end
272+
273+
test "report raises if passed an argument that is not an Exception" do
274+
error = assert_raises ArgumentError do
275+
@reporter.report(Object.new)
276+
end
277+
assert_includes error.message, "Reported error must be an Exception"
278+
end
279+
280+
test "report raises if passed a String" do
281+
error = assert_raises ArgumentError do
282+
@reporter.report("An error message")
283+
end
284+
assert_includes error.message, "Reported error must be an Exception"
285+
end
286+
287+
test "report accepts context as nil" do
288+
@reporter.report(@error, context: nil)
289+
assert_equal({}, @subscriber.events.last[4])
290+
end
291+
260292
test "report errors only once" do
261293
assert_difference -> { @subscriber.events.size }, +1 do
262294
@reporter.report(@error, handled: false)

0 commit comments

Comments
 (0)