Skip to content

Commit

Permalink
chore: remove unneccessary calls from terminate callback
Browse files Browse the repository at this point in the history
  • Loading branch information
@tlux
tlux committed Mar 29, 2024
1 parent ad1915a commit 20de476
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 35 deletions.
39 changes: 24 additions & 15 deletions lib/graphql_ws_client.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,9 @@ defmodule GraphQLWSClient do
"""
@spec query!(client, query, variables) :: any | no_return
def query!(client, query, variables \\ %{}) do
case query(client, query, variables) do
{:ok, result} -> result
{:error, error} -> raise error
end
client
|> query(query, variables)
|> bang!()
end

@doc """
Expand Down Expand Up @@ -429,8 +428,8 @@ defmodule GraphQLWSClient do
state = put_init_payload(state, info)

case Driver.connect(config, state.init_payload) do
{:ok, %Conn{} = conn} ->
monitor_ref = Process.monitor(conn.pid)
{:ok, %Conn{pid: pid} = conn} ->
monitor_ref = maybe_monitor(pid)

with {:open, from, _} <- info do
Connection.reply(from, :ok)
Expand Down Expand Up @@ -482,15 +481,19 @@ defmodule GraphQLWSClient do
end

@impl true
def terminate(reason, %State{} = state) do
def terminate(reason, %State{conn: nil}) do
Logger.debug(format_log("Terminating client (reason: #{inspect(reason)})"))
end

def terminate(reason, %State{conn: conn}) do
Logger.debug(
format_log(
"Terminating client, closing connection " <>
"(reason: #{inspect(reason)})"
)
)

close_connection(state)
Driver.disconnect(conn)
end

@impl true
Expand Down Expand Up @@ -575,7 +578,7 @@ defmodule GraphQLWSClient do
case Map.fetch(listeners, id) do
{:ok, %State.Listener{monitor_ref: monitor_ref}} ->
Driver.push_complete(state.conn, id)
Process.demonitor(monitor_ref)
maybe_demonitor(monitor_ref, [:flush])

{:reply, :ok, State.remove_listener(state, id)}

Expand Down Expand Up @@ -641,18 +644,15 @@ defmodule GraphQLWSClient do

:disconnect ->
Logger.debug(format_log("Socket went down"))

{:disconnect, {:error, %SocketError{cause: :closed}}, state}

:ignore ->
Logger.debug(format_log("Ignored unexpected payload: #{inspect(msg)}"))

{:noreply, state}
end
end

def handle_info(_msg, state) do
# ignore unexpected payload
{:noreply, state}
end

Expand All @@ -663,7 +663,7 @@ defmodule GraphQLWSClient do

with {:ok, %State.Listener{pid: pid, monitor_ref: monitor_ref}} <-
Map.fetch(state.listeners, id) do
Process.demonitor(monitor_ref)
maybe_demonitor(monitor_ref, [:flush])
send(pid, %Event{subscription_id: id, type: :complete})
end

Expand All @@ -684,7 +684,7 @@ defmodule GraphQLWSClient do
{:ok, %State.Listener{pid: pid, monitor_ref: monitor_ref}} ->
Logger.debug(format_log("Message #{id} - error: #{inspect(payload)}"))

Process.demonitor(monitor_ref)
maybe_demonitor(monitor_ref, [:flush])
send(pid, %Event{subscription_id: id, type: :error, payload: error})

{:noreply, State.remove_listener(state, id)}
Expand Down Expand Up @@ -748,7 +748,7 @@ defmodule GraphQLWSClient do
defp close_connection(%State{connected?: false} = state), do: state

defp close_connection(%State{conn: conn, monitor_ref: monitor_ref} = state) do
Process.demonitor(monitor_ref)
maybe_demonitor(monitor_ref, [:flush])
Driver.disconnect(conn)
State.reset_conn(state)
end
Expand All @@ -757,6 +757,15 @@ defmodule GraphQLWSClient do
%{query: query, variables: Map.new(variables)}
end

defp maybe_monitor(nil), do: nil
defp maybe_monitor(pid) when is_pid(pid), do: Process.monitor(pid)

defp maybe_demonitor(nil, _opts), do: true

defp maybe_demonitor(ref, opts) when is_reference(ref) do
Process.demonitor(ref, opts)
end

defp bang!(:ok), do: :ok
defp bang!({:ok, result}), do: result
defp bang!({:error, error}), do: raise(error)
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql_ws_client/state.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ defmodule GraphQLWSClient.State do
queries: %{}
]

