Skip to content

Commit d1b22d9

Browse files
authored
Merge pull request #19 from hiett/18-if-the-redis-server-goes-down-and-a-closed-is-returned-the-pool-isnt-properly-destroyed
Ensure pool is destroyed when failing to connect to Redis
2 parents 739b573 + f085bb2 commit d1b22d9

File tree

5 files changed

+50
-5
lines changed

5 files changed

+50
-5
lines changed

lib/srh/http/base_router.ex

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ defmodule Srh.Http.BaseRouter do
2626
end
2727

2828
match _ do
29-
handle_response({:not_found, "SRH: Endpoint not found. SRH might not support this feature yet."}, conn)
29+
handle_response(
30+
{:not_found, "SRH: Endpoint not found. SRH might not support this feature yet."},
31+
conn
32+
)
3033
end
3134

3235
defp do_command_request(conn, success_lambda) do

lib/srh/http/command_handler.ex

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ defmodule Srh.Http.CommandHandler do
130130
# Fire back the result here, because the initial Multi was successful
131131
result
132132

133+
{:error, %{reason: :closed} = error} ->
134+
# Ensure that this pool is killed, but still pass the error up the chain for the response
135+
Client.destroy_workers(client_pid)
136+
decode_error(error, srh_id)
137+
133138
{:error, error} ->
134139
decode_error(error, srh_id)
135140
end
@@ -175,6 +180,12 @@ defmodule Srh.Http.CommandHandler do
175180
{:ok, res} ->
176181
{:ok, %{result: res}}
177182

183+
# Jedix connection error
184+
{:error, %{reason: :closed} = error} ->
185+
# Ensure that this pool is killed, but still pass the error up the chain for the response
186+
Client.destroy_workers(pid)
187+
decode_error(error, srh_id)
188+
178189
{:error, error} ->
179190
decode_error(error, srh_id)
180191
end

lib/srh/http/content_type_check_plug.ex

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ defmodule Srh.Http.ContentTypeCheckPlug do
3535
# Return a custom error, ensuring the same format as the other errors
3636
conn
3737
|> put_resp_content_type("application/json")
38-
|> send_resp(400, Jason.encode!(%{error: "Invalid content type. Expected application/json."}))
38+
|> send_resp(
39+
400,
40+
Jason.encode!(%{error: "Invalid content type. Expected application/json."})
41+
)
3942
|> halt()
4043
end
4144
end

lib/srh/redis/client.ex

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ defmodule Srh.Redis.Client do
33
alias Srh.Redis.ClientRegistry
44
alias Srh.Redis.ClientWorker
55

6-
@idle_death_time 1000 * 15
6+
@idle_death_time 1000 * 60 * 15 # 15 minutes
77

88
def start_link(max_connections, connection_info) do
99
GenServer.start_link(__MODULE__, {max_connections, connection_info}, [])
@@ -35,6 +35,10 @@ defmodule Srh.Redis.Client do
3535
GenServer.cast(client, {:return_worker, pid})
3636
end
3737

38+
def destroy_workers(client) do
39+
GenServer.cast(client, {:destroy_workers})
40+
end
41+
3842
def handle_call({:find_worker}, _from, %{registry_pid: registry_pid} = state)
3943
when is_pid(registry_pid) do
4044
{:ok, worker} = ClientRegistry.find_worker(registry_pid)
@@ -59,13 +63,17 @@ defmodule Srh.Redis.Client do
5963
{:noreply, state}
6064
end
6165

66+
def handle_cast({:destroy_workers}, state) do
67+
ClientRegistry.destroy_workers(state.registry_pid)
68+
{:stop, :normal, state}
69+
end
70+
6271
def handle_cast(_msg, state) do
6372
{:noreply, state}
6473
end
6574

6675
def handle_info(:idle_death, state) do
6776
ClientRegistry.destroy_workers(state.registry_pid)
68-
6977
{:stop, :normal, state}
7078
end
7179

lib/srh/redis/client_worker.ex

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,29 @@ defmodule Srh.Redis.ClientWorker do
6262
} = state
6363
)
6464
when is_binary(connection_string) do
65+
enable_ssl = String.starts_with?(connection_string, "rediss://")
66+
67+
socket_opts =
68+
case enable_ssl do
69+
true ->
70+
[
71+
customize_hostname_check: [
72+
match_fun: :public_key.pkix_verify_hostname_match_fun(:https)
73+
]
74+
]
75+
76+
false ->
77+
[]
78+
end
79+
6580
# NOTE: Redix only seems to open the connection when the first command is sent
6681
# This means that this will return :ok even if the connection string may not actually be connectable
67-
{:ok, pid} = Redix.start_link(connection_string)
82+
{:ok, pid} =
83+
Redix.start_link(connection_string,
84+
ssl: enable_ssl,
85+
socket_opts: socket_opts
86+
)
87+
6888
{:noreply, %{state | redix_pid: pid}}
6989
end
7090

0 commit comments

Comments
 (0)