Skip to content

Commit c53d837

Browse files
committed
Use #report parameters in error context middleware
`ErrorReporter#report` has a number of other parameters besides context. `source` in particular can be useful for creating metadata based on the source of the error. Error context middleware `#call` now has the same signature as `#report`.
1 parent 8009769 commit c53d837

File tree

3 files changed

+31
-11
lines changed

3 files changed

+31
-11
lines changed

activesupport/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
When reporting an error, the error context middleware will be called with the reported error
44
and base execution context. The stack may mutate the context hash. The mutated context will
5-
then be passed to error subscribers.
5+
then be passed to error subscribers. Middleware receives the same parameters as `ErrorReporter#report`.
66

77
*Andrew Novoselac*, *Sam Schmidt*
88

activesupport/lib/active_support/error_reporter.rb

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ def set_context(...)
209209
# before passing to subscribers. Allows creation of entries in error context that
210210
# are shared by all subscribers.
211211
#
212-
# A context middleware receives the error and current state of the context hash.
212+
# A context middleware receives the same parameters as #report.
213213
# It must return a hash - the middleware stack returns the hash after it has
214214
# run through all middlewares. A middleware can mutate or replace the hash.
215215
#
@@ -242,7 +242,10 @@ def report(error, handled: true, severity: handled ? :warning : :error, context:
242242

243243
full_context = @context_middlewares.execute(
244244
error,
245-
ActiveSupport::ExecutionContext.to_h.merge(context || {})
245+
context: ActiveSupport::ExecutionContext.to_h.merge(context || {}),
246+
handled:,
247+
severity:,
248+
source:
246249
)
247250

248251
disabled_subscribers = ActiveSupport::IsolatedExecutionState[self]
@@ -308,9 +311,9 @@ def use(middleware)
308311
@stack << middleware
309312
end
310313

311-
# Run all middlewares in the stack on an error and context.
312-
def execute(error, context)
313-
@stack.inject(context) { |c, middleware| middleware.call(error, c) }
314+
# Run all middlewares in the stack
315+
def execute(error, handled:, severity:, context:, source:)
316+
@stack.inject(context) { |c, middleware| middleware.call(error, context: c, handled:, severity:, source:) }
314317
end
315318
end
316319
end

activesupport/test/error_reporter_test.rb

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ def report(_error, handled:, severity:, context:, source:)
364364
end
365365

366366
test "error context middleware can mutate context hash" do
367-
middleware = -> (_, context) { context.merge({ foo: :bar }) }
367+
middleware = -> (_, context:, **kwargs) { context.merge({ foo: :bar }) }
368368

369369
error = ArgumentError.new("Oops")
370370

@@ -375,13 +375,13 @@ def report(_error, handled:, severity:, context:, source:)
375375
end
376376

377377
class MyErrorContextMiddleware
378-
def call(_, context)
378+
def call(_, context:, **kwargs)
379379
context.merge({ bar: :baz })
380380
end
381381
end
382382

383383
test "can have multiple error context middlewares" do
384-
@reporter.add_middleware(-> (_, context) { context.merge({ foo: :bar }) })
384+
@reporter.add_middleware(-> (_, context:, **kwargs) { context.merge({ foo: :bar }) })
385385
@reporter.add_middleware(MyErrorContextMiddleware.new)
386386

387387
error = ArgumentError.new("Oops")
@@ -391,11 +391,28 @@ def call(_, context)
391391
end
392392

393393
test "last error context middleware to update a key wins" do
394-
@reporter.add_middleware(-> (_, context) { context.merge({ foo: :bar }) })
395-
@reporter.add_middleware(-> (_, context) { context.merge({ foo: :baz }) })
394+
@reporter.add_middleware(-> (_, context:, **kwargs) { context.merge({ foo: :bar }) })
395+
@reporter.add_middleware(-> (_, context:, **kwargs) { context.merge({ foo: :baz }) })
396396
error = ArgumentError.new("Oops")
397397
@reporter.report(error)
398398

399399
assert_equal [[error, true, :warning, "application", { foo: :baz }]], @subscriber.events
400400
end
401+
402+
test "error context middleware receives same parameters as #report" do
403+
reported_error = ArgumentError.new("Oops")
404+
@reporter.add_middleware(-> (error, context:, handled:, severity:, source:) {
405+
assert_equal reported_error, error
406+
assert_equal Hash.new, context
407+
assert_equal true, handled
408+
assert_equal :warning, severity
409+
assert_equal ActiveSupport::ErrorReporter::DEFAULT_SOURCE, source
410+
411+
context.merge({ foo: :bar })
412+
})
413+
414+
@reporter.report(reported_error)
415+
416+
assert_equal [[reported_error, true, :warning, "application", { foo: :bar }]], @subscriber.events
417+
end
401418
end

0 commit comments

Comments
 (0)