Use different error code for code action errors (#2634)

Author: vinistock

lib/ruby_lsp/base_server.rb

@@ -130,6 +130,12 @@ def process_message(message); end
     sig { abstract.void }
     def shutdown; end
 
+    sig { params(id: Integer, message: String, type: Integer).void }
+    def fail_request_and_notify(id, message, type: Constant::MessageType::INFO)
+      send_message(Error.new(id: id, code: Constant::ErrorCodes::REQUEST_FAILED, message: message))
+      send_message(Notification.window_show_message(message, type: type))
+    end
+
     sig { returns(Thread) }
     def new_worker
       Thread.new do

lib/ruby_lsp/server.rb

@@ -285,8 +285,9 @@ def run_initialized
         rescue RuboCop::Error => e
           # The user may have provided unknown config switches in .rubocop or
           # is trying to load a non-existant config file.
-          send_message(Notification.window_show_error(
+          send_message(Notification.window_show_message(
             "RuboCop configuration error: #{e.message}. Formatting will not be available.",
+            type: Constant::MessageType::ERROR,
           ))
         end
       end
@@ -531,10 +532,16 @@ def text_document_formatting(message)
       response = Requests::Formatting.new(@global_state, document).perform
       send_message(Result.new(id: message[:id], response: response))
     rescue Requests::Request::InvalidFormatter => error
-      send_message(Notification.window_show_error("Configuration error: #{error.message}"))
+      send_message(Notification.window_show_message(
+        "Configuration error: #{error.message}",
+        type: Constant::MessageType::ERROR,
+      ))
       send_empty_response(message[:id])
     rescue StandardError, LoadError => error
-      send_message(Notification.window_show_error("Formatting error: #{error.message}"))
+      send_message(Notification.window_show_message(
+        "Formatting error: #{error.message}",
+        type: Constant::MessageType::ERROR,
+      ))
       send_empty_response(message[:id])
     end
 
@@ -673,30 +680,19 @@ def code_action_resolve(message)
       document = @store.get(uri)
 
       unless document.is_a?(RubyDocument)
-        send_message(Notification.window_show_error("Code actions are currently only available for Ruby documents"))
-        raise Requests::CodeActionResolve::CodeActionError
+        fail_request_and_notify(message[:id], "Code actions are currently only available for Ruby documents")
+        return
       end
 
       result = Requests::CodeActionResolve.new(document, params).perform
 
       case result
       when Requests::CodeActionResolve::Error::EmptySelection
-        send_message(Notification.window_show_error("Invalid selection for Extract Variable refactor"))
-        raise Requests::CodeActionResolve::CodeActionError
+        fail_request_and_notify(message[:id], "Invalid selection for extract variable refactor")
       when Requests::CodeActionResolve::Error::InvalidTargetRange
-        send_message(
-          Notification.window_show_error(
-            "Couldn't find an appropriate location to place extracted refactor",
-          ),
-        )
-        raise Requests::CodeActionResolve::CodeActionError
+        fail_request_and_notify(message[:id], "Couldn't find an appropriate location to place extracted refactor")
       when Requests::CodeActionResolve::Error::UnknownCodeAction
-        send_message(
-          Notification.window_show_error(
-            "Unknown code action",
-          ),
-        )
-        raise Requests::CodeActionResolve::CodeActionError
+        fail_request_and_notify(message[:id], "Unknown code action")
       else
         send_message(Result.new(id: message[:id], response: result))
       end
@@ -729,10 +725,16 @@ def text_document_diagnostic(message)
         ),
       )
     rescue Requests::Request::InvalidFormatter => error
-      send_message(Notification.window_show_error("Configuration error: #{error.message}"))
+      send_message(Notification.window_show_message(
+        "Configuration error: #{error.message}",
+        type: Constant::MessageType::ERROR,
+      ))
       send_empty_response(message[:id])
     rescue StandardError, LoadError => error
-      send_message(Notification.window_show_error("Error running diagnostics: #{error.message}"))
+      send_message(Notification.window_show_message(
+        "Error running diagnostics: #{error.message}",
+        type: Constant::MessageType::ERROR,
+      ))
       send_empty_response(message[:id])
     end
 
@@ -971,7 +973,7 @@ def perform_initial_indexing
         rescue StandardError => error
           message = "Error while indexing (see [troubleshooting steps]" \
             "(https://shopify.github.io/ruby-lsp/troubleshooting#indexing)): #{error.message}"
-          send_message(Notification.window_show_error(message))
+          send_message(Notification.window_show_message(message, type: Constant::MessageType::ERROR))
         end
 
         # Indexing produces a high number of short lived object allocations. That might lead to some fragmentation and
@@ -1056,8 +1058,9 @@ def check_formatter_is_available
         @global_state.formatter = "none"
 
         send_message(
-          Notification.window_show_error(
+          Notification.window_show_message(
             "Ruby LSP formatter is set to `rubocop` but RuboCop was not found in the Gemfile or gemspec.",
+            type: Constant::MessageType::ERROR,
           ),
         )
       end

lib/ruby_lsp/utils.rb

@@ -64,14 +64,11 @@ class Notification < Message
     class << self
       extend T::Sig
 
-      sig { params(message: String).returns(Notification) }
-      def window_show_error(message)
+      sig { params(message: String, type: Integer).returns(Notification) }
+      def window_show_message(message, type: Constant::MessageType::INFO)
         new(
           method: "window/showMessage",
-          params: Interface::ShowMessageParams.new(
-            type: Constant::MessageType::ERROR,
-            message: message,
-          ),
+          params: Interface::ShowMessageParams.new(type: type, message: message),
         )
       end
 

vscode/src/client.ts

@@ -27,6 +27,7 @@ import {
   ClientCapabilities,
   FeatureState,
   ServerCapabilities,
+  ErrorCodes,
 } from "vscode-languageclient/node";
 
 import {
@@ -397,7 +398,15 @@ export default class Client extends LanguageClient implements ClientInterface {
         } else {
           const { errorMessage, errorClass, backtrace } = error.data;
 
-          if (errorMessage && errorClass && backtrace) {
+          // We only want to produce telemetry events for errors that have all the data we need and that are internal
+          // server errors. Other errors do not necessarily indicate bugs in the server. You can check LSP error codes
+          // here https://microsoft.github.io/language-server-protocol/specification/#errorCodes
+          if (
+            errorMessage &&
+            errorClass &&
+            backtrace &&
+            error.code === ErrorCodes.InternalError
+          ) {
             // Sanitize the backtrace coming from the server to remove the user's home directory from it, then mark it
             // as a trusted value. Otherwise the VS Code telemetry logger redacts the entire backtrace and we are unable
             // to see where in the server the error occurred