feat: properly respond to and log initialization errors

Author: badosu

apps/expert/lib/expert.ex

@@ -53,20 +53,25 @@ defmodule Expert do
     state = assigns(lsp).state
     Process.send_after(self(), :default_config, :timer.seconds(5))
 
-    case State.initialize(state, request) do
-      {:ok, response, state} ->
-        lsp = assign(lsp, state: state)
-        {:ok, response} = Expert.Protocol.Convert.to_lsp(response)
-
-        {:reply, response, lsp}
+    with {:ok, response, state} <- State.initialize(state, request),
+         {:ok, response} <- Expert.Protocol.Convert.to_lsp(response) do
+      lsp = assign(lsp, state: state)
 
-      {:error, error} ->
-        response = %GenLSP.ErrorResponse{
-          code: GenLSP.Enumerations.ErrorCodes.invalid_request(),
-          message: to_string(error)
-        }
+      {:reply, response, lsp}
+    else
+      {:error, {:shutdown, {:failed_to_start_child, _, _}}} ->
+        {:reply,
+         %GenLSP.ErrorResponse{
+           code: GenLSP.Enumerations.ErrorCodes.server_not_initialized(),
+           message: "Failed to start node"
+         }, lsp}
 
-        {:reply, response, lsp}
+      {:error, reason} ->
+        {:reply,
+         %GenLSP.ErrorResponse{
+           code: GenLSP.Enumerations.ErrorCodes.server_not_initialized(),
+           message: to_string(reason)
+         }, lsp}
     end
   end
 

apps/expert/lib/expert/engine_node.ex

@@ -5,13 +5,16 @@ defmodule Expert.EngineNode do
   use Expert.Project.Progress.Support
 
   defmodule State do
+    require Logger
+
     defstruct [
       :project,
       :port,
       :cookie,
       :stopped_by,
       :stop_timeout,
       :started_by,
+      :last_message,
       :status
     ]
 
@@ -103,6 +106,19 @@ defmodule Expert.EngineNode do
       end
     end
 
+    def on_exit_status(%__MODULE__{} = state, exit_status) do
+      if exit_status > 0 do
+        GenLSP.error(
+          Expert.get_lsp(),
+          "Node exited with status #{exit_status}, last message: #{to_string(state.last_message)}"
+        )
+      else
+        Logger.debug("Node exited with status 0")
+      end
+
+      %{state | status: :stopped}
+    end
+
     def maybe_reply_to_stopper(%State{stopped_by: stopped_by} = state)
         when is_tuple(stopped_by) do
       GenServer.reply(state.stopped_by, :ok)
@@ -252,6 +268,10 @@ defmodule Expert.EngineNode do
           Logger.debug("Building engine: #{to_string(data)}")
           wait_for_engine(port, data)
 
+        {^port, {:exit_status, status}} ->
+          Logger.debug("Engine build script exited with status: #{to_string(status)}")
+          {:error, {:exit_status, status}, last_line}
+
         {:EXIT, ^port, reason} ->
           Logger.error("Engine build script exited with reason: #{inspect(reason)} #{last_line}")
           {:error, reason, last_line}
@@ -372,10 +392,17 @@ defmodule Expert.EngineNode do
     {:stop, :shutdown, state}
   end
 
+  @impl true
+  def handle_info({_port, {:exit_status, exit_status}}, %State{} = state) do
+    state = State.on_exit_status(state, exit_status)
+
+    {:stop, :shutdown, state}
+  end
+
   @impl true
   def handle_info({_port, {:data, message}}, %State{} = state) do
     Logger.debug("Node port message: #{to_string(message)}")
-    {:noreply, state}
+    {:noreply, %{state | last_message: message}}
   end
 
   @impl true

apps/expert/lib/expert/port.ex

@@ -112,7 +112,7 @@ defmodule Expert.Port do
         opts
       end
 
