Introduce Addon::notify to standardize error handling for add-ons

lib/ruby_lsp/addon.rb

@@ -2,7 +2,7 @@
 # frozen_string_literal: true
 
 module RubyLsp
-  # To register an add-on, inherit from this class and implement both `name` and `activate`
+  # To register an add-on, inherit from this class and implement `name`, `activate`, and `deactivate`.
   #
   # # Example
   #
@@ -13,6 +13,10 @@ module RubyLsp
   #       # Perform any relevant initialization
   #     end
   #
+  #     def deactivate
+  #       # Clean up resources like file watchers or child processes
+  #     end
+  #
   #     def name
   #       "My add-on name"
   #     end
@@ -145,6 +149,23 @@ def depend_on_ruby_lsp!(*version_constraints)
             "Add-on is not compatible with this version of the Ruby LSP. Skipping its activation"
         end
       end
+
+      # Notifies the given add-on of a specific event by invoking the corresponding method on the add-on.
+      #
+      # Prevents the add-on from crashing the process if it raises a StandardError or SystemExit.
+      #
+      # Example:
+      # ```ruby
+      # Addon.notify(addon, :create_code_lens_listener, response_builder, uri, dispatcher)
+      # ```
+      #: (Addon addon, Symbol event, *Object) -> untyped
+      def notify(addon, event, *args)
+        T.unsafe(addon).send(event, *args) if addon.respond_to?(event)
+      rescue SystemExit
+        puts "Add-on #{addon.name} attempted to exit the process while calling #{event} with args (#{args}). Ignoring..."
+      rescue StandardError => e
+        puts "Add-on #{addon.name} raised an error while calling #{event} with args (#{args}): #{e.full_message}"
+      end
     end
 
     #: -> void

lib/ruby_lsp/requests/code_lens.rb

@@ -26,7 +26,7 @@ def initialize(global_state, uri, dispatcher)
         Listeners::CodeLens.new(@response_builder, global_state, uri, dispatcher)
 
         Addon.addons.each do |addon|
-          addon.create_code_lens_listener(@response_builder, uri, dispatcher)
+          Addon.notify(addon, :create_code_lens_listener, @response_builder, uri, dispatcher)
         end
       end
 

lib/ruby_lsp/requests/completion.rb

@@ -74,7 +74,7 @@ def initialize(document, global_state, params, sorbet_level, dispatcher)
         )
 
         Addon.addons.each do |addon|
-          addon.create_completion_listener(@response_builder, node_context, dispatcher, document.uri)
+          Addon.notify(addon, :create_completion_listener, @response_builder, node_context, dispatcher, document.uri)
         end
 
         matched = node_context.node

lib/ruby_lsp/requests/definition.rb

@@ -86,7 +86,7 @@ def initialize(document, global_state, position, dispatcher, sorbet_level)
           )
 
           Addon.addons.each do |addon|
-            addon.create_definition_listener(@response_builder, document.uri, node_context, dispatcher)
+            Addon.notify(addon, :create_definition_listener, @response_builder, document.uri, node_context, dispatcher)
           end
         end
 

lib/ruby_lsp/requests/discover_tests.rb

@@ -40,7 +40,7 @@ def perform
           Listeners::SpecStyle.new(@response_builder, @global_state, @dispatcher, @document.uri)
 
           Addon.addons.each do |addon|
-            addon.create_discover_tests_listener(@response_builder, @dispatcher, @document.uri)
+            Addon.notify(addon, :create_discover_tests_listener, @response_builder, @dispatcher, @document.uri)
           end
 
           @dispatcher.visit(@document.parse_result.value)
@@ -58,7 +58,7 @@ def perform
             Listeners::SpecStyle.new(@response_builder, @global_state, @dispatcher, @document.uri)
 
             Addon.addons.each do |addon|
-              addon.create_discover_tests_listener(@response_builder, @dispatcher, @document.uri)
+              Addon.notify(addon, :create_discover_tests_listener, @response_builder, @dispatcher, @document.uri)
             end
 
             # Dispatch the events both for indexing the test file and discovering the tests. The order here is

lib/ruby_lsp/requests/document_symbol.rb

@@ -27,7 +27,7 @@ def initialize(uri, dispatcher)
         Listeners::DocumentSymbol.new(@response_builder, uri, dispatcher)
 
         Addon.addons.each do |addon|
