From 5455e7aea13fc88c317d18472152a432002fe31f Mon Sep 17 00:00:00 2001 From: Digvijay Singh Rawat Date: Tue, 14 Apr 2026 20:36:32 +0530 Subject: [PATCH] Add centralized error reporting via Rage::Errors Allow users to register error reporters that receive all framework-caught exceptions, so they can forward them to services like Sentry or Bugsnag without manually rescuing in every component. Rage.configure do config.errors << SentryReporter.new end Reporters can accept just the exception or an optional context hash. Signature detection uses Rage::Internal.build_arguments at registration time to keep the reporting path fast. Each reporter is called in its own rescue block so a broken reporter doesn't affect the rest. Duplicate prevention uses an instance variable on the exception (similar to Rails). Manually-created exceptions get a real backtrace via re-raise rather than set_backtrace(caller). Hooked into all existing framework rescue sites: controllers, fiber wrapper, Cable connections/channels, Redis adapter, deferred tasks, event subscribers, SSE streams, and telemetry tracers. Signed-off-by: Digvijay Singh Rawat --- CHANGELOG.md | 1 + lib/rage/application.rb | 1 + lib/rage/cable/adapters/redis.rb | 1 + lib/rage/cable/cable.rb | 3 +- lib/rage/cable/channel.rb | 3 +- lib/rage/configuration.rb | 8 ++ lib/rage/deferred/task.rb | 2 + lib/rage/errors.rb | 68 ++++++++++++++ lib/rage/events/subscriber.rb | 7 +- lib/rage/middleware/fiber_wrapper.rb | 1 + lib/rage/sse/application.rb | 1 + lib/rage/telemetry/tracer.rb | 1 + spec/cable/adapters/redis_spec.rb | 14 +++ spec/deferred/task_spec.rb | 6 ++ spec/errors_spec.rb | 130 +++++++++++++++++++++++++++ spec/events/deferred_spec.rb | 3 + spec/events/publish_spec.rb | 1 + spec/sse/application_spec.rb | 3 + spec/telemetry/tracer_spec.rb | 2 + 19 files changed, 253 insertions(+), 3 deletions(-) create mode 100644 spec/errors_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 4530b6cc..1fcc5d04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Fix `Rage::UploadedFile#close` (#262). ### Added +- [Errors] Add centralized error reporting interface via Rage::Errors by [@Digvijay-x1](https://github.com/Digvijay-x1). - [SSE] Add tests for log context propagation across fiber boundaries by [@jsxs0](https://github.com/jsxs0) (#267). - Add singular `resource` routing with plural controller mapping and document the helper by [@anuj-pal27](https://github.com/anuj-pal27) (#247). - [SSE] Add support for unbounded streams (#266). diff --git a/lib/rage/application.rb b/lib/rage/application.rb index 56a4799a..4af66392 100644 --- a/lib/rage/application.rb +++ b/lib/rage/application.rb @@ -23,6 +23,7 @@ def call(env) response = @exception_app.call(400, e) rescue Exception => e + Rage::Errors.report(e) response = @exception_app.call(500, e) ensure diff --git a/lib/rage/cable/adapters/redis.rb b/lib/rage/cable/adapters/redis.rb index 3016f461..7cce848f 100644 --- a/lib/rage/cable/adapters/redis.rb +++ b/lib/rage/cable/adapters/redis.rb @@ -117,6 +117,7 @@ def poll rescue RedisClient::Error => e Rage.logger.error("Subscriber error: #{e.message} (#{e.class})") + Rage::Errors.report(e) sleep error_backoff_intervals.next rescue => e @stopping ? break : raise(e) diff --git a/lib/rage/cable/cable.rb b/lib/rage/cable/cable.rb index 5e66131c..aa23ca3b 100644 --- a/lib/rage/cable/cable.rb +++ b/lib/rage/cable/cable.rb @@ -125,7 +125,8 @@ def schedule_fiber(connection) end def log_error(e) - Rage.logger.error("Unhandled exception has occured - #{e.class} (#{e.message}):\n#{e.backtrace.join("\n")}") + Rage.logger.error("Unhandled exception has occurred - #{e.class} (#{e.message}):\n#{e.backtrace.join("\n")}") + Rage::Errors.report(e) end end diff --git a/lib/rage/cable/channel.rb b/lib/rage/cable/channel.rb index f9e752c7..8cc086ff 100644 --- a/lib/rage/cable/channel.rb +++ b/lib/rage/cable/channel.rb @@ -331,7 +331,8 @@ def set_up_periodic_timers Fiber.schedule do slice.each { |channel| callback.call(channel) } rescue => e - Rage.logger.error("Unhandled exception has occured - #{e.class} (#{e.message}):\n#{e.backtrace.join("\n")}") + Rage.logger.error("Unhandled exception has occurred - #{e.class} (#{e.message}):\n#{e.backtrace.join("\n")}") + Rage::Errors.report(e) end end end diff --git a/lib/rage/configuration.rb b/lib/rage/configuration.rb index 528718f4..714544e9 100644 --- a/lib/rage/configuration.rb +++ b/lib/rage/configuration.rb @@ -219,6 +219,14 @@ def cable end # @!endgroup + # @!group Error Reporting + # Returns the centralized error reporting interface. + # @return [Rage::Errors] + def errors + Rage::Errors + end + # @!endgroup + # @!group OpenAPI Configuration # Allows configuring OpenAPI settings. # @return [Rage::Configuration::OpenAPI] diff --git a/lib/rage/deferred/task.rb b/lib/rage/deferred/task.rb index 820b98cd..1e29680c 100644 --- a/lib/rage/deferred/task.rb +++ b/lib/rage/deferred/task.rb @@ -80,6 +80,8 @@ def __perform(context) true rescue Exception => e + Rage::Errors.report(e) + unless respond_to?(:__deferred_suppress_exception_logging?, true) && __deferred_suppress_exception_logging? Rage.logger.with_context(task_log_context) do Rage.logger.error("Deferred task failed with exception: #{e.class} (#{e.message}):\n#{e.backtrace.join("\n")}") diff --git a/lib/rage/errors.rb b/lib/rage/errors.rb index 3d1f2ba1..49e64ffa 100644 --- a/lib/rage/errors.rb +++ b/lib/rage/errors.rb @@ -1,4 +1,72 @@ +# frozen_string_literal: true + module Rage::Errors + @reporters = [] + + class << self + # Register a new error reporter. + # A reporter should respond to `#call` and accept one of: + # - `call(exception)` + # - `call(exception, context: {})` + # + # @param reporter [#call] + # @return [self] + def <<(reporter) + raise ArgumentError, "reporter must respond to #call" unless reporter.respond_to?(:call) + + index = @reporters.length + @reporters << reporter + + arguments = Rage::Internal.build_arguments( + reporter.method(:call), + { context: "context" } + ) + call_arguments = arguments.empty? ? "" : ", #{arguments}" + + singleton_class.class_eval <<~RUBY, __FILE__, __LINE__ + 1 + def __report_#{index}(exception, context) + @reporters[#{index}].call(exception#{call_arguments}) + end + RUBY + + self + end + + # Forward an exception to all registered reporters. + # + # @param exception [Exception] + # @param context [Hash] + # @return [nil] + def report(exception, context: {}) + return if @reporters.empty? + return if exception.instance_variable_defined?(:@_rage_error_reported) + + ensure_backtrace(exception) + + @reporters.length.times do |i| + __send__(:"__report_#{i}", exception, context) + rescue => e + Rage.logger.error("Error reporter #{@reporters[i].class} failed: #{e.class} (#{e.message})") + end + + exception.instance_variable_set(:@_rage_error_reported, true) unless exception.frozen? + + nil + end + + private + + def ensure_backtrace(exception) + return if exception.frozen? + return unless exception.backtrace.nil? + + begin + raise exception + rescue exception.class + end + end + end + class BadRequest < StandardError end diff --git a/lib/rage/events/subscriber.rb b/lib/rage/events/subscriber.rb index cff45cbe..0eed1e5f 100644 --- a/lib/rage/events/subscriber.rb +++ b/lib/rage/events/subscriber.rb @@ -90,7 +90,12 @@ def __call(event, context: nil) Rage.logger.with_context(self.class.__log_context) do Rage.logger.error("Subscriber failed with exception: #{e.class} (#{e.message}):\n#{e.backtrace.join("\n")}") end - raise e if self.class.__is_deferred + + if self.class.__is_deferred + raise e + else + Rage::Errors.report(e) + end end private diff --git a/lib/rage/middleware/fiber_wrapper.rb b/lib/rage/middleware/fiber_wrapper.rb index f77db773..3f8a65a3 100644 --- a/lib/rage/middleware/fiber_wrapper.rb +++ b/lib/rage/middleware/fiber_wrapper.rb @@ -20,6 +20,7 @@ def call(env) rescue Exception => e exception_str = "#{e.class} (#{e.message}):\n#{e.backtrace.join("\n")}" Rage.logger << exception_str + Rage::Errors.report(e) if Rage.env.development? [500, {}, [exception_str]] else diff --git a/lib/rage/sse/application.rb b/lib/rage/sse/application.rb index 60ecd0db..cbc49266 100644 --- a/lib/rage/sse/application.rb +++ b/lib/rage/sse/application.rb @@ -63,6 +63,7 @@ def start_stream(connection) end rescue => e Rage.logger.error("SSE stream failed with exception: #{e.class} (#{e.message}):\n#{e.backtrace.join("\n")}") + Rage::Errors.report(e) ensure Iodine.task_dec! end diff --git a/lib/rage/telemetry/tracer.rb b/lib/rage/telemetry/tracer.rb index fda6bca4..1f92abae 100644 --- a/lib/rage/telemetry/tracer.rb +++ b/lib/rage/telemetry/tracer.rb @@ -74,6 +74,7 @@ def #{tracer_name(span.id)}(#{parameters}) #{calls_chain} rescue Exception => e Rage.logger.error("Telemetry handler failed with error \#{e}:\\n\#{e.backtrace.join("\\n")}") + Rage::Errors.report(e) end unless yield_called diff --git a/spec/cable/adapters/redis_spec.rb b/spec/cable/adapters/redis_spec.rb index 1b41b158..4ab1469f 100644 --- a/spec/cable/adapters/redis_spec.rb +++ b/spec/cable/adapters/redis_spec.rb @@ -185,6 +185,7 @@ describe "#poll" do before do allow_any_instance_of(described_class).to receive(:pick_a_worker).and_yield + allow_any_instance_of(described_class).to receive(:sleep) allow(Iodine).to receive(:on_state).with(:start_shutdown).and_yield end @@ -267,5 +268,18 @@ subject end + + it "reports redis subscriber errors" do + allow(mock_redis).to receive(:blocking_call).and_invoke( + proc { raise RedisClient::CannotConnectError, "redis down" }, + proc { raise } + ) + + logger = double(error: nil) + allow(Rage).to receive(:logger).and_return(logger) + expect(Rage::Errors).to receive(:report).with(instance_of(RedisClient::CannotConnectError)) + + subject + end end end diff --git a/spec/deferred/task_spec.rb b/spec/deferred/task_spec.rb index 8cce2131..0e47c538 100644 --- a/spec/deferred/task_spec.rb +++ b/spec/deferred/task_spec.rb @@ -305,6 +305,7 @@ def perform(arg, kwarg:) allow(Rage::Deferred::Context).to receive(:get_log_context).with(context).and_return({}) allow(task).to receive(:perform).and_raise(error) allow(error).to receive(:backtrace).and_return(["line 1", "line 2"]) + allow(Rage::Errors).to receive(:report) end it "logs the error" do @@ -312,6 +313,11 @@ def perform(arg, kwarg:) expect(logger).to have_received(:error).with("Deferred task failed with exception: StandardError (Something went wrong):\nline 1\nline 2") end + it "reports the error" do + task.__perform(context) + expect(Rage::Errors).to have_received(:report).with(error) + end + it "returns the exception" do expect(task.__perform(context)).to be(error) end diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb new file mode 100644 index 00000000..fe30bc6b --- /dev/null +++ b/spec/errors_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +RSpec.describe Rage::Errors do + around do |example| + original_reporters = described_class.instance_variable_get(:@reporters) + described_class.instance_variable_set(:@reporters, []) + example.run + ensure + described_class.instance_variable_set(:@reporters, original_reporters) + end + + describe ".<<" do + it "requires reporters to respond to #call" do + expect { + described_class << Object.new + }.to raise_error(ArgumentError, "reporter must respond to #call") + end + end + + describe ".report" do + let(:logger) { double(error: nil) } + + before do + allow(Rage).to receive(:logger).and_return(logger) + end + + it "returns nil when no reporters are registered" do + expect(described_class.report(StandardError.new("test"))).to be_nil + end + + it "calls a simple reporter with exception only" do + reporter = Class.new do + attr_reader :exception + + def call(exception) + @exception = exception + end + end.new + + error = StandardError.new("test") + described_class << reporter + described_class.report(error, context: { user_id: 42 }) + + expect(reporter.exception).to be(error) + end + + it "calls a reporter with context when supported" do + reporter = Class.new do + attr_reader :exception, :context + + def call(exception, context: {}) + @exception = exception + @context = context + end + end.new + + error = StandardError.new("test") + described_class << reporter + described_class.report(error, context: { user_id: 42 }) + + expect(reporter.exception).to be(error) + expect(reporter.context).to eq({ user_id: 42 }) + end + + it "adds backtrace for manually-created exceptions" do + reporter = Class.new do + attr_reader :exception + + def call(exception) + @exception = exception + end + end.new + + error = StandardError.new("test") + expect(error.backtrace).to be_nil + + described_class << reporter + described_class.report(error) + + expect(reporter.exception.backtrace).to be_an(Array) + expect(reporter.exception.backtrace).not_to be_empty + end + + it "continues reporting when one reporter fails" do + failed_reporter = Class.new do + def call(_exception) + raise "boom" + end + end.new + + successful_reporter = Class.new do + attr_reader :exception + + def call(exception) + @exception = exception + end + end.new + + error = StandardError.new("test") + described_class << failed_reporter + described_class << successful_reporter + + described_class.report(error) + + expect(successful_reporter.exception).to be(error) + expect(logger).to have_received(:error).with(/Error reporter .* failed: RuntimeError \(boom\)/) + end + + it "does not report the same exception twice" do + call_count = 0 + reporter = Class.new do + define_method(:call) { |_exception| call_count += 1 } + end.new + + error = StandardError.new("test") + described_class << reporter + + described_class.report(error) + described_class.report(error) + + expect(call_count).to eq(1) + end + end + + describe "configuration integration" do + it "is available via config.errors" do + expect(Rage::Configuration.new.errors).to eq(described_class) + end + end +end diff --git a/spec/events/deferred_spec.rb b/spec/events/deferred_spec.rb index 0f226a8b..815dba1e 100644 --- a/spec/events/deferred_spec.rb +++ b/spec/events/deferred_spec.rb @@ -74,11 +74,14 @@ def call(event) context "with exception" do it "it re-raises the exception" do + allow(Rage::Errors).to receive(:report) expect(logger).to receive(:error).with(/Subscriber failed with exception: RuntimeError/) expect { EventsDeferredSpec::EventWithExceptionSubscriber.new.perform(EventsDeferredSpec::EventWithException.new) }.to raise_error(RuntimeError, "test") + + expect(Rage::Errors).not_to have_received(:report) end end end diff --git a/spec/events/publish_spec.rb b/spec/events/publish_spec.rb index 298cb118..bbc862e1 100644 --- a/spec/events/publish_spec.rb +++ b/spec/events/publish_spec.rb @@ -232,6 +232,7 @@ class EventWithAppendSubscriber < BaseSubscriber context "with exception inside subscriber" do it "correctly handles events" do expect(logger).to receive(:error).with(/test error/) + expect(Rage::Errors).to receive(:report).with(instance_of(RuntimeError)) expect { described_class.publish(EventsPublishSpec::EventWithException.new) diff --git a/spec/sse/application_spec.rb b/spec/sse/application_spec.rb index f3c6ee7e..edb860b0 100644 --- a/spec/sse/application_spec.rb +++ b/spec/sse/application_spec.rb @@ -56,12 +56,15 @@ def open? logger = double("logger") allow(Rage).to receive(:logger).and_return(logger) allow(logger).to receive(:error) + allow(Rage::Errors).to receive(:report) expect(Iodine).to receive(:task_inc!) expect(Iodine).to receive(:task_dec!) app = described_class.new(stream) app.on_open(connection) + + expect(Rage::Errors).to have_received(:report).with(instance_of(RuntimeError)) end end diff --git a/spec/telemetry/tracer_spec.rb b/spec/telemetry/tracer_spec.rb index f56d9981..f2af2f5a 100644 --- a/spec/telemetry/tracer_spec.rb +++ b/spec/telemetry/tracer_spec.rb @@ -271,6 +271,7 @@ def self.test_instrumentation end it "logs the error" do + expect(Rage::Errors).to receive(:report).with(instance_of(RuntimeError)) expect(Rage.logger).to receive(:error) do |msg| expect(msg).to match(/Telemetry handler failed with error/) expect(msg).to include("test error") @@ -304,6 +305,7 @@ def self.test_instrumentation end it "logs the error" do + expect(Rage::Errors).to receive(:report).with(instance_of(RuntimeError)) expect(Rage.logger).to receive(:error) do |msg| expect(msg).to match(/Telemetry handler failed with error/) expect(msg).to include("test error")