@spec put_conn(t, Conn.t(), reference) :: t
@spec put_conn(t, Conn.t(), nil | reference) :: t
def put_conn(%__MODULE__{} = state, conn, monitor_ref) do
%{
state
Expand Down
7 changes: 4 additions & 3 deletions test/graphql_ws_client/integration_test.exs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
defmodule GraphQLWSClient.IntegrationTest do
use ExUnit.Case
use ExUnit.Case, async: true

alias GraphQLWSClient.{Event, GraphQLError, TestClient}

Expand Down Expand Up @@ -155,12 +155,13 @@ defmodule GraphQLWSClient.IntegrationTest do
describe "stop" do
test "close handle", %{client: client} do
assert Process.alive?(client)
%{mod_state: %{conn: %{pid: pid}}} = :sys.get_state(client)
%{mod_state: %{conn: %{pid: driver_pid}}} = :sys.get_state(client)
assert Process.alive?(driver_pid)

stop_supervised!(:test_client)

refute Process.alive?(client)
refute Process.alive?(pid)
refute Process.alive?(driver_pid)
end
end

Expand Down
32 changes: 16 additions & 16 deletions test/graphql_ws_client_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ defmodule GraphQLWSClientTest do
{:ok, conn}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.connected?(client) == false
assert GraphQLWSClient.open(client) == :ok
Expand All @@ -101,13 +101,13 @@ defmodule GraphQLWSClientTest do
{:error, error}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.open(client) == {:error, error}
end

test "no-op when already connected", %{config: config, conn: conn} do
{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

expect(MockDriver, :connect, fn ^conn ->
{:ok, conn}
Expand All @@ -130,7 +130,7 @@ defmodule GraphQLWSClientTest do
{:ok, conn}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.connected?(client) == false
assert GraphQLWSClient.open!(client) == :ok
Expand All @@ -144,7 +144,7 @@ defmodule GraphQLWSClientTest do
{:error, error}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert_raise SocketError, Exception.message(error), fn ->
GraphQLWSClient.open!(client)
Expand All @@ -167,7 +167,7 @@ defmodule GraphQLWSClientTest do
{:ok, conn}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.connected?(client) == false
assert GraphQLWSClient.open_with(client, @init_payload) == :ok
Expand All @@ -181,13 +181,13 @@ defmodule GraphQLWSClientTest do
{:error, error}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.open_with(client, @init_payload) == {:error, error}
end

test "no-op when already connected", %{config: config, conn: conn} do
{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

expect(MockDriver, :connect, fn ^conn ->
{:ok, conn}
Expand All @@ -210,7 +210,7 @@ defmodule GraphQLWSClientTest do
|> expect(:disconnect, fn ^conn -> :ok end)
|> expect(:connect, fn ^reconnect_conn -> {:ok, reconnect_conn} end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.open_with(client, @init_payload) == :ok
assert GraphQLWSClient.close(client) == :ok
Expand All @@ -235,7 +235,7 @@ defmodule GraphQLWSClientTest do
{:ok, conn}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.open_with(client, @init_payload) == {:error, error}
assert_receive :reconnected_msg
Expand All @@ -260,7 +260,7 @@ defmodule GraphQLWSClientTest do
{:ok, conn}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.open_with(client, @init_payload) == :ok
assert_receive :reconnected_msg
Expand All @@ -281,7 +281,7 @@ defmodule GraphQLWSClientTest do
{:ok, conn}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.connected?(client) == false
assert GraphQLWSClient.open_with!(client, @init_payload) == :ok
Expand All @@ -295,7 +295,7 @@ defmodule GraphQLWSClientTest do
{:error, error}
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert_raise SocketError, Exception.message(error), fn ->
GraphQLWSClient.open_with!(client, @init_payload)
Expand Down Expand Up @@ -383,7 +383,7 @@ defmodule GraphQLWSClientTest do

test "not connected" do
config = %{@config | connect_on_start: false}
{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.query(client, @query, @variables) ==
{:error, %SocketError{cause: :closed}}
Expand All @@ -405,7 +405,7 @@ defmodule GraphQLWSClientTest do
:ok
end)

{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})

assert GraphQLWSClient.query(client, @query, @variables) ==
{:error, %SocketError{cause: :timeout}}
Expand Down Expand Up @@ -588,7 +588,7 @@ defmodule GraphQLWSClientTest do

test "not connected" do
config = %{@config | connect_on_start: false}
{:ok, client} = GraphQLWSClient.start_link(config)
client = start_supervised!({GraphQLWSClient, config})
error = %SocketError{cause: :closed}

assert_raise SocketError, Exception.message(error), fn ->
Expand Down

0 comments on commit 20de476

Please sign in to comment.