Adopt cmd error handling like sgx-jmp

Amolith created

So it doesn't fall over on errors and we have to rely on a supervisor to
bring it back up a couple minutes after. This also adds Sentry error
reporting, only tested with the mock, not with actual GlitchTip.

Change summary

sgx-bwmsgsv2.rb        | 78 ++++++++++++++++++++++++++++++++++++-------
test/test_component.rb | 29 ++++++++++++++++
2 files changed, 93 insertions(+), 14 deletions(-)

Detailed changes

sgx-bwmsgsv2.rb 🔗

@@ -114,27 +114,77 @@ def anonymous_tel?(dest)
 end
 
 class SGXClient < Blather::Client
-	def register_handler(type, *guards, &block)
-		super(type, *guards) { |*args| wrap_handler(*args, &block) }
+	def handle_data(stanza)
+		promise = EMPromise.resolve(nil).then {
+			with_sentry(stanza) do |scope|
+				super
+			rescue StandardError => e
+				handle_error(scope, stanza, e)
+			end
+		}.catch { |e| panic(e) }
+		promise.sync if ENV["ENV"] == "test"
+		promise
 	end
 
-	def register_handler_before(type, *guards, &block)
-		check_handler(type, guards)
-		handler = lambda { |*args| wrap_handler(*args, &block) }
+	# Override the default call_handler to syncify during testing.
+	def call_handler(handler, guards, stanza)
+		result = if guards.first.respond_to?(:to_str)
+			found = stanza.find(*guards)
+			throw :pass if found.empty?
+
+			handler.call(stanza, found)
+		else
+			throw :pass if guarded?(guards, stanza)
+
+			handler.call(stanza)
+		end
 
-		@handlers[type] ||= []
-		@handlers[type].unshift([guards, handler])
+		# Up to here, identical to upstream impl
+
+		return result unless result.is_a?(Promise)
+
+		result.sync if ENV["ENV"] == "test"
+		result
 	end
 
 protected
 
-	def wrap_handler(*args)
-		v = yield(*args)
-		v = v.sync if ENV['ENV'] == 'test' && v.is_a?(Promise)
-		v.catch(&method(:panic)) if v.is_a?(Promise)
-		true # Do not run other handlers unless throw :pass
-	rescue Exception => e
-		panic(e)
+	def with_sentry(stanza)
+		Sentry.clone_hub_to_current_thread
+
+		Sentry.with_scope do |scope|
+			setup_scope(stanza, scope)
+			yield scope
+		ensure
+			scope.get_transaction&.then do |tx|
+				tx.set_status("ok") unless tx.status
+				tx.finish
+			end
+		end
+	end
+
+	def setup_scope(stanza, scope)
+		name = stanza.respond_to?(:node) ? stanza.node : stanza.name
+		scope.clear_breadcrumbs
+		scope.set_transaction_name(name)
+		scope.set_user(jid: stanza.from&.stripped.to_s)
+
+		transaction = Sentry.start_transaction(
+			name: name,
+			op: "blather.handle_data"
+		)
+		scope.set_span(transaction) if transaction
+	end
+
+	def handle_error(scope, stanza, e)
+		puts "Error raised during #{scope.transaction_name}: #{e.class}"
+		puts e.message
+		puts e.backtrace
+		Sentry.capture_exception(e) unless e.is_a?(Sentry::Error)
+		scope.get_transaction&.set_status("internal_error")
+		return if e.respond_to?(:replied?) && e.replied?
+
+		SGXbwmsgsv2.write_to_stream stanza.as_error("internal-server-error", :cancel)
 	end
 end
 

test/test_component.rb 🔗

@@ -513,4 +513,33 @@ class ComponentTest < Minitest::Test
 		end
 	end
 	em :test_port_out_pin_validation
+
+	def test_sentry_captures_handler_exception
+		captured_exceptions = []
+		repo = SGXbwmsgsv2.instance_variable_get(:@registration_repo)
+
+		Sentry.stub :capture_exception, ->(*args, **) { captured_exceptions << args.first } do
+			# repo.find is called during message processing to look up creds;
+			# raising here bubbles up to SGXClient#handle_error
+			repo.stub :find, ->(_) { raise StandardError, "test error" } do
+				m = Blather::Stanza::Message.new("+15551234567@component", "hello")
+				m.from = "test@example.com"
+				process_stanza(m)
+			end
+		end
+
+		assert_equal 1, captured_exceptions.length
+		assert_instance_of StandardError, captured_exceptions.first
+		assert_equal "test error", captured_exceptions.first.message
+
+		assert_equal 1, written.length
+		stanza = Blather::XMPPNode.parse(written.first.to_xml)
+		assert stanza.error?
+		error = stanza.find_first("error")
+		assert_equal "cancel", error["type"]
+		assert_equal "internal-server-error", xmpp_error_name(error)
+
+		SGXbwmsgsv2.instance_variable_set(:@written, [])
+	end
+	em :test_sentry_captures_handler_exception
 end