From c98c045affe053ba37e062e2754f32dcd65a0b30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= <essen@ninenines.eu>
Date: Mon, 21 Nov 2022 17:53:29 +0100
Subject: Don't infinite loop gun_pool on bad configuration

We leave the pool in a degraded state if the configuration
given is invalid and a lookup/connect can't be made.
---
 src/gun_pool.erl    | 17 ++++++++++++-----
 test/pool_SUITE.erl | 19 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/src/gun_pool.erl b/src/gun_pool.erl
index 5d49a7c..e99a364 100644
--- a/src/gun_pool.erl
+++ b/src/gun_pool.erl
@@ -631,14 +631,21 @@ handle_common(info, {gun_down, ConnPid, Protocol, _Reason, _KilledStreams}, _, S
 	{up, Protocol, _} = maps:get(ConnPid, Conns),
 	{next_state, degraded, StateData#state{conns=Conns#{ConnPid => down}}};
 %% @todo We do not want to reconnect automatically when the pool is dynamic.
-handle_common(info, {'DOWN', _MRef, process, ConnPid0, _Reason}, _,
+handle_common(info, {'DOWN', _MRef, process, ConnPid0, Reason}, _,
 		StateData=#state{host=Host, port=Port, opts=Opts, table=Tid, conns=Conns0, conns_meta=ConnsMeta0}) ->
 	Conns = maps:remove(ConnPid0, Conns0),
 	ConnsMeta = maps:remove(ConnPid0, ConnsMeta0),
-	ConnOpts = conn_opts(Tid, Opts),
-	{ok, ConnPid} = gun:open(Host, Port, ConnOpts),
-	_ = monitor(process, ConnPid),
-	{next_state, degraded, StateData#state{conns=Conns#{ConnPid => down}, conns_meta=ConnsMeta}};
+	case Reason of
+		%% The process is down because of a configuration error.
+		%% Do NOT attempt to reconnect, leave the pool in a degraded state.
+		badarg ->
+			{next_state, degraded, StateData#state{conns=Conns, conns_meta=ConnsMeta}};
+		_ ->
+			ConnOpts = conn_opts(Tid, Opts),
+			{ok, ConnPid} = gun:open(Host, Port, ConnOpts),
+			_ = monitor(process, ConnPid),
+			{next_state, degraded, StateData#state{conns=Conns#{ConnPid => down}, conns_meta=ConnsMeta}}
+	end;
 handle_common({call, From}, info, StateName, #state{host=Host, port=Port,
 		opts=Opts, table=Tid, conns=Conns, conns_meta=ConnsMeta}) ->
 	{keep_state_and_data, {reply, From, {StateName, #{
diff --git a/test/pool_SUITE.erl b/test/pool_SUITE.erl
index 7643368..3105aa6 100644
--- a/test/pool_SUITE.erl
+++ b/test/pool_SUITE.erl
@@ -376,8 +376,25 @@ reconnect_h2(Config) ->
 %% @todo reconnect_ws
 
 stop_pool(Config) ->
-	doc("Confirm the pool can be used for HTTP/1.1 connections."),
+	doc("Confirm the pool can be stopped."),
 	Port = config(port, Config),
 	{ok, ManagerPid} = gun_pool:start_pool("localhost", Port, #{scope => ?FUNCTION_NAME}),
 	gun_pool:await_up(ManagerPid),
 	gun_pool:stop_pool("localhost", Port, #{scope => ?FUNCTION_NAME}).
+
+degraded_configuration_error(Config) ->
+	doc("Confirm the pool ends up in a degraded state "
+		"when connection is impossible because of bad configuration."),
+	Port = config(port, Config),
+	%% We attempt to connect to an unreachable IP.
+	{ok, ManagerPid} = gun_pool:start_pool({20, 20, 20, 1}, Port, #{
+		conn_opts => #{tcp_opts => [{ip, {127, 0, 0, 1}}]},
+		scope => ?FUNCTION_NAME,
+		size => 1
+	}),
+	%% Wait for the lookup/connect to fail.
+	timer:sleep(500),
+	{degraded, #{conns := Conns}} = gun_pool:info(ManagerPid),
+	true = Conns =:= #{},
+	%% We can stop the pool even if degraded.
+	gun_pool:stop_pool({20, 20, 20, 1}, Port, #{scope => ?FUNCTION_NAME}).
-- 
cgit v1.2.3