fix: don't infinitely loop prompting to fetch dependencies if it fails

apps/engine/lib/engine/build.ex

@@ -78,9 +78,9 @@ defmodule Engine.Build do
 
   @impl GenServer
   def handle_call({:clean_and_fetch_deps, %Project{} = project}, _from, %State{} = state) do
-    State.fetch_deps(state, project)
+    {state, result} = State.fetch_deps(state, project)
 
-    {:reply, :ok, state}
+    {:reply, result, state}
   end
 
   @impl GenServer

apps/engine/lib/engine/build/state.ex

@@ -88,11 +88,15 @@ defmodule Engine.Build.State do
         Logger.warning("Failed to remove build path #{path}: #{inspect(reason)}")
     end
 
-    Engine.Build.Project.fetch_deps(project)
+    result = Engine.Build.Project.fetch_deps(project)
 
-    state
+    {state, normalize_fetch_deps_result(result)}
   end
 
+  defp normalize_fetch_deps_result({:ok, :ok}), do: :ok
+  defp normalize_fetch_deps_result({:ok, result}), do: {:ok, result}
+  defp normalize_fetch_deps_result(result), do: result
+
   defp compile_project(%__MODULE__{} = state, initial?) do
     state = increment_build_number(state)
     project = state.project

apps/engine/test/engine/build/state_test.exs

@@ -149,4 +149,24 @@ defmodule Engine.Build.StateTest do
       assert_called(Build.Project.compile(_, _))
     end
   end
+
+  describe "fetching deps" do
+    test "returns :ok when deps fetch succeeds" do
+      {:ok, state} = with_project_state(:project_metadata)
+
+      patch(File, :rm_rf, fn _path -> {:ok, []} end)
+      patch(Build.Project, :fetch_deps, fn _project -> :ok end)
+
+      assert {^state, :ok} = State.fetch_deps(state, state.project)
+    end
+
+    test "returns error when deps fetch fails" do
+      {:ok, state} = with_project_state(:project_metadata)
+
+      patch(File, :rm_rf, fn _path -> {:ok, []} end)
+      patch(Build.Project, :fetch_deps, fn _project -> {:error, "deps failed"} end)
+
+      assert {^state, {:error, "deps failed"}} = State.fetch_deps(state, state.project)
+    end
+  end
 end

apps/expert/lib/expert.ex

@@ -24,7 +24,6 @@ defmodule Expert do
     GenLSP.Notifications.Exit,
     GenLSP.Requests.Shutdown
   ]
-
   @dialyzer {:nowarn_function, apply_to_state: 2}
 
   def vsn, do: :expert |> Application.spec(:vsn) |> to_string()
@@ -246,6 +245,9 @@ defmodule Expert do
   end
 
   def handle_info({:engine_initialized, project, {:ok, _pid}}, lsp) do
+    ActiveProjects.set_blocked(project, false)
+    ActiveProjects.set_ready(project, true)
+
     Logger.info(
       "Engine initialized for project #{Project.name(project)}",
       project: project
@@ -259,6 +261,7 @@ defmodule Expert do
   end
 
   def handle_info({:engine_initialized, project, {:error, {:shutdown, :deps_error}}}, lsp) do
+    ActiveProjects.set_blocked(project, false)
     ActiveProjects.set_ready(project, false)
 
     log_error(
@@ -271,6 +274,7 @@ defmodule Expert do
   end
 
   def handle_info({:engine_initialized, project, {:error, reason}}, lsp) do
+    ActiveProjects.set_blocked(project, false)
     ActiveProjects.set_ready(project, false)
 
     error_message = initialization_error_message(reason)
@@ -283,33 +287,16 @@ defmodule Expert do
     state = assigns(lsp).state
 
     supports_show_message = Expert.Configuration.client_support(:show_message)
+    blocked = ActiveProjects.blocked?(project)
 
     # Avoids spamming the user with the same prompt if they already declined
     deps_declined = State.deps_declined?(state, project)
 
-    if supports_show_message && not deps_declined do
-      project_name = Project.name(project)
-
+    if supports_show_message && not deps_declined && not blocked do
       ActiveProjects.set_blocked(project, true)
       ActiveProjects.set_ready(project, false)
 
-      response =
-        GenLSP.request(
-          lsp,
-          %Requests.WindowShowMessageRequest{
-            id: Id.next(),
-            params: %Structures.ShowMessageRequestParams{
-              type: Enumerations.MessageType.error(),
-              message:
-                "The Expert engine failed with errors on dependencies for project #{project_name}. Would you like to fetch them?",
-              actions: [
-                %Structures.MessageActionItem{title: "Yes"},
-                %Structures.MessageActionItem{title: "No"}
-              ]
-            }
-          },
-          :infinity
-        )
+      response = prompt_deps_fetch(lsp, project)
 
       handle_deps_fetch_result(response, lsp, project)
     else
@@ -332,59 +319,164 @@ defmodule Expert do
     end
   end
 
-  defp handle_deps_fetch_result(%Structures.MessageActionItem{title: "Yes"}, lsp, project) do
-    Logger.info("Running mix deps.get for #{Project.name(project)}", project: project)
+  defp handle_deps_fetch_result(
+         %Structures.MessageActionItem{title: "Yes"},
+         lsp,
+         project
+       ) do
+    start_deps_fetch_task(lsp, project)
+  end
 
-    Task.Supervisor.start_child(:expert_task_queue, fn ->
-      result =
-        try do
-          Expert.EngineApi.clean_and_fetch_deps(project)
-        rescue
-          error in ErlangError ->
-            {:error, error.original}
-        end
+  defp handle_deps_fetch_result(
+         %Structures.MessageActionItem{title: "No"},
+         lsp,
+         project
+       ) do
+    ActiveProjects.set_blocked(project, false)
+    ActiveProjects.set_ready(project, true)
+
+    Logger.info("User declined to run mix deps.get for #{Project.name(project)}",
+      project: project
+    )
+
+    state = assigns(lsp).state
+    new_state = State.mark_deps_declined(state, project)
+    assign(lsp, state: new_state)
+  end
+
+  defp handle_deps_fetch_result(_, lsp, project) do
+    ActiveProjects.set_blocked(project, false)
+    ActiveProjects.set_ready(project, false)
+
+    Logger.info("Dismissed dependency fetch prompt for #{Project.name(project)}",
+      project: project
+    )
+
+    lsp
+  end
 
-      case result do
-        :ok ->
-          Logger.info("mix deps.get completed successfully", project: project)
+  defp start_deps_fetch_task(lsp, project) do
+    case Task.Supervisor.start_child(:expert_task_queue, fn ->
+           run_deps_fetch_recovery(lsp, project)
+         end) do
+      {:ok, _pid} ->
+        lsp
 
-          Expert.Project.Supervisor.stop_node(project)
-          ActiveProjects.set_blocked(project, false)
+      {:error, reason} ->
+        ActiveProjects.set_blocked(project, false)
+        ActiveProjects.set_ready(project, false)
+
+        log_error(
+          lsp,
+          project,
+          "Failed to start dependency fetch task: #{inspect(reason)}"
+        )
 
-          Logger.info("Restarting engine for #{Project.name(project)}", project: project)
-          start_result = Expert.Project.Supervisor.ensure_node_started(project)
-          send(lsp.pid, {:engine_initialized, project, start_result})
+        lsp
+    end
+  end
 
-        {:error, msg} ->
-          log_error(lsp, project, "mix deps.get failed: #{inspect(msg)}")
+  defp run_deps_fetch_recovery(lsp, project) do
+    Logger.info("Running mix deps.get for #{Project.name(project)}", project: project)
 
-        error ->
-          log_error(
-            lsp,
-            project,
-            "Unexpected error from clean_and_fetch_deps: #{inspect(error)}"
-          )
+    result =
+      try do
+        Expert.EngineApi.clean_and_fetch_deps(project)
+      rescue
+        error in ErlangError ->
+          {:error, error.original}
       end
-    end)
 
-    lsp
+    case result do
+      :ok ->
+        Logger.info("mix deps.get completed successfully", project: project)
+
+        Expert.Project.Supervisor.stop_node(project)
+
+        Logger.info("Restarting engine for #{Project.name(project)}", project: project)
+        start_result = Expert.Project.Supervisor.ensure_node_started(project, blocked?: false)
+        send(lsp.pid, {:engine_initialized, project, start_result})
+
+      {:error, msg} ->
+        prompt_deps_fetch_retry(lsp, project, "mix deps.get failed: #{inspect(msg)}")
+
+      error ->
+        prompt_deps_fetch_retry(
+          lsp,
+          project,
+          "Unexpected error from clean_and_fetch_deps: #{inspect(error)}"
+        )
+    end
+  end
+
+  defp prompt_deps_fetch(lsp, project) do
+    project_name = Project.name(project)
+
+    prompt_with_actions(
+      lsp,
+      "The Expert engine failed with errors on dependencies for project #{project_name}. Would you like to fetch them?",
+      ["Yes", "No"]
+    )
+  end
+
+  defp prompt_deps_fetch_retry(lsp, project, message) do
+    Logger.error(message, project: project)
+
+    response =
+      prompt_with_actions(
+        lsp,
+        "Fetching dependencies for project #{Project.name(project)} failed. #{message}. Would you like to retry?",
+        ["Retry", "Cancel"]
+      )
+
+    handle_deps_fetch_retry_result(response, lsp, project)
   end
 
-  defp handle_deps_fetch_result(%Structures.MessageActionItem{title: "No"}, lsp, project) do
+  defp handle_deps_fetch_retry_result(
+         %Structures.MessageActionItem{title: "Retry"},
+         lsp,
+         project
+       ) do
+    run_deps_fetch_recovery(lsp, project)
+  end
+
+  defp handle_deps_fetch_retry_result(
+         %Structures.MessageActionItem{title: "Cancel"},
+         _lsp,
+         project
+       ) do
     ActiveProjects.set_blocked(project, false)
-    ActiveProjects.set_ready(project, true)
+    ActiveProjects.set_ready(project, false)
 
-    Logger.info("User declined to run mix deps.get for #{Project.name(project)}",
+    Logger.info("Cancelled dependency fetch retry for #{Project.name(project)}", project: project)
+
+    :ok
+  end
+
+  defp handle_deps_fetch_retry_result(_, _lsp, project) do
+    ActiveProjects.set_blocked(project, false)
+    ActiveProjects.set_ready(project, false)
+
+    Logger.info("Dismissed dependency fetch retry prompt for #{Project.name(project)}",
       project: project
     )
 
-    state = assigns(lsp).state
-    new_state = State.mark_deps_declined(state, project)
-    assign(lsp, state: new_state)
+    :ok
   end
 
-  defp handle_deps_fetch_result(_, lsp, _project) do
-    lsp
+  defp prompt_with_actions(lsp, message, actions) do
+    GenLSP.request(
+      lsp,
+      %Requests.WindowShowMessageRequest{
+        id: Id.next(),
+        params: %Structures.ShowMessageRequestParams{
+          type: Enumerations.MessageType.error(),
+          message: message,
+          actions: Enum.map(actions, &%Structures.MessageActionItem{title: &1})
+        }
+      },
+      :infinity
+    )
   end
 
   # When logging errors we also notify the client to display the message

apps/expert/lib/expert/project/supervisor.ex

@@ -45,7 +45,13 @@ defmodule Expert.Project.Supervisor do
   end
 
   def ensure_node_started(%Project{} = project) do
-    if ActiveProjects.blocked?(project) do
+    ensure_node_started(project, blocked?: true)
+  end
+
+  def ensure_node_started(%Project{} = project, opts) when is_list(opts) do
+    blocked? = Keyword.get(opts, :blocked?, true)
+
+    if blocked? and ActiveProjects.blocked?(project) do
       Logger.info("Project node start blocked for #{Project.name(project)}")
       {:error, :deps_error}
     else