From 2949f1ab029318ff4698e9eee72c7d713ea76e0e Mon Sep 17 00:00:00 2001 From: Serhii Snozyk Date: Fri, 4 Sep 2026 18:55:41 +0200 Subject: [PATCH 1/5] fix: start the close handshake for every close frame Only a close frame received from the server, or returned as `{:close, code, reason, state}` from a callback, went through the close handshake. One sent with `Minch.send_frame/2` or returned as `{:reply, frame, state}` was written straight to the socket: no `:close_timeout` was armed, further frames were still accepted, and the disconnect was reported as a transport error instead of the frame. Every close frame now starts the handshake, whichever path it comes from. Frames that arrive while it is in flight are still delivered to `handle_frame/2` rather than dropped, a ping is still answered, and the connection is torn down as soon as both sides have sent a close frame instead of waiting for the server to close the socket. Frames sent during a handshake are rejected with the new `{:error, :closing}`. --- lib/minch.ex | 10 ++++-- lib/minch/conn.ex | 72 ++++++++++++++++++++++++++++---------- test/minch/client_test.exs | 31 ++++++++++++++++ 3 files changed, 93 insertions(+), 20 deletions(-) diff --git a/lib/minch.ex b/lib/minch.ex index 52383fd..35fa650 100644 --- a/lib/minch.ex +++ b/lib/minch.ex @@ -10,6 +10,7 @@ defmodule Minch do @type frame :: Mint.WebSocket.frame() | Mint.WebSocket.shorthand_frame() @type option :: {:close_timeout, non_neg_integer()} | GenServer.option() @type error :: Mint.WebSocket.error() | {:invalid_scheme, String.t() | nil} + @type send_error :: :not_connected | :closing | Mint.WebSocket.error() @type callback_result :: {:ok, state()} @@ -125,9 +126,14 @@ defmodule Minch do @doc """ Sends a WebSocket frame. + + Sending a close frame starts the close handshake: further frames are rejected with + `{:error, :closing}`, incoming frames are still delivered to `c:handle_frame/2`, and + `c:handle_disconnect/3` is invoked with that frame once the server answers or + `:close_timeout` elapses. Note that the default `c:handle_disconnect/3` implementation + reconnects. """ - @spec send_frame(client(), Mint.WebSocket.frame() | Mint.WebSocket.shorthand_frame()) :: - :ok | {:error, term()} + @spec send_frame(client(), frame()) :: :ok | {:error, send_error()} def send_frame(client, frame) do GenServer.call(client, {:send_frame, frame}) end diff --git a/lib/minch/conn.ex b/lib/minch/conn.ex index 061cb0d..4416d3a 100644 --- a/lib/minch/conn.ex +++ b/lib/minch/conn.ex @@ -62,8 +62,7 @@ defmodule Minch.Conn do @impl true def terminate(reason, %State{} = state) do - send_frame(state, :close) - state = close(state) + state = state |> send_frame(:close) |> discard_error() |> close() state.callback.terminate(reason, state.callback_state) end @@ -98,10 +97,7 @@ defmodule Minch.Conn do @impl true def handle_info({@internal, {:send_frame, frame}}, state) do - case send_frame(state, frame) do - {:ok, state} -> {:noreply, state} - {:error, state, error} -> handle_error(error, state) - end + state |> send_frame(frame) |> handle_send() end def handle_info({@internal, :reconnect}, %State{} = state) do @@ -180,13 +176,20 @@ defmodule Minch.Conn do {:noreply, state} end - defp handle_frame({:close, _, _} = frame, state) do - {:noreply, send_close(state, frame)} + # the server initiated close + defp handle_frame({:close, _, _} = frame, %State{close_frame: nil} = state) do + state = state |> stream_frame(frame) |> discard_error() + handle_disconnect(frame, state) + end + + # the server answered our close frame + defp handle_frame({:close, _, _}, %State{} = state) do + handle_disconnect(state.close_frame, state) end + # a ping must be answered even after we have sent a close frame defp handle_frame({:ping, data}, %State{} = state) do - internal_event({:send_frame, {:pong, data}}) - {:noreply, state} + state |> stream_frame({:pong, data}) |> handle_send() end defp handle_frame(frame, %State{} = data) do @@ -222,16 +225,42 @@ defmodule Minch.Conn do {:noreply, %{state | callback_state: callback_state}} {:close, code, reason, callback_state} -> - {:noreply, send_close(%{state | callback_state: callback_state}, {:close, code, reason})} + %{state | callback_state: callback_state} + |> send_close({:close, code, reason}) + |> handle_close() {:stop, reason, callback_state} -> {:stop, reason, %{state | callback_state: callback_state}} end end - defp send_frame(%State{websocket: nil} = state, _frame), do: {:error, state, :not_connected} + defp handle_send({:ok, state}), do: {:noreply, state} + defp handle_send({:error, state, error}), do: handle_error(error, state) + + defp handle_close({:ok, state}), do: {:noreply, state} + + # nothing was sent: there is no connection, or a handshake is already in flight + defp handle_close({:error, state, reason}) when reason in [:not_connected, :closing] do + {:noreply, state} + end - defp send_frame(%State{websocket: websocket} = state, frame) do + # unlike an ordinary send error, a close that can't be sent still tears the connection down + defp handle_close({:error, state, error}), do: handle_disconnect(error, state) + + defp discard_error({:ok, state}), do: state + defp discard_error({:error, state, _error}), do: state + + defp send_frame(state, {:close, _, _} = frame), do: send_close(state, frame) + defp send_frame(state, :close = frame), do: send_close(state, frame) + defp send_frame(%State{close_frame: nil} = state, frame), do: stream_frame(state, frame) + # no frame may follow the close frame that started the handshake + defp send_frame(state, _frame), do: {:error, state, :closing} + + defp stream_frame(%State{websocket: nil} = state, _frame) do + {:error, state, :not_connected} + end + + defp stream_frame(%State{websocket: websocket} = state, frame) do case Mint.WebSocket.encode(websocket, frame) do {:ok, websocket, bin} -> case Mint.WebSocket.stream_request_body(state.conn, state.request_ref, bin) do @@ -286,13 +315,20 @@ defmodule Minch.Conn do defp schemes("wss"), do: {:ok, :https, :wss} defp schemes(scheme), do: {:error, {:invalid_scheme, scheme}} - defp send_close(%State{} = state, frame) do - send_frame(state, frame) - cancel_timer(state.close_timer) - close_timer = internal_event(:close_timeout, state.close_timeout) - %{state | websocket: nil, close_timer: close_timer, close_frame: frame} + defp send_close(%State{close_frame: nil} = state, frame) do + with {:ok, state} <- stream_frame(state, frame) do + close_timer = internal_event(:close_timeout, state.close_timeout) + {:ok, %{state | close_timer: close_timer, close_frame: normalize_close(frame)}} + end end + # only one close handshake at a time + defp send_close(state, _frame), do: {:error, state, :closing} + + # Mint decodes a payload-less close as 1000/"", so report our shorthand the same way + defp normalize_close(:close), do: {:close, 1000, ""} + defp normalize_close(frame), do: frame + defp close(%State{conn: conn} = state) do if conn, do: Mint.HTTP.close(conn) cancel_timer(state.close_timer) diff --git a/test/minch/client_test.exs b/test/minch/client_test.exs index 57db0f9..988001a 100644 --- a/test/minch/client_test.exs +++ b/test/minch/client_test.exs @@ -184,6 +184,37 @@ defmodule Minch.ClientTest do assert {:error, :not_connected} = Minch.send_frame(pid, {:text, "hello"}) end + test "starts the close handshake for a :close frame sent with send_frame/2", ctx do + assert_receive {:client, :handle_connect, _} + assert :ok = Minch.send_frame(ctx.client, {:close, 1000, "bye"}) + assert {:error, :closing} = Minch.send_frame(ctx.client, {:text, "hello"}) + assert {:error, :closing} = Minch.send_frame(ctx.client, {:close, 1001, "again"}) + assert_receive {:server, :terminate, {:remote, 1000, "bye"}} + assert_receive {:client, :handle_disconnect, [{:close, 1000, "bye"}, 1, _]} + end + + test "handle_frame/2 is called with frames received after sending a :close frame", ctx do + assert_receive {:client, :handle_connect, _} + # suspended so the frames are queued ahead of our close frame in the server's mailbox + :sys.suspend(ctx.server) + Server.send_frame(ctx.server, [{:text, "a"}, {:text, "b"}]) + assert :ok = Minch.send_frame(ctx.client, {:close, 1000, "bye"}) + :sys.resume(ctx.server) + assert_receive {:client, :handle_frame, [{:text, "a"}, _state]} + assert_receive {:client, :handle_frame, [{:text, "b"}, _state]} + assert_receive {:client, :handle_disconnect, [{:close, 1000, "bye"}, 1, _state]} + end + + @tag client_state: %{opts: [close_timeout: 50]} + test "send_frame/2 with a :close frame arms the close timeout", ctx do + assert_receive {:client, :handle_connect, _} + # suspended so the server never reads our close frame, and never closes the socket + :sys.suspend(ctx.server) + assert :ok = Minch.send_frame(ctx.client, :close) + assert_receive {:client, :handle_disconnect, [{:close, 1000, ""}, 1, _state]} + :sys.resume(ctx.server) + end + test "gracefully closes the connection by returning a :close tuple from a callback", ctx do assert_receive {:client, :handle_connect, _} send(ctx.client, {:close, 1000, "bye"}) From 5544bb204682126e0e75843c48d8ef1128380803 Mon Sep 17 00:00:00 2001 From: Serhii Snozyk Date: Fri, 4 Sep 2026 19:17:30 +0200 Subject: [PATCH 2/5] test: cover a close frame replied from a callback The third way an application can start the handshake, alongside `Minch.send_frame/2` and the `{:close, code, reason, state}` callback result. --- test/minch/client_test.exs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/minch/client_test.exs b/test/minch/client_test.exs index 988001a..16fdea4 100644 --- a/test/minch/client_test.exs +++ b/test/minch/client_test.exs @@ -222,6 +222,20 @@ defmodule Minch.ClientTest do assert_receive {:client, :handle_disconnect, [{:close, 1000, "bye"}, 1, _]} end + @tag client_state: %{reconnect: 10} + test "starts the close handshake for a :close frame replied from a callback", ctx do + assert_receive {:client, :handle_connect, _} + send(ctx.client, {:reply, {:close, 1000, "bye"}}) + assert_receive {:server, :terminate, {:remote, 1000, "bye"}} + assert_receive {:client, :handle_disconnect, [{:close, 1000, "bye"}, 1, _state]} + + assert_receive {:server, :init, _} + assert_receive {:client, :handle_connect, _} + send(ctx.client, {:reply, :close}) + assert_receive {:server, :terminate, :remote} + assert_receive {:client, :handle_disconnect, [{:close, 1000, ""}, 1, _state]} + end + test "stops the client process by returning a :stop tuple from a callback", ctx do assert_receive {:client, :handle_connect, _} send(ctx.client, {:stop, :normal}) From fb3a9555ecf4aaf613448cee4716a3afb5fbb848 Mon Sep 17 00:00:00 2001 From: Serhii Snozyk Date: Fri, 4 Sep 2026 19:28:39 +0200 Subject: [PATCH 3/5] refactor: match the ignored close errors instead of guarding --- lib/minch.ex | 7 ++----- lib/minch/conn.ex | 10 ++-------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/minch.ex b/lib/minch.ex index 35fa650..cdaa0f7 100644 --- a/lib/minch.ex +++ b/lib/minch.ex @@ -127,11 +127,8 @@ defmodule Minch do @doc """ Sends a WebSocket frame. - Sending a close frame starts the close handshake: further frames are rejected with - `{:error, :closing}`, incoming frames are still delivered to `c:handle_frame/2`, and - `c:handle_disconnect/3` is invoked with that frame once the server answers or - `:close_timeout` elapses. Note that the default `c:handle_disconnect/3` implementation - reconnects. + A close frame starts the close handshake; frames sent after it are rejected with + `{:error, :closing}`. """ @spec send_frame(client(), frame()) :: :ok | {:error, send_error()} def send_frame(client, frame) do diff --git a/lib/minch/conn.ex b/lib/minch/conn.ex index 4416d3a..75cc545 100644 --- a/lib/minch/conn.ex +++ b/lib/minch/conn.ex @@ -238,13 +238,8 @@ defmodule Minch.Conn do defp handle_send({:error, state, error}), do: handle_error(error, state) defp handle_close({:ok, state}), do: {:noreply, state} - - # nothing was sent: there is no connection, or a handshake is already in flight - defp handle_close({:error, state, reason}) when reason in [:not_connected, :closing] do - {:noreply, state} - end - - # unlike an ordinary send error, a close that can't be sent still tears the connection down + defp handle_close({:error, state, :not_connected}), do: {:noreply, state} + defp handle_close({:error, state, :closing}), do: {:noreply, state} defp handle_close({:error, state, error}), do: handle_disconnect(error, state) defp discard_error({:ok, state}), do: state @@ -253,7 +248,6 @@ defmodule Minch.Conn do defp send_frame(state, {:close, _, _} = frame), do: send_close(state, frame) defp send_frame(state, :close = frame), do: send_close(state, frame) defp send_frame(%State{close_frame: nil} = state, frame), do: stream_frame(state, frame) - # no frame may follow the close frame that started the handshake defp send_frame(state, _frame), do: {:error, state, :closing} defp stream_frame(%State{websocket: nil} = state, _frame) do From cd4dff68ee1da1730536b8c0058f8997f9b21c8e Mon Sep 17 00:00:00 2001 From: Serhii Snozyk Date: Fri, 4 Sep 2026 19:41:06 +0200 Subject: [PATCH 4/5] remove comments --- lib/minch/conn.ex | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/minch/conn.ex b/lib/minch/conn.ex index 75cc545..277d972 100644 --- a/lib/minch/conn.ex +++ b/lib/minch/conn.ex @@ -316,10 +316,8 @@ defmodule Minch.Conn do end end - # only one close handshake at a time defp send_close(state, _frame), do: {:error, state, :closing} - # Mint decodes a payload-less close as 1000/"", so report our shorthand the same way defp normalize_close(:close), do: {:close, 1000, ""} defp normalize_close(frame), do: frame From d0ae47ab6bb242c55604e5f9fd13e3c28b974631 Mon Sep 17 00:00:00 2001 From: Serhii Snozyk Date: Fri, 4 Sep 2026 19:59:42 +0200 Subject: [PATCH 5/5] test: cover the close results that are ignored --- test/minch/client_test.exs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/minch/client_test.exs b/test/minch/client_test.exs index 16fdea4..6bf7008 100644 --- a/test/minch/client_test.exs +++ b/test/minch/client_test.exs @@ -236,6 +236,28 @@ defmodule Minch.ClientTest do assert_receive {:client, :handle_disconnect, [{:close, 1000, ""}, 1, _state]} end + @tag client_state: %{opts: [close_timeout: 200]} + test "a :close result during the close handshake is ignored", ctx do + assert_receive {:client, :handle_connect, _} + # suspended so the server never answers our close frame + :sys.suspend(ctx.server) + assert :ok = Minch.send_frame(ctx.client, {:close, 1000, "bye"}) + send(ctx.client, {:close, 1001, "again"}) + refute_receive {:client, :handle_disconnect, _}, 50 + assert_receive {:client, :handle_disconnect, [{:close, 1000, "bye"}, 1, _state]}, 300 + :sys.resume(ctx.server) + end + + @tag server_state: %{init_result: :unauthorized} + @tag client_state: %{reconnect: 100} + test "a :close result while disconnected is ignored", ctx do + assert_receive {:client, :handle_disconnect, [_error, 1, _state]} + send(ctx.client, {:close, 1000, "bye"}) + + assert_receive {:client, :handle_disconnect, + [%Mint.WebSocket.UpgradeFailureError{status_code: 401}, 2, _state]} + end + test "stops the client process by returning a :stop tuple from a callback", ctx do assert_receive {:client, :handle_connect, _} send(ctx.client, {:stop, :normal})