From f33ff7cbacb204adae9d53ad15829f44c4140525 Mon Sep 17 00:00:00 2001
From: Alexey Lebedeff <alebedev@mirantis.com>
Date: Fri, 22 Apr 2016 17:33:34 +0300
Subject: Improve error reporting

Simplify some return values, improve error messages for
eaddrinuse and no_cert.

Amended to add tests and simpler code. Also hides the
contents of cert and key transport options, if any.
---
 src/ranch.erl               | 13 ++++++++++++-
 src/ranch_acceptors_sup.erl | 13 ++++++++++---
 src/ranch_ssl.erl           | 15 +++++++++++----
 test/acceptor_SUITE.erl     | 37 +++++++++++++++++++++++++++++++++++--
 4 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/src/ranch.erl b/src/ranch.erl
index 7f751cf..9eb9a47 100644
--- a/src/ranch.erl
+++ b/src/ranch.erl
@@ -74,9 +74,20 @@ start_listener(Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts)
 				_ ->
 					ok
 			end,
-			Res
+			maybe_started(Res)
 	end.
 
+maybe_started({error, {{shutdown,
+		{failed_to_start_child, ranch_acceptors_sup,
+			{listen_error, _, Reason}}}, _}} = Error) ->
+	start_error(Reason, Error);
+maybe_started(Res) ->
+	Res.
+
+start_error(E=eaddrinuse, _) -> {error, E};
+start_error(E=no_cert, _) -> {error, E};
+start_error(_, Error) -> Error.
+
 -spec stop_listener(ref()) -> ok | {error, not_found}.
 stop_listener(Ref) ->
 	case supervisor:terminate_child(ranch_sup, {ranch_listener_sup, Ref}) of
diff --git a/src/ranch_acceptors_sup.erl b/src/ranch_acceptors_sup.erl
index 67854d5..d716a33 100644
--- a/src/ranch_acceptors_sup.erl
+++ b/src/ranch_acceptors_sup.erl
@@ -49,8 +49,15 @@ init([Ref, NumAcceptors, Transport, TransOpts]) ->
 	{ok, {{one_for_one, 1, 5}, Procs}}.
 
 -spec listen_error(any(), module(), any(), atom()) -> no_return().
-listen_error(Ref, Transport, TransOpts2, Reason) ->
+listen_error(Ref, Transport, TransOpts0, Reason) ->
+	TransOpts1 = lists:keyreplace(cert, 1, TransOpts0, {cert, '...'}),
+	TransOpts = lists:keyreplace(key, 1, TransOpts1, {key, '...'}),
 	error_logger:error_msg(
-		"Failed to start Ranch listener ~p in ~p:listen(~p) for reason ~p (~s)~n",
-		[Ref, Transport, TransOpts2, Reason, inet:format_error(Reason)]),
+		"Failed to start Ranch listener ~p in ~p:listen(~999999p) for reason ~p (~s)~n",
+		[Ref, Transport, TransOpts, Reason, format_error(Reason)]),
 	exit({listen_error, Ref, Reason}).
+
+format_error(no_cert) ->
+	"no certificate provided; see cert, certfile, sni_fun or sni_hosts options";
+format_error(Reason) ->
+	inet:format_error(Reason).
diff --git a/src/ranch_ssl.erl b/src/ranch_ssl.erl
index b3fab51..0385065 100644
--- a/src/ranch_ssl.erl
+++ b/src/ranch_ssl.erl
@@ -89,10 +89,17 @@ messages() -> {ssl, ssl_closed, ssl_error}.
 
 -spec listen(opts()) -> {ok, ssl:sslsocket()} | {error, atom()}.
 listen(Opts) ->
-	true = lists:keymember(cert, 1, Opts)
-		orelse lists:keymember(certfile, 1, Opts)
-		orelse lists:keymember(sni_fun, 1, Opts)
-		orelse lists:keymember(sni_hosts, 1, Opts),
+	case lists:keymember(cert, 1, Opts)
+			orelse lists:keymember(certfile, 1, Opts)
+			orelse lists:keymember(sni_fun, 1, Opts)
+			orelse lists:keymember(sni_hosts, 1, Opts) of
+		true ->
+			do_listen(Opts);
+		false ->
+			{error, no_cert}
+	end.
+
+do_listen(Opts) ->
 	Opts2 = ranch:set_option_default(Opts, backlog, 1024),
 	Opts3 = ranch:set_option_default(Opts2, ciphers, unbroken_cipher_suites()),
 	Opts4 = ranch:set_option_default(Opts3, nodelay, true),
diff --git a/test/acceptor_SUITE.erl b/test/acceptor_SUITE.erl
index 547d1c0..7235420 100644
--- a/test/acceptor_SUITE.erl
+++ b/test/acceptor_SUITE.erl
@@ -34,14 +34,17 @@ groups() ->
 		tcp_max_connections_infinity,
 		tcp_set_max_connections,
 		tcp_set_max_connections_clean,
-		tcp_upgrade
+		tcp_upgrade,
+		tcp_error_eaddrinuse
 	]}, {ssl, [
 		ssl_accept_error,
 		ssl_accept_socket,
 		ssl_active_echo,
 		ssl_echo,
 		ssl_sni_echo,
-		ssl_sni_fail
+		ssl_sni_fail,
+		ssl_error_eaddrinuse,
+		ssl_error_no_cert
 	]}, {misc, [
 		misc_bad_transport,
 		misc_bad_transport_options
@@ -177,6 +180,24 @@ do_ssl_sni_fail() ->
 	{'EXIT', _} = begin catch ranch:get_port(Name) end,
 	ok.
 
+ssl_error_eaddrinuse(_) ->
+	doc("Check that eaddrinuse returns a simplified error."),
+	Name = name(),
+	Opts = ct_helper:get_certs_from_ets(),
+	{ok, _} = ranch:start_listener(Name, 1, ranch_ssl, Opts, active_echo_protocol, []),
+	Port = ranch:get_port(Name),
+	{error, eaddrinuse} = ranch:start_listener({Name, fails}, 1,
+		ranch_ssl, [{port, Port}|Opts], active_echo_protocol, []),
+	ok = ranch:stop_listener(Name),
+	%% Make sure the listener stopped.
+	{'EXIT', _} = begin catch ranch:get_port(Name) end,
+	ok.
+
+ssl_error_no_cert(_) ->
+	doc("Check that missing certificate returns a simplified error."),
+	{error, no_cert} = ranch:start_listener(name(), 1, ranch_ssl, [], active_echo_protocol, []),
+	ok.
+
 %% tcp.
 
 tcp_accept_socket(_) ->
@@ -350,6 +371,18 @@ tcp_upgrade(_) ->
 	receive upgraded -> ok after 1000 -> error(timeout) end,
 	ok = ranch:stop_listener(Name).
 
+tcp_error_eaddrinuse(_) ->
+	doc("Check that eaddrinuse returns a simplified error."),
+	Name = name(),
+	{ok, _} = ranch:start_listener(Name, 1, ranch_tcp, [], active_echo_protocol, []),
+	Port = ranch:get_port(Name),
+	{error, eaddrinuse} = ranch:start_listener({Name, fails}, 1,
+		ranch_tcp, [{port, Port}], active_echo_protocol, []),
+	ok = ranch:stop_listener(Name),
+	%% Make sure the listener stopped.
+	{'EXIT', _} = begin catch ranch:get_port(Name) end,
+	ok.
+
 %% Supervisor tests
 
 connection_type_supervisor(_) ->
-- 
cgit v1.2.3