fix: ignore requests until the relevant engine is started

Author: doorgan

apps/engine/benchmarks/ast_analyze.exs

@@ -1,3 +1,5 @@
+Mix.install([:benchee])
+
 alias Forge.Ast
 alias Forge.Document
 

apps/engine/benchmarks/enum_index.exs

@@ -1,3 +1,5 @@
+Mix.install([:benchee])
+
 alias Engine.Search.Indexer
 
 path =

apps/engine/benchmarks/ets_bench.exs

@@ -1,3 +1,5 @@
+Mix.install([:benchee])
+
 alias Forge.Project
 
 alias Engine.Search.Store.Backends.Ets

apps/engine/benchmarks/versions_bench.exs

@@ -1,3 +1,5 @@
+Mix.install([:benchee])
+
 alias Forge.VM.Versions
 
 Benchee.run(%{

apps/engine/mix.exs

@@ -44,7 +44,6 @@ defmodule Engine.MixProject do
 
   defp deps do
     [
-      {:benchee, "~> 1.3", only: :test},
       {:deps_nix, "~> 2.4", only: :dev},
       Mix.Credo.dependency(),
       Mix.Dialyzer.dependency(),

apps/expert/lib/expert.ex

@@ -1,4 +1,6 @@
 defmodule Expert do
+  alias Forge.Project
+  alias Expert.ActiveProjects
   alias Expert.Protocol.Convert
   alias Expert.Protocol.Id
   alias Expert.Provider.Handlers
@@ -94,42 +96,72 @@ defmodule Expert do
   def handle_request(request, lsp) do
     state = assigns(lsp).state
 
-    if state.engine_initialized? do
-      with {:ok, handler} <- fetch_handler(request),
-           {:ok, request} <- Convert.to_native(request),
-           {:ok, response} <- handler.handle(request, state.configuration),
-           {:ok, response} <- Expert.Protocol.Convert.to_lsp(response) do
-        {:reply, response, lsp}
-      else
-        {:error, {:unhandled, _}} ->
-          Logger.info("Unhandled request: #{request.method}")
-
-          {:reply,
-           %GenLSP.ErrorResponse{
-             code: GenLSP.Enumerations.ErrorCodes.method_not_found(),
-             message: "Method not found"
-           }, lsp}
-
-        error ->
-          message = "Failed to handle #{request.method}, #{inspect(error)}"
-          Logger.error(message)
-
-          {:reply,
-           %GenLSP.ErrorResponse{
-             code: GenLSP.Enumerations.ErrorCodes.internal_error(),
-             message: message
-           }, lsp}
-      end
+    with {:ok, handler} <- fetch_handler(request),
+         {:ok, request} <- Convert.to_native(request),
+         :ok <- check_engine_initialized(request),
+         {:ok, response} <- handler.handle(request, state.configuration),
+         {:ok, response} <- Expert.Protocol.Convert.to_lsp(response) do
+      {:reply, response, lsp}
     else
-      GenLSP.warning(
-        lsp,
-        "Received request #{request.method} before engine was initialized. Ignoring."
-      )
+      {:error, {:unhandled, _}} ->
+        Logger.info("Unhandled request: #{request.method}")
 
-      {:noreply, lsp}
+        {:reply,
+         %GenLSP.ErrorResponse{
+           code: GenLSP.Enumerations.ErrorCodes.method_not_found(),
+           message: "Method not found"
+         }, lsp}
+
+      {:error, :engine_not_initialized, project} ->
+        GenLSP.info(
+          lsp,
+          "Received request #{request.method} before engine for #{project && Project.name(project)} was initialized. Ignoring."
+        )
+
+        {:reply, nil, lsp}
+
+      error ->
+        message = "Failed to handle #{request.method}, #{inspect(error)}"
+        Logger.error(message)
+
+        {:reply,
+         %GenLSP.ErrorResponse{
+           code: GenLSP.Enumerations.ErrorCodes.internal_error(),
+           message: message
+         }, lsp}
     end
   end
 
+  defp check_engine_initialized(request) do
+    if document_request?(request) do
+      case Forge.Document.Container.context_document(request, nil) do
+        %Forge.Document{} = document ->
+          projects = ActiveProjects.projects()
+          project = Project.project_for_document(projects, document)
+
+          if ActiveProjects.active?(project) do
+            :ok
+          else
+            {:error, :engine_not_initialized, project}
+          end
+
+        nil ->
+          {:error, :engine_not_initialized, nil}
+      end
+    else
+      :ok
+    end
+  end
+
+  defp document_request?(%{document: %Forge.Document{}}), do: true
+
+  defp document_request?(%{params: params}) do
+    document_request?(params)
+  end
+
+  defp document_request?(%{text_document: %{uri: _}}), do: true
+  defp document_request?(_), do: false
+
   def handle_notification(%GenLSP.Notifications.Initialized{}, lsp) do
     Logger.info("Server initialized, registering capabilities")
     registrations = registrations()
@@ -175,18 +207,6 @@ defmodule Expert do
     end
   end
 
-  def handle_info(:engine_initialized, lsp) do
-    state = assigns(lsp).state
-
-    new_state = %{state | engine_initialized?: true}
-
-    lsp = assign(lsp, state: new_state)
-
-    Logger.info("Engine initialized")
-
-    {:noreply, lsp}
-  end
-
   def handle_info(:default_config, lsp) do
     state = assigns(lsp).state
 

apps/expert/lib/expert/active_projects.ex

@@ -5,6 +5,7 @@ defmodule Expert.ActiveProjects do
   Since GenLSP events happen asynchronously, we use an ets table to keep track of
   them and avoid race conditions when we try to update the list of active projects.
   """
+  alias Forge.Project
 
   use GenServer
 
@@ -21,6 +22,10 @@ defmodule Expert.ActiveProjects do
 
   def init(_) do
     __MODULE__ = :ets.new(__MODULE__, [:set, :named_table, :public, read_concurrency: true])
+
+    __MODULE__.Ready =
+      :ets.new(__MODULE__.Ready, [:set, :named_table, :public, read_concurrency: true])
+
     {:ok, nil}
   end
 
@@ -47,4 +52,19 @@ defmodule Expert.ActiveProjects do
     :ets.delete_all_objects(__MODULE__)
     add_projects(new_projects)
   end
+
+  def set_ready(%Project{} = project, ready?) when is_boolean(ready?) do
+    if ready? do
+      :ets.insert(__MODULE__.Ready, {project.root_uri, true})
+    else
+      :ets.delete(__MODULE__.Ready, project.root_uri)
+    end
+  end
+
+  def active?(%Project{} = project) do
+    case :ets.lookup(__MODULE__.Ready, project.root_uri) do
+      [{_, true}] -> true
+      _ -> false
+    end
+  end
 end

apps/expert/lib/expert/state.ex

@@ -18,7 +18,6 @@ defmodule Expert.State do
 
   defstruct configuration: nil,
             initialized?: false,
-            engine_initialized?: false,
             shutdown_received?: false,
             in_flight_requests: %{}
 
@@ -71,13 +70,11 @@ defmodule Expert.State do
 
     ActiveProjects.set_projects(projects)
 
-    Task.Supervisor.start_child(:expert_task_queue, fn ->
-      for project <- projects do
+    for project <- projects do
+      Task.Supervisor.start_child(:expert_task_queue, fn ->
         ensure_project_node_started(project)
-      end
-
-      send(Expert, :engine_initialized)
-    end)
+      end)
+    end
 
     {:ok, response, new_state}
   end
@@ -152,22 +149,31 @@ defmodule Expert.State do
     projects = ActiveProjects.projects()
     project = Project.project_for_uri(projects, uri)
 
-    case Document.Store.get_and_update(
-           uri,
-           # TODO: this function needs to accept the GenLSP data structure
-           &Document.apply_content_changes(&1, version, params.content_changes)
-         ) do
-      {:ok, updated_source} ->
-        updated_message =
-          file_changed(
-            uri: updated_source.uri,
-            open?: true,
-            from_version: version,
-            to_version: updated_source.version
-          )
-
-        EngineApi.broadcast(project, updated_message)
-        EngineApi.compile_document(project, updated_source)
+    with true <- ActiveProjects.active?(project),
+         {:ok, updated_source} <-
+           Document.Store.get_and_update(
+             uri,
+             # TODO: this function needs to accept the GenLSP data structure
+             &Document.apply_content_changes(&1, version, params.content_changes)
+           ) do
+      updated_message =
+        file_changed(
+          uri: updated_source.uri,
+          open?: true,
+          from_version: version,
+          to_version: updated_source.version
+        )
+
+      EngineApi.broadcast(project, updated_message)
+      EngineApi.compile_document(project, updated_source)
+      {:ok, state}
+    else
+      false ->
+        GenLSP.info(
+          Expert.get_lsp(),
+          "Received request textDocument/didChange before engine for #{Project.name(project)} was initialized. Ignoring."
+        )
+
         {:ok, state}
 
       error ->
@@ -269,6 +275,7 @@ defmodule Expert.State do
   defp ensure_project_node_started(project) do
     case Expert.Project.Supervisor.start(project) do
       {:ok, _pid} ->
+        ActiveProjects.set_ready(project, true)
         Logger.info("Project node started for #{Project.name(project)}")
 
         GenLSP.log(Expert.get_lsp(), "Started project node for #{Project.name(project)}")
@@ -281,9 +288,9 @@ defmodule Expert.State do
           "Failed to start project node for #{Project.name(project)}: #{inspect(reason, pretty: true)}"
         )
 
-        GenLSP.log(
+        GenLSP.error(
           Expert.get_lsp(),
-          "Failed to start project node for #{Project.name(project)}"
+          "Failed to start project node for #{Project.name(project)}: #{inspect(reason, pretty: true)}"
         )
 
         {:error, reason}
@@ -292,6 +299,7 @@ defmodule Expert.State do
 
   defp stop_project_node(project) do
     Expert.Project.Supervisor.stop(project)
+    ActiveProjects.set_ready(project, false)
 
     GenLSP.log(
       Expert.get_lsp(),