Fallback to NullClient if initializing server fails (#266)

* Fallback to `NullClient` if initializing server fails

* Fail gracefully on server initialization error

Author: vinistock

lib/ruby_lsp/ruby_lsp_rails/addon.rb

@@ -14,11 +14,15 @@ class Addon < ::RubyLsp::Addon
 
       sig { returns(RunnerClient) }
       def client
-        @client ||= T.let(RunnerClient.new, T.nilable(RunnerClient))
+        @client ||= T.let(RunnerClient.create_client, T.nilable(RunnerClient))
       end
 
       sig { override.params(message_queue: Thread::Queue).void }
-      def activate(message_queue); end
+      def activate(message_queue)
+        # Eagerly initialize the client in a thread. This allows the indexing from the Ruby LSP to continue running even
+        # while we boot large Rails applications in the background
+        Thread.new { client }
+      end
 
       sig { override.void }
       def deactivate

lib/ruby_lsp/ruby_lsp_rails/runner_client.rb

@@ -4,17 +4,38 @@
 require "json"
 require "open3"
 
-# NOTE: We should avoid printing to stderr since it causes problems. We never read the standard error pipe
-# from the client, so it will become full and eventually hang or crash.
-# Instead, return a response with an `error` key.
-
 module RubyLsp
   module Rails
     class RunnerClient
+      class << self
+        extend T::Sig
+
+        sig { returns(RunnerClient) }
+        def create_client
+          new
+        rescue Errno::ENOENT, StandardError => e # rubocop:disable Lint/ShadowedException
+          warn("Ruby LSP Rails failed to initialize server: #{e.message}\n#{e.backtrace&.join("\n")}")
+          warn("Server dependent features will not be available")
+          NullClient.new
+        end
+      end
+
+      class InitializationError < StandardError; end
+      class IncompleteMessageError < StandardError; end
+
       extend T::Sig
 
       sig { void }
       def initialize
+        # Spring needs a Process session ID. It uses this ID to "attach" itself to the parent process, so that when the
+        # parent ends, the spring process ends as well. If this is not set, Spring will throw an error while trying to
+        # set its own session ID
+        begin
+          Process.setsid
+        rescue Errno::EPERM
+          # If we can't set the session ID, continue
+        end
+
         stdin, stdout, stderr, wait_thread = Open3.popen3(
           "bin/rails",
           "runner",
@@ -27,11 +48,20 @@ def initialize
         @wait_thread = T.let(wait_thread, Process::Waiter)
         @stdin.binmode # for Windows compatibility
         @stdout.binmode # for Windows compatibility
+
+        warn("Ruby LSP Rails booting server")
+        read_response
+        warn("Finished booting Ruby LSP Rails server")
+      rescue Errno::EPIPE, IncompleteMessageError
+        raise InitializationError, @stderr.read
       end
 
       sig { params(name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
       def model(name)
         make_request("model", name: name)
+      rescue IncompleteMessageError
+        warn("Ruby LSP Rails failed to get model information: #{@stderr.read}")
+        nil
       end
 
       sig { void }
@@ -48,13 +78,18 @@ def stopped?
 
       private
 
-      sig { params(request: T.untyped, params: T.untyped).returns(T.untyped) }
+      sig do
+        params(
+          request: String,
+          params: T.nilable(T::Hash[Symbol, T.untyped]),
+        ).returns(T.nilable(T::Hash[Symbol, T.untyped]))
+      end
       def make_request(request, params = nil)
         send_message(request, params)
         read_response
       end
 
-      sig { params(request: T.untyped, params: T.untyped).void }
+      sig { params(request: String, params: T.nilable(T::Hash[Symbol, T.untyped])).void }
       def send_message(request, params = nil)
         message = { method: request }
         message[:params] = params if params
@@ -68,8 +103,9 @@ def send_message(request, params = nil)
       sig { returns(T.nilable(T::Hash[Symbol, T.untyped])) }
       def read_response
         headers = @stdout.gets("\r\n\r\n")
-        raw_response = @stdout.read(T.must(headers)[/Content-Length: (\d+)/i, 1].to_i)
+        raise IncompleteMessageError unless headers
 
+        raw_response = @stdout.read(headers[/Content-Length: (\d+)/i, 1].to_i)
         response = JSON.parse(T.must(raw_response), symbolize_names: true)
 
         if response[:error]
@@ -80,5 +116,35 @@ def read_response
         response.fetch(:result)
       end
     end
+
+    class NullClient < RunnerClient
+      extend T::Sig
+
+      sig { void }
+      def initialize # rubocop:disable Lint/MissingSuper
+      end
+
+      sig { override.void }
+      def shutdown
+        # no-op
+      end
+
+      sig { override.returns(T::Boolean) }
+      def stopped?
+        true
+      end
+
+      private
+
+      sig { override.params(request: String, params: T.nilable(T::Hash[Symbol, T.untyped])).void }
+      def send_message(request, params = nil)
+        # no-op
+      end
+
+      sig { override.returns(T.nilable(T::Hash[Symbol, T.untyped])) }
+      def read_response
+        # no-op
+      end
+    end
   end
 end

lib/ruby_lsp/ruby_lsp_rails/server.rb

@@ -19,13 +19,63 @@
   nil
 end
 
+# NOTE: We should avoid printing to stderr since it causes problems. We never read the standard error pipe from the
+# client, so it will become full and eventually hang or crash. Instead, return a response with an `error` key.
+
 module RubyLsp
   module Rails
     class Server
       VOID = Object.new
 
       extend T::Sig
 
+      sig { void }
+      def initialize
+        $stdin.sync = true
+        $stdout.sync = true
+        @running = T.let(true, T::Boolean)
+      end
+
+      sig { void }
+      def start
+        initialize_result = { result: { message: "ok" } }.to_json
+        $stdout.write("Content-Length: #{initialize_result.length}\r\n\r\n#{initialize_result}")
+
+        while @running
+          headers = $stdin.gets("\r\n\r\n")
+          json = $stdin.read(headers[/Content-Length: (\d+)/i, 1].to_i)
+
+          request = JSON.parse(json, symbolize_names: true)
+          response = execute(request.fetch(:method), request[:params])
+          next if response == VOID
+
+          json_response = response.to_json
+          $stdout.write("Content-Length: #{json_response.length}\r\n\r\n#{json_response}")
+        end
+      end
+
+      sig do
+        params(
+          request: String,
+          params: T::Hash[Symbol, T.untyped],
+        ).returns(T.any(Object, T::Hash[Symbol, T.untyped]))
+      end
+      def execute(request, params = {})
+        case request
+        when "shutdown"
+          @running = false
+          VOID
+        when "model"
+          resolve_database_info_from_model(params.fetch(:name))
+        else
+          VOID
+        end
+      rescue => e
+        { error: e.full_message(highlight: false) }
+      end
+
+      private
+
       sig { params(model_name: String).returns(T::Hash[Symbol, T.untyped]) }
       def resolve_database_info_from_model(model_name)
         const = ActiveSupport::Inflector.safe_constantize(model_name)
@@ -48,41 +98,7 @@ def resolve_database_info_from_model(model_name)
         end
         info
       rescue => e
-        {
-          error: e.message,
-        }
-      end
-
-      sig { void }
-      def start
-        $stdin.sync = true
-        $stdout.sync = true
-
-        running = T.let(true, T::Boolean)
-
-        while running
-          headers = $stdin.gets("\r\n\r\n")
-          request = $stdin.read(headers[/Content-Length: (\d+)/i, 1].to_i)
-
-          json = JSON.parse(request, symbolize_names: true)
-          request_method = json.fetch(:method)
-          params = json[:params]
-
-          response = case request_method
-          when "shutdown"
-            running = false
-            VOID
-          when "model"
-            resolve_database_info_from_model(params.fetch(:name))
-          else
-            VOID
-          end
-
-          next if response == VOID
-
-          json_response = response.to_json
-          $stdout.write("Content-Length: #{json_response.length}\r\n\r\n#{json_response}")
-        end
+        { error: e.full_message(highlight: false) }
       end
     end
   end

test/ruby_lsp_rails/hover_test.rb

@@ -194,18 +194,22 @@ def hover_on_source(source, position)
         executor.instance_variable_get(:@index).index_single(
           RubyIndexer::IndexablePath.new(nil, T.must(uri.to_standardized_path)), source
         )
-        response = executor.execute(
-          {
-            method: "textDocument/hover",
-            params: {
-              textDocument: { uri: uri },
-              position: position,
+
+        response = T.let(nil, T.nilable(RubyLsp::Result))
+        capture_io do
+          response = executor.execute(
+            {
+              method: "textDocument/hover",
+              params: {
+                textDocument: { uri: uri },
+                position: position,
+              },
             },
-          },
-        )
+          )
+        end
 
-        assert_nil(response.error)
-        response.response
+        assert_nil(T.must(response).error)
+        T.must(response).response
       end
 
       def dummy_root

test/ruby_lsp_rails/runner_client_test.rb

@@ -8,7 +8,9 @@ module RubyLsp
   module Rails
     class RunnerClientTest < ActiveSupport::TestCase
       setup do
-        @client = T.let(RunnerClient.new, RunnerClient)
+        capture_io do
+          @client = T.let(RunnerClient.new, RunnerClient)
+        end
       end
 
       teardown do
@@ -36,6 +38,20 @@ class RunnerClientTest < ActiveSupport::TestCase
       test "returns nil if the request returns a nil response" do
         assert_nil @client.model("ApplicationRecord") # ApplicationRecord is abstract
       end
+
+      test "failing to spawn server creates a null client" do
+        FileUtils.mv("bin/rails", "bin/rails_backup")
+
+        assert_output("", %r{No such file or directory - bin/rails}) do
+          client = RunnerClient.create_client
+
+          assert_instance_of(NullClient, client)
+          assert_nil(client.model("User"))
+          assert_predicate(client, :stopped?)
+        end
+      ensure
+        FileUtils.mv("bin/rails_backup", "bin/rails")
+      end
     end
   end
 end

test/ruby_lsp_rails/server_test.rb

@@ -10,24 +10,24 @@ class ServerTest < ActiveSupport::TestCase
   end
 
   test "returns nil if model doesn't exist" do
-    response = @server.resolve_database_info_from_model("Foo")
+    response = @server.execute("model", { name: "Foo" })
     assert_nil(response.fetch(:result))
   end
 
   test "returns nil if class is not a model" do
-    response = @server.resolve_database_info_from_model("Time")
+    response = @server.execute("model", { name: "Time" })
     assert_nil(response.fetch(:result))
   end
 
   test "returns nil if class is an abstract model" do
-    response = @server.resolve_database_info_from_model("ApplicationRecord")
+    response = @server.execute("model", { name: "ApplicationRecord" })
     assert_nil(response.fetch(:result))
   end
 
   test "handles older Rails version which don't have `schema_dump_path`" do
     ActiveRecord::Tasks::DatabaseTasks.send(:alias_method, :old_schema_dump_path, :schema_dump_path)
     ActiveRecord::Tasks::DatabaseTasks.undef_method(:schema_dump_path)
-    response = @server.resolve_database_info_from_model("User")
+    response = @server.execute("model", { name: "User" })
     assert_nil(response.fetch(:result)[:schema_file])
   ensure
     ActiveRecord::Tasks::DatabaseTasks.send(:alias_method, :schema_dump_path, :old_schema_dump_path)