-    Port.open({:spawn_executable, launcher}, [:stderr_to_stdout | opts])
+    Port.open({:spawn_executable, launcher}, [:stderr_to_stdout, :exit_status] ++ opts)
   end
 
   @doc """

apps/expert/lib/expert/state.ex

@@ -50,17 +50,15 @@ defmodule Expert.State do
       end
 
     config = Configuration.new(event.root_uri, event.capabilities, client_name)
-    new_state = %__MODULE__{state | configuration: config, initialized?: true}
     Logger.info("Starting project at uri #{config.project.root_uri}")
 
-    response = initialize_result()
-
-    Task.Supervisor.start_child(:expert_task_queue, fn ->
-      {:ok, _pid} = Project.Supervisor.start(config.project)
+    with {:ok, _pid} <- Project.Supervisor.start(config.project) do
       send(Expert, :engine_initialized)
-    end)
+      new_state = %__MODULE__{state | configuration: config, initialized?: true}
+      response = initialize_result()
 
-    {:ok, response, new_state}
+      {:ok, response, new_state}
+    end
   end
 
   def initialize(%__MODULE__{initialized?: true}, %Requests.Initialize{}) do

apps/expert/test/expert/engine_node_test.exs

@@ -4,6 +4,7 @@ defmodule Expert.EngineNodeTest do
 
   import Forge.Test.EventualAssertions
   import Forge.Test.Fixtures
+  import Expert.Test.Protocol.TransportSupport
 
   use ExUnit.Case, async: false
   use Patch
@@ -64,4 +65,34 @@ defmodule Expert.EngineNodeTest do
     assert_receive {:stopped, 1}
     assert_receive {:lsp_log, "Couldn't find an elixir executable for project" <> _}
   end
+
+  test "sends a message to client if exited with error code", %{project: project} do
+    with_patched_transport()
+
+    {:ok, _node_name, node_pid} = EngineNode.start(project)
+
+    project_alive? = project |> EngineNode.name() |> Process.whereis() |> Process.alive?()
+    assert project_alive?
+
+    exit_status = 127
+
+    send(node_pid, {nil, {:exit_status, exit_status}})
+
+    error_message_type = GenLSP.Enumerations.MessageType.error()
+
+    assert_receive {:transport,
+                    %GenLSP.Notifications.WindowLogMessage{
+                      params: %GenLSP.Structures.LogMessageParams{
+                        type: ^error_message_type,
+                        message: message
+                      }
+                    }}
+
+    assert String.starts_with?(
+             message,
+             "Node exited with status #{exit_status}, last message:"
+           )
+
+    assert_eventually Process.whereis(EngineNode.name(project)) == nil, :timer.seconds(5)
+  end
 end

apps/expert/test/expert/project/diagnostics_test.exs

@@ -13,6 +13,7 @@ defmodule Expert.Project.DiagnosticsTest do
 
   import Forge.EngineApi.Messages
   import Forge.Test.Fixtures
+  import Expert.Test.Protocol.TransportSupport
 
   setup do
     project = project()
@@ -37,24 +38,6 @@ defmodule Expert.Project.DiagnosticsTest do
     struct(Diagnostic.Result, values)
   end
 
-  def with_patched_transport(_) do
-    test = self()
-
-    patch(GenLSP, :notify_server, fn _, message ->
-      send(test, {:transport, message})
-    end)
-
-    patch(GenLSP, :notify, fn _, message ->
-      send(test, {:transport, message})
-    end)
-
-    patch(GenLSP, :request, fn _, message ->
-      send(test, {:transport, message})
-    end)
-
-    :ok
-  end
-
   defp open_file(project, contents) do
     uri = file_uri(project, "lib/project.ex")
     :ok = Document.Store.open(uri, contents, 0)

apps/expert/test/expert/project/progress_test.exs

@@ -9,6 +9,7 @@ defmodule Expert.Project.ProgressTest do
 
   import Forge.Test.Fixtures
   import Forge.EngineApi.Messages
+  import Expert.Test.Protocol.TransportSupport
 
   use ExUnit.Case
   use Patch
@@ -44,20 +45,6 @@ defmodule Expert.Project.ProgressTest do
     project_progress(label: label, message: message, stage: stage)
   end
 
-  def with_patched_transport(_) do
-    test = self()
-
-    patch(GenLSP, :notify, fn _, message ->
-      send(test, {:transport, message})
-    end)
-
-    patch(GenLSP, :request, fn _, message ->
-      send(test, {:transport, message})
-    end)
-
-    :ok
-  end
-
   def with_work_done_progress_support(_) do
     patch(Configuration, :client_supports?, fn :work_done_progress -> true end)
     :ok

apps/expert/test/expert_test.exs

@@ -0,0 +1,35 @@
+defmodule Expert.ExpertTest do
+  use ExUnit.Case, async: true
+  use Patch
+
+  describe "handle_request/2" do
+    test "it replies with server_not_initialized on initialization error" do
+      reason = :something_bad
+
+      patch(Expert.State, :initialize, fn _state, _request ->
+        {:error, reason}
+      end)
+
+      assigns = start_supervised!(GenLSP.Assigns, id: make_ref())
+      GenLSP.Assigns.merge(assigns, %{state: %{}})
+
+      lsp = %GenLSP.LSP{
+        mod: Elixir,
+        assigns: assigns
+      }
+
+      initialize_request = %GenLSP.Requests.Initialize{
+        id: 1,
+        jsonrpc: "2.0",
+        method: "initialize"
+      }
+
+      {:reply, response, _lsp} = Expert.handle_request(initialize_request, lsp)
+
+      message = to_string(reason)
+      error_code = GenLSP.Enumerations.ErrorCodes.server_not_initialized()
+
+      assert %GenLSP.ErrorResponse{code: ^error_code, data: nil, message: ^message} = response
+    end
+  end
+end

apps/expert/test/support/test/transport_support.ex

@@ -0,0 +1,19 @@
+defmodule Expert.Test.Protocol.TransportSupport do
+  def with_patched_transport(_ \\ nil) do
+    test = self()
+
+    Patch.patch(GenLSP, :notify_server, fn _, message ->
+      send(test, {:transport, message})
+    end)
+
+    Patch.patch(GenLSP, :notify, fn _, message ->
+      send(test, {:transport, message})
+    end)
+
+    Patch.patch(GenLSP, :request, fn _, message ->
+      send(test, {:transport, message})
+    end)
+
+    :ok
+  end
+end