-          addon.create_document_symbol_listener(@response_builder, dispatcher)
+          Addon.notify(addon, :create_document_symbol_listener, @response_builder, dispatcher)
         end
       end
 

lib/ruby_lsp/requests/hover.rb

@@ -53,7 +53,7 @@ def initialize(document, global_state, position, dispatcher, sorbet_level)
         @response_builder = ResponseBuilders::Hover.new #: ResponseBuilders::Hover
         Listeners::Hover.new(@response_builder, global_state, uri, node_context, dispatcher, sorbet_level)
         Addon.addons.each do |addon|
-          addon.create_hover_listener(@response_builder, node_context, dispatcher)
+          Addon.notify(addon, :create_hover_listener, @response_builder, node_context, dispatcher)
         end
 
         @dispatcher = dispatcher

lib/ruby_lsp/requests/semantic_highlighting.rb

@@ -87,7 +87,7 @@ def initialize(global_state, dispatcher, document, previous_result_id, range: ni
         Listeners::SemanticHighlighting.new(dispatcher, @response_builder)
 
         Addon.addons.each do |addon|
-          addon.create_semantic_highlighting_listener(@response_builder, dispatcher)
+          Addon.notify(addon, :create_semantic_highlighting_listener, @response_builder, dispatcher)
         end
       end
 

lib/ruby_lsp/server.rb

@@ -1056,12 +1056,7 @@ def workspace_did_change_watched_files(message)
       end
 
       Addon.file_watcher_addons.each do |addon|
-        T.unsafe(addon).workspace_did_change_watched_files(changes)
-      rescue => e
-        send_log_message(
-          "Error in #{addon.name} add-on while processing watched file notifications: #{e.full_message}",
-          type: Constant::MessageType::ERROR,
-        )
+        Addon.notify(addon, :workspace_did_change_watched_files, changes)
       end
     end
 
@@ -1362,7 +1357,7 @@ def window_show_message_request(message)
       addon = Addon.addons.find { |addon| addon.name == addon_name }
       return unless addon
 
-      addon.handle_window_show_message_response(result[:title])
+      Addon.notify(addon, :handle_window_show_message_response, result[:title])
     end
 
     # NOTE: all servers methods are void because they can produce several messages for the client. The only reason this
@@ -1462,7 +1457,7 @@ def resolve_test_commands(message)
       commands = Listeners::TestStyle.resolve_test_commands(items)
 
       Addon.addons.each do |addon|
-        commands.concat(addon.resolve_test_commands(items))
+        commands.concat(Addon.notify(addon, :resolve_test_commands, items))
       end
 
       send_message(Result.new(

test/addon_test.rb

@@ -188,5 +188,53 @@ def version
         assert_predicate(T.unsafe(addon), :hello)
       end
     end
+
+    def test_an_addon_calling_exit_or_raising_does_not_quit_lsp
+      Addon.load_addons(@global_state, @outgoing_queue)
+
+      addon = Addon.get("My Add-on", "0.1.0")
+
+      listeners = addon.public_methods.grep(/_listener$/)
+      other_commands = [
+        :handle_window_show_message_response,
+        :resolve_test_commands,
+        :workspace_did_change_watched_files,
+      ]
+
+      commands_that_should_not_quit_lsp = [*listeners, *other_commands]
+
+      failures = []
+
+      commands_that_should_not_quit_lsp.each do |command|
+        # Check how `exit` is handled
+        addon.define_singleton_method(command) { Kernel.exit }
+
+        begin
+          capture_io { Addon.notify(addon, command, nil) }
+        rescue SystemExit
+          failures << "Addon.notify(addon, #{command.inspect}, args) should not be able to exit the LSP process"
+        end
+
+        # Check how `abort` is handled
+        addon.define_singleton_method(command) { Kernel.abort }
+
+        begin
+          capture_io { Addon.notify(addon, command, nil) }
+        rescue SystemExit
+          failures << "Addon.notify(addon, #{command.inspect}, args) should not be able to abort the LSP process"
+        end
+
+        # Check how `raise` is handled
+        addon.define_singleton_method(command) { raise StandardError }
+
+        begin
+          capture_io { Addon.notify(addon, command, nil) }
+        rescue StandardError
+          failures << "Addon.notify(addon, #{command.inspect}, args) should not raise an error"
+        end
+      end
+
+      assert_empty(failures, failures.join("\n"))
+    end
   end
 end