diff --git a/lib/aprsme/shutdown_handler.ex b/lib/aprsme/shutdown_handler.ex index 86fba09..ac8d87f 100644 --- a/lib/aprsme/shutdown_handler.ex +++ b/lib/aprsme/shutdown_handler.ex @@ -51,50 +51,44 @@ defmodule Aprsme.ShutdownHandler do # Server callbacks + def handle_call(:shutdown, _from, %{shutting_down: true} = state) do + {:reply, :already_shutting_down, state} + end + def handle_call(:shutdown, _from, state) do - if state.shutting_down do - {:reply, :already_shutting_down, state} - else - Logger.info("Initiating graceful shutdown...") - new_state = initiate_shutdown(state) - {:reply, :ok, new_state} - end + Logger.info("Initiating graceful shutdown...") + {:reply, :ok, initiate_shutdown(state)} end def handle_call(:shutting_down?, _from, state) do {:reply, state.shutting_down, state} end - # Handle terminate callback from OTP when SIGTERM is received + # Handle terminate callback from OTP when SIGTERM is received. def terminate(reason, state) do Logger.info("ShutdownHandler terminating: #{inspect(reason)}") - - # Only initiate shutdown for specific reasons, not during normal shutdown - should_graceful_shutdown = - case reason do - # Normal shutdown, don't interfere - :shutdown -> false - # Normal shutdown with reason - {:shutdown, _} -> false - # Normal termination - :normal -> false - # Abnormal termination, do graceful shutdown - _ -> true - end - - if should_graceful_shutdown and not state.shutting_down do - # Mark unhealthy so load balancer stops sending traffic, then - # block for drain_timeout to let existing connections finish. - # Note: Process.send_after is pointless here since no messages - # are processed during terminate — we just sleep directly. - mark_unhealthy() - stop_accepting_connections() - Process.sleep(state.drain_timeout) - end - + maybe_graceful_drain(graceful_shutdown_needed?(reason), state) :ok end + # Normal exits don't need the drain-and-sleep dance. + defp graceful_shutdown_needed?(:shutdown), do: false + defp graceful_shutdown_needed?({:shutdown, _reason}), do: false + defp graceful_shutdown_needed?(:normal), do: false + defp graceful_shutdown_needed?(_abnormal), do: true + + # Mark unhealthy so load balancers stop sending traffic, then block for + # drain_timeout to let existing connections finish. Process.send_after + # is pointless inside terminate/2 — no messages are processed during + # termination, so we sleep directly. + defp maybe_graceful_drain(true, %{shutting_down: false} = state) do + mark_unhealthy() + stop_accepting_connections() + Process.sleep(state.drain_timeout) + end + + defp maybe_graceful_drain(_, _state), do: :ok + def handle_info({:EXIT, _pid, reason}, state) do Logger.debug("Received EXIT signal: #{inspect(reason)}") {:noreply, state} diff --git a/test/aprsme/shutdown_handler_test.exs b/test/aprsme/shutdown_handler_test.exs new file mode 100644 index 0000000..41647e6 --- /dev/null +++ b/test/aprsme/shutdown_handler_test.exs @@ -0,0 +1,50 @@ +defmodule Aprsme.ShutdownHandlerTest do + # async: false — shares app env and the named ShutdownHandler process. + use ExUnit.Case, async: false + + alias Aprsme.ShutdownHandler + + describe "handle_call(:shutting_down?, _, state)" do + test "reflects the shutting_down flag in state" do + # Pure callback invocation — no side effects. + assert {:reply, false, %{shutting_down: false}} = + ShutdownHandler.handle_call(:shutting_down?, self(), %{ + shutting_down: false, + drain_timeout: 0 + }) + + assert {:reply, true, %{shutting_down: true}} = + ShutdownHandler.handle_call(:shutting_down?, self(), %{ + shutting_down: true, + drain_timeout: 0 + }) + end + end + + describe "handle_call(:shutdown, _, state)" do + test "returns :already_shutting_down when already in progress" do + state = %{shutting_down: true, drain_timeout: 0} + + assert {:reply, :already_shutting_down, ^state} = + ShutdownHandler.handle_call(:shutdown, self(), state) + end + end + + describe "handle_info({:EXIT, pid, reason}, state)" do + test "is a no-op that keeps state intact" do + state = %{shutting_down: false, drain_timeout: 0} + + assert {:noreply, ^state} = + ShutdownHandler.handle_info({:EXIT, self(), :normal}, state) + end + end + + describe "shutting_down?/0 without the GenServer running" do + test "returns false via the rescue clause" do + # The supervised ShutdownHandler may or may not be running. If the + # GenServer.call raises, the `rescue _ -> false` kicks in and we get + # a boolean back either way. + assert is_boolean(ShutdownHandler.shutting_down?()) + end + end +end