From 57dc835a224b4b9ee767c04c3809854260172e35 Mon Sep 17 00:00:00 2001
From: Vinicius Stock <vinicius.stock@shopify.com>
Date: Fri, 29 Nov 2024 14:01:50 -0500
Subject: [PATCH] Fail requests that are searching for a non existing position

---
 lib/ruby_lsp/document.rb   | 11 ++++++++++-
 lib/ruby_lsp/server.rb     | 14 +++++++++++++-
 test/ruby_document_test.rb | 23 +++++++++++++++++++++++
 test/server_test.rb        | 30 ++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/lib/ruby_lsp/document.rb b/lib/ruby_lsp/document.rb
index 503f8eeb8..62b35b516 100644
--- a/lib/ruby_lsp/document.rb
+++ b/lib/ruby_lsp/document.rb
@@ -15,6 +15,7 @@ class LanguageId < T::Enum
     extend T::Helpers
     extend T::Generic
 
+    class LocationNotFoundError < StandardError; end
     ParseResultType = type_member
 
     # This maximum number of characters for providing expensive features, like semantic highlighting and diagnostics.
@@ -144,7 +145,15 @@ def initialize(source, encoding)
       def find_char_position(position)
         # Find the character index for the beginning of the requested line
         until @current_line == position[:line]
-          @pos += 1 until LINE_BREAK == @source[@pos]
+          until LINE_BREAK == @source[@pos]
+            @pos += 1
+
+            if @pos >= @source.length
+              # Pack the code points back into the original string to provide context in the error message
+              raise LocationNotFoundError, "Requested position: #{position}\nSource:\n\n#{@source.pack("U*")}"
+            end
+          end
+
           @pos += 1
           @current_line += 1
         end
diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb
index eb488cf4d..b7b1d5770 100644
--- a/lib/ruby_lsp/server.rb
+++ b/lib/ruby_lsp/server.rb
@@ -119,12 +119,24 @@ def process_message(message)
         # If a document is deleted before we are able to process all of its enqueued requests, we will try to read it
         # from disk and it raise this error. This is expected, so we don't include the `data` attribute to avoid
         # reporting these to our telemetry
-        if e.is_a?(Store::NonExistingDocumentError)
+        case e
+        when Store::NonExistingDocumentError
           send_message(Error.new(
             id: message[:id],
             code: Constant::ErrorCodes::INVALID_PARAMS,
             message: e.full_message,
           ))
+        when Document::LocationNotFoundError
+          send_message(Error.new(
+            id: message[:id],
+            code: Constant::ErrorCodes::REQUEST_FAILED,
+            message: <<~MESSAGE,
+              Request #{message[:method]} failed to find the target position.
+              The file might have been modified while the server was in the middle of searching for the target.
+              If you experience this regularly, please report any findings and extra information on
+              https://github.com/Shopify/ruby-lsp/issues/2446
+            MESSAGE
+          ))
         else
           send_message(Error.new(
             id: message[:id],
diff --git a/test/ruby_document_test.rb b/test/ruby_document_test.rb
index ecc3dbd25..edbb1e910 100644
--- a/test/ruby_document_test.rb
+++ b/test/ruby_document_test.rb
@@ -770,6 +770,29 @@ class Foo
     assert_nil(document.cache_get("textDocument/codeLens"))
   end
 
+  def test_locating_a_non_existing_location_raises
+    document = RubyLsp::RubyDocument.new(source: <<~RUBY.chomp, version: 1, uri: URI("file:///foo/bar.rb"))
+      class Foo
+      end
+    RUBY
+
+    # Exactly at the last character doesn't raise
+    document.locate_node({ line: 1, character: 2 })
+
+    # Anything beyond does
+    error = assert_raises(RubyLsp::Document::LocationNotFoundError) do
+      document.locate_node({ line: 3, character: 2 })
+    end
+
+    assert_equal(<<~MESSAGE.chomp, error.message)
+      Requested position: {:line=>3, :character=>2}
+      Source:
+
+      class Foo
+      end
+    MESSAGE
+  end
+
   private
 
   def assert_error_edit(actual, error_range)
diff --git a/test/server_test.rb b/test/server_test.rb
index 8cc87ce37..66f77c198 100644
--- a/test/server_test.rb
+++ b/test/server_test.rb
@@ -714,6 +714,36 @@ def handle_window_show_message_response(title)
     end
   end
 
+  def test_requests_to_a_non_existing_position_return_error
+    uri = URI("file:///foo.rb")
+
+    @server.process_message({
+      method: "textDocument/didOpen",
+      params: {
+        textDocument: {
+          uri: uri,
+          text: "class Foo\nend",
+          version: 1,
+          languageId: "ruby",
+        },
+      },
+    })
+
+    @server.process_message({
+      id: 1,
+      method: "textDocument/completion",
+      params: {
+        textDocument: {
+          uri: uri,
+        },
+        position: { line: 10, character: 0 },
+      },
+    })
+
+    error = find_message(RubyLsp::Error)
+    assert_match("Request textDocument/completion failed to find the target position.", error.message)
+  end
+
   private
 
   def with_uninstalled_rubocop(&block)