Restore request cancellation with correct implementation (#3791)

Ruby LSP doesn't currently cancel requests, because it processes  `$/cancelRequest` notifications in the same queue as other requests, so previous requests will get processed before they get the chance to get cancelled.

This was initially addressed in #2939, with cancelled requests returning an error response. This is matching the LSP spec, which requires either an incomplete or an error response. However, this started causing issues with Neovim as reported in #3019, so that implementation was ultimately reverted.

The problem wasn't actually the error responses, it was that Ruby LSP was also returning a regular response, *on top of* the error response. So, a cancellation notification was resulting in *two* responses being returned for the same request, which rightfully caused errors in Neovim, as it didn't know how to handle the 2nd response.

This happened because the `next` keyword breaks out of the nearest loop *or* closure, which was the `Mutex#synchronize` block, it didn't actually skip to the next iteration. This caused `process_message` to execute even if the cancel response was already sent, which ended up sending another (regular) response.

To avoid introducing boolean local variables, I extracted the loop body into a separate method, so that I can just use `return` to break out. I verified that this works as expected in Zed, so I also expect it to work well in Neovim.

Author: janko

lib/ruby_lsp/base_server.rb

@@ -83,7 +83,7 @@ def start
         # The following requests need to be executed in the main thread directly to avoid concurrency issues. Everything
         # else is pushed into the incoming queue
         case method
-        when "initialize", "initialized", "rubyLsp/diagnoseState"
+        when "initialize", "initialized", "rubyLsp/diagnoseState", "$/cancelRequest"
           process_message(message)
         when "shutdown"
           @global_state.synchronize do
@@ -154,20 +154,29 @@ def fail_request_and_notify(id, message, type: Constant::MessageType::INFO)
     def new_worker
       Thread.new do
         while (message = @incoming_queue.pop)
-          id = message[:id]
-
-          # Check if the request was cancelled before trying to process it
-          @global_state.synchronize do
-            if id && @cancelled_requests.include?(id)
-              send_message(Result.new(id: id, response: nil))
-              @cancelled_requests.delete(id)
-              next
-            end
-          end
+          handle_incoming_message(message)
+        end
+      end
+    end
 
-          process_message(message)
+    #: (Hash[Symbol, untyped]) -> void
+    def handle_incoming_message(message)
+      id = message[:id]
+
+      # Check if the request was cancelled before trying to process it
+      @global_state.synchronize do
+        if id && @cancelled_requests.include?(id)
+          send_message(Error.new(
+            id: id,
+            code: Constant::ErrorCodes::REQUEST_CANCELLED,
+            message: "Request #{id} was cancelled",
+          ))
+          @cancelled_requests.delete(id)
+          return
         end
       end
+
+      process_message(message)
     end
 
     #: ((Result | Error | Notification | Request) message) -> void

test/server_test.rb

@@ -828,7 +828,7 @@ def handle_window_show_message_response(title)
     end
   end
 
-  def test_cancelling_requests_returns_nil
+  def test_cancelling_requests_returns_expected_error_code
     uri = URI("file:///foo.rb")
 
     @server.process_message({
@@ -868,8 +868,9 @@ def test_cancelling_requests_returns_nil
     mutex.unlock
     thread.join
 
-    result = find_message(RubyLsp::Result)
-    assert_nil(result.response)
+    error = find_message(RubyLsp::Error)
+    assert_equal(RubyLsp::Constant::ErrorCodes::REQUEST_CANCELLED, error.code)
+    assert_equal("Request 1 was cancelled", error.message)
   end
 
   def test_unsaved_changes_are_indexed_when_computing_automatic_features