Skip to content

Commit

Permalink
Return error tuple when operating on dead process
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jwilger committed Feb 17, 2024
1 parent d7ea9b7 commit ef79249
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 14 deletions.
47 changes: 34 additions & 13 deletions lib/gpt_agent.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
67 changes: 67 additions & 0 deletions test/gpt_agent_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit ef79249

Please sign in to comment.