From ef79249b4e9c23b03f080b9a8453e129a97ea799 Mon Sep 17 00:00:00 2001 From: John Wilger Date: Sat, 17 Feb 2024 09:17:04 -0800 Subject: [PATCH] Return error tuple when operating on dead process Most of the functions of the GptAgent module expect to receive a pid of a running GptAgent process as their first argument. Prior to this change, if that pid did not represent a running process (e.g. the GptAgent process has been shut down for any reason), then the function call would result in an unhandled (by this library) exception. This changes the behavior such that we return an error tuple that the calling code can more easily handle. --- lib/gpt_agent.ex | 47 +++++++++++++++++++++-------- mix.exs | 2 +- test/gpt_agent_test.exs | 67 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 14 deletions(-) diff --git a/lib/gpt_agent.ex b/lib/gpt_agent.ex index 79c3915..166328e 100644 --- a/lib/gpt_agent.ex +++ b/lib/gpt_agent.ex @@ -45,12 +45,14 @@ defmodule GptAgent do @callback create_thread() :: {:ok, Types.thread_id()} @callback start_link(t()) :: Types.result(pid(), term()) @callback connect(connect_opts()) :: Types.result(pid(), :invalid_thread_id) - @callback shutdown(pid()) :: Types.success() - @callback add_user_message(pid(), Types.nonblank_string()) :: Types.result(:run_in_progress) + @callback shutdown(pid()) :: Types.result({:process_not_alive, pid()}) + @callback add_user_message(pid(), Types.nonblank_string()) :: + Types.result(:run_in_progress | {:process_not_alive, pid()}) @callback submit_tool_output(pid(), Types.tool_name(), Types.tool_output()) :: - Types.result(:invalid_tool_call_id) - @callback run_in_progress?(pid()) :: boolean() - @callback set_assistant_id(pid(), Types.assistant_id()) :: Types.success() + Types.result(:invalid_tool_call_id | {:process_not_alive, pid()}) + @callback run_in_progress?(pid()) :: boolean() | Types.error({:process_not_alive, pid()}) + @callback set_assistant_id(pid(), Types.assistant_id()) :: + Types.result({:process_not_alive, pid()}) defp noreply(%__MODULE__{} = state), do: {:noreply, state, state.timeout_ms} defp noreply(%__MODULE__{} = state, next), do: {:noreply, state, next} @@ -565,38 +567,57 @@ defmodule GptAgent do defp default_timeout_ms, do: Application.get_env(:gpt_agent, :timeout_ms, 120_000) + defp handle_dead_process(pid) do + log("GPT Agent with PID #{inspect(pid)} is not alive", :warning) + {:error, {:process_not_alive, pid}} + end + @impl true def shutdown(pid) do log("Shutting down GPT Agent with PID #{inspect(pid)}") if Process.alive?(pid) do log("GPT Agent with PID #{inspect(pid)} is alive, terminating") - :ok = DynamicSupervisor.terminate_child(GptAgent.Supervisor, pid) + DynamicSupervisor.terminate_child(GptAgent.Supervisor, pid) else - log("GPT Agent with PID #{inspect(pid)} is not alive") + handle_dead_process(pid) end - - :ok end @impl true def add_user_message(pid, message) do - GenServer.call(pid, {:add_user_message, %UserMessage{content: message}}) + if Process.alive?(pid) do + GenServer.call(pid, {:add_user_message, %UserMessage{content: message}}) + else + handle_dead_process(pid) + end end @impl true def submit_tool_output(pid, tool_call_id, tool_output) do - GenServer.call(pid, {:submit_tool_output, tool_call_id, tool_output}) + if Process.alive?(pid) do + GenServer.call(pid, {:submit_tool_output, tool_call_id, tool_output}) + else + handle_dead_process(pid) + end end @impl true def run_in_progress?(pid) do - GenServer.call(pid, :run_in_progress?) + if Process.alive?(pid) do + GenServer.call(pid, :run_in_progress?) + else + handle_dead_process(pid) + end end @impl true def set_assistant_id(pid, assistant_id) do - GenServer.cast(pid, {:set_assistant_id, assistant_id}) + if Process.alive?(pid) do + GenServer.cast(pid, {:set_assistant_id, assistant_id}) + else + handle_dead_process(pid) + end end end end diff --git a/mix.exs b/mix.exs index 3f11a99..39b24fc 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule GptAgent.MixProject do def project do [ app: :gpt_agent, - version: "7.0.1", + version: "8.0.0", elixir: "~> 1.16", start_permanent: Mix.env() == :prod, aliases: aliases(), diff --git a/test/gpt_agent_test.exs b/test/gpt_agent_test.exs index 316849d..5221a04 100644 --- a/test/gpt_agent_test.exs +++ b/test/gpt_agent_test.exs @@ -477,9 +477,36 @@ defmodule GptAgentTest do refute Process.alive?(pid) assert_eventually(Registry.lookup(GptAgent.Registry, thread_id) == []) end + + @tag capture_log: true + test "returns error if the process was not alive to begin with", %{ + thread_id: thread_id, + assistant_id: assistant_id + } do + {:ok, pid} = + GptAgent.connect(thread_id: thread_id, last_message_id: nil, assistant_id: assistant_id) + + :ok = GptAgent.shutdown(pid) + + assert {:error, {:process_not_alive, ^pid}} = GptAgent.shutdown(pid) + end end describe "add_user_message/2" do + @tag capture_log: true + test "returns error if the agent process is not alive", %{ + thread_id: thread_id, + assistant_id: assistant_id + } do + {:ok, pid} = + GptAgent.connect(thread_id: thread_id, last_message_id: nil, assistant_id: assistant_id) + + :ok = GptAgent.shutdown(pid) + + assert {:error, {:process_not_alive, ^pid}} = + GptAgent.add_user_message(pid, Faker.Lorem.sentence()) + end + test "adds the user message to the agent's thread via the OpenAI API", %{ bypass: bypass, thread_id: thread_id, @@ -1037,6 +1064,20 @@ defmodule GptAgentTest do end describe "submit_tool_output/3" do + @tag capture_log: true + test "returns error if the agent process is not alive", %{ + thread_id: thread_id, + assistant_id: assistant_id + } do + {:ok, pid} = + GptAgent.connect(thread_id: thread_id, last_message_id: nil, assistant_id: assistant_id) + + :ok = GptAgent.shutdown(pid) + + assert {:error, {:process_not_alive, ^pid}} = + GptAgent.submit_tool_output(pid, UUID.uuid4(), %{}) + end + test "returns {:error, :not_running} if there is no run in progress", %{ assistant_id: assistant_id, thread_id: thread_id @@ -1362,6 +1403,19 @@ defmodule GptAgentTest do end describe "run_in_progress?/1" do + @tag capture_log: true + test "returns error if the agent process is not alive", %{ + thread_id: thread_id, + assistant_id: assistant_id + } do + {:ok, pid} = + GptAgent.connect(thread_id: thread_id, last_message_id: nil, assistant_id: assistant_id) + + :ok = GptAgent.shutdown(pid) + + assert {:error, {:process_not_alive, ^pid}} = GptAgent.run_in_progress?(pid) + end + test "returns true if the agent has a run in progress", %{ assistant_id: assistant_id, thread_id: thread_id @@ -1388,6 +1442,19 @@ defmodule GptAgentTest do end describe "set_assistant_id/2" do + @tag capture_log: true + test "returns error if the agent process is not alive", %{ + thread_id: thread_id, + assistant_id: assistant_id + } do + {:ok, pid} = + GptAgent.connect(thread_id: thread_id, last_message_id: nil, assistant_id: assistant_id) + + :ok = GptAgent.shutdown(pid) + + assert {:error, {:process_not_alive, ^pid}} = GptAgent.set_assistant_id(pid, UUID.uuid4()) + end + test "updates the assistant_id in the agent's state", %{ assistant_id: assistant_id, thread_id: thread_id