diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb index 33bdafb005..c7f9f5ad47 100644 --- a/lib/ruby_lsp/server.rb +++ b/lib/ruby_lsp/server.rb @@ -98,16 +98,27 @@ def process_message(message) rescue StandardError, LoadError => e # If an error occurred in a request, we have to return an error response or else the editor will hang if message[:id] - send_message(Error.new( - id: message[:id], - code: Constant::ErrorCodes::INTERNAL_ERROR, - message: e.full_message, - data: { - errorClass: e.class.name, - errorMessage: e.message, - backtrace: e.backtrace&.join("\n"), - }, - )) + # 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) + send_message(Error.new( + id: message[:id], + code: Constant::ErrorCodes::INVALID_PARAMS, + message: e.full_message, + )) + else + send_message(Error.new( + id: message[:id], + code: Constant::ErrorCodes::INTERNAL_ERROR, + message: e.full_message, + data: { + errorClass: e.class.name, + errorMessage: e.message, + backtrace: e.backtrace&.join("\n"), + }, + )) + end end $stderr.puts("Error processing #{message[:method]}: #{e.full_message}") diff --git a/lib/ruby_lsp/store.rb b/lib/ruby_lsp/store.rb index 8b84565629..68dea8f301 100644 --- a/lib/ruby_lsp/store.rb +++ b/lib/ruby_lsp/store.rb @@ -5,6 +5,8 @@ module RubyLsp class Store extend T::Sig + class NonExistingDocumentError < StandardError; end + sig { returns(T::Boolean) } attr_accessor :supports_progress @@ -45,6 +47,8 @@ def get(uri) end set(uri: uri, source: File.binread(path), version: 0, language_id: language_id) T.must(@state[uri.to_s]) + rescue Errno::ENOENT + raise NonExistingDocumentError, uri.to_s end sig do diff --git a/test/server_test.rb b/test/server_test.rb index e3b4fb189e..b239b3a1f7 100644 --- a/test/server_test.rb +++ b/test/server_test.rb @@ -462,6 +462,65 @@ def test_errors_include_telemetry_data assert_match("mocha/exception_raiser.rb", data[:backtrace]) end + def test_closing_document_before_computing_features_does_not_error + uri = URI("file:///foo.rb") + + capture_subprocess_io do + @server.process_message({ + method: "textDocument/didOpen", + params: { + textDocument: { + uri: uri, + text: "class Foo\nend", + version: 1, + languageId: "ruby", + }, + }, + }) + + # Close the file in a thread to increase the chance that it gets closed during the processing of the 10 document + # symbol requests below + thread = Thread.new do + @server.process_message({ + method: "textDocument/didClose", + params: { + textDocument: { + uri: uri, + }, + }, + }) + end + + 10.times do |i| + @server.process_message({ + id: i, + method: "textDocument/documentSymbol", + params: { + textDocument: { + uri: uri, + }, + }, + }) + end + + thread.join + end + + # Even if the thread technique, this test is not 100% reliable since it's trying to emulate a concurrency issue. If + # we tried to always expect an error back, we would likely get infinite loops + error = T.let(nil, T.nilable(T.any(RubyLsp::Error, RubyLsp::Message))) + + 10.times do + error = @server.pop_response + break if error.is_a?(RubyLsp::Error) + end + + if error.is_a?(RubyLsp::Error) + assert_instance_of(RubyLsp::Error, error) + assert_match("file:///foo.rb (RubyLsp::Store::NonExistingDocumentError)", error.message) + end + end + private def with_uninstalled_rubocop(&block)