Skip to content

Commit 31f7c2e

Browse files
michaelklishinmichaelklishin
authored
Merge pull request #12305 from rabbitmq/mergify/bp/v4.0.x/pr-12304
Revert "Log errors from `ranch:handshake`" (backport #12304)
2 parents 9cb1985 + bafbdc4 commit 31f7c2e

File tree

5 files changed

+80
-104
lines changed

5 files changed

+80
-104
lines changed

‎deps/rabbit/src/rabbit_networking.erl‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ failed_to_recv_proxy_header(Ref, Error) ->
559559
end,
560560
rabbit_log:debug(Msg, [Error]),
561561
% The following call will clean up resources then exit
562-
_ = catch ranch:handshake(Ref),
562+
_ = ranch:handshake(Ref),
563563
exit({shutdown, failed_to_recv_proxy_header}).
564564

565565
handshake(Ref, ProxyProtocolEnabled) ->
@@ -571,22 +571,14 @@ handshake(Ref, ProxyProtocolEnabled) ->
571571
{error, protocol_error, Error} ->
572572
failed_to_recv_proxy_header(Ref, Error);
573573
{ok, ProxyInfo} ->
574-
case catch ranch:handshake(Ref) of
575-
{'EXIT', normal} ->
576-
{error, handshake_failed};
577-
{ok, Sock} ->
578-
ok = tune_buffer_size(Sock),
579-
{ok, {rabbit_proxy_socket, Sock, ProxyInfo}}
580-
end
574+
{ok, Sock} = ranch:handshake(Ref),
575+
ok = tune_buffer_size(Sock),
576+
{ok, {rabbit_proxy_socket, Sock, ProxyInfo}}
581577
end;
582578
false ->
583-
case catch ranch:handshake(Ref) of
584-
{'EXIT', normal} ->
585-
{error, handshake_failed};
586-
{ok, Sock} ->
587-
ok = tune_buffer_size(Sock),
588-
{ok, Sock}
589-
end
579+
{ok, Sock} = ranch:handshake(Ref),
580+
ok = tune_buffer_size(Sock),
581+
{ok, Sock}
590582
end.
591583

592584
tune_buffer_size(Sock) ->

‎deps/rabbit/src/rabbit_reader.erl‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,6 @@ shutdown(Pid, Explanation) ->
164164
no_return().
165165
init(Parent, HelperSups, Ref) ->
166166
?LG_PROCESS_TYPE(reader),
167-
%% Note:
168-
%% This function could return an error if the handshake times out.
169-
%% It is less likely to happen here as compared to MQTT, so
170-
%% crashing with a `badmatch` seems appropriate.
171167
{ok, Sock} = rabbit_networking:handshake(Ref,
172168
application:get_env(rabbit, proxy_protocol, false)),
173169
Deb = sys:debug_options([]),

‎deps/rabbitmq_mqtt/src/rabbit_mqtt_reader.erl‎

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -71,39 +71,34 @@ close_connection(Pid, Reason) ->
7171
init(Ref) ->
7272
process_flag(trap_exit, true),
7373
logger:set_process_metadata(#{domain => ?RMQLOG_DOMAIN_CONN ++ [mqtt]}),
74-
ProxyProtocolEnabled = application:get_env(?APP_NAME, proxy_protocol, false),
75-
case rabbit_networking:handshake(Ref, ProxyProtocolEnabled) of
74+
{ok, Sock} = rabbit_networking:handshake(Ref,
75+
application:get_env(?APP_NAME, proxy_protocol, false)),
76+
RealSocket = rabbit_net:unwrap_socket(Sock),
77+
case rabbit_net:connection_string(Sock, inbound) of
78+
{ok, ConnStr} ->
79+
ConnName = rabbit_data_coercion:to_binary(ConnStr),
80+
?LOG_DEBUG("MQTT accepting TCP connection ~tp (~ts)", [self(), ConnName]),
81+
_ = rabbit_alarm:register(self(), {?MODULE, conserve_resources, []}),
82+
LoginTimeout = application:get_env(?APP_NAME, login_timeout, 10_000),
83+
erlang:send_after(LoginTimeout, self(), login_timeout),
84+
State0 = #state{socket = RealSocket,
85+
proxy_socket = rabbit_net:maybe_get_proxy_socket(Sock),
86+
conn_name = ConnName,
87+
await_recv = false,
88+
connection_state = running,
89+
conserve = false,
90+
parse_state = rabbit_mqtt_packet:init_state()},
91+
State1 = control_throttle(State0),
92+
State = rabbit_event:init_stats_timer(State1, #state.stats_timer),
93+
gen_server:enter_loop(?MODULE, [], State);
94+
{error, Reason = enotconn} ->
95+
?LOG_INFO("MQTT could not get connection string: ~s", [Reason]),
96+
rabbit_net:fast_close(RealSocket),
97+
ignore;
7698
{error, Reason} ->
77-
?LOG_ERROR("MQTT could not establish connection: ~s", [Reason]),
78-
{stop, Reason};
79-
{ok, Sock} ->
80-
RealSocket = rabbit_net:unwrap_socket(Sock),
81-
case rabbit_net:connection_string(Sock, inbound) of
82-
{ok, ConnStr} ->
83-
ConnName = rabbit_data_coercion:to_binary(ConnStr),
84-
?LOG_DEBUG("MQTT accepting TCP connection ~tp (~ts)", [self(), ConnName]),
85-
_ = rabbit_alarm:register(self(), {?MODULE, conserve_resources, []}),
86-
LoginTimeout = application:get_env(?APP_NAME, login_timeout, 10_000),
87-
erlang:send_after(LoginTimeout, self(), login_timeout),
88-
State0 = #state{socket = RealSocket,
89-
proxy_socket = rabbit_net:maybe_get_proxy_socket(Sock),
90-
conn_name = ConnName,
91-
await_recv = false,
92-
connection_state = running,
93-
conserve = false,
94-
parse_state = rabbit_mqtt_packet:init_state()},
95-
State1 = control_throttle(State0),
96-
State = rabbit_event:init_stats_timer(State1, #state.stats_timer),
97-
gen_server:enter_loop(?MODULE, [], State);
98-
{error, Reason = enotconn} ->
99-
?LOG_INFO("MQTT could not get connection string: ~s", [Reason]),
100-
rabbit_net:fast_close(RealSocket),
101-
ignore;
102-
{error, Reason} ->
103-
?LOG_ERROR("MQTT could not get connection string: ~p", [Reason]),
104-
rabbit_net:fast_close(RealSocket),
105-
{stop, Reason}
106-
end
99+
?LOG_ERROR("MQTT could not get connection string: ~p", [Reason]),
100+
rabbit_net:fast_close(RealSocket),
101+
{stop, Reason}
107102
end.
108103

109104
handle_call({info, InfoItems}, _From, State) ->

‎deps/rabbitmq_stomp/src/rabbit_stomp_reader.erl‎

Lines changed: 42 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -63,55 +63,51 @@ close_connection(Pid, Reason) ->
6363

6464
init([SupHelperPid, Ref, Configuration]) ->
6565
process_flag(trap_exit, true),
66-
ProxyProtocolEnabled = application:get_env(rabbitmq_stomp, proxy_protocol, false),
67-
case rabbit_networking:handshake(Ref, ProxyProtocolEnabled) of
66+
{ok, Sock} = rabbit_networking:handshake(Ref,
67+
application:get_env(rabbitmq_stomp, proxy_protocol, false)),
68+
RealSocket = rabbit_net:unwrap_socket(Sock),
69+
70+
case rabbit_net:connection_string(Sock, inbound) of
71+
{ok, ConnStr} ->
72+
ConnName = rabbit_data_coercion:to_binary(ConnStr),
73+
ProcInitArgs = processor_args(Configuration, Sock),
74+
ProcState = rabbit_stomp_processor:initial_state(Configuration,
75+
ProcInitArgs),
76+
77+
rabbit_log_connection:info("accepting STOMP connection ~tp (~ts)",
78+
[self(), ConnName]),
79+
80+
ParseState = rabbit_stomp_frame:initial_state(),
81+
_ = register_resource_alarm(),
82+
83+
LoginTimeout = application:get_env(rabbitmq_stomp, login_timeout, 10_000),
84+
MaxFrameSize = application:get_env(rabbitmq_stomp, max_frame_size, ?DEFAULT_MAX_FRAME_SIZE),
85+
erlang:send_after(LoginTimeout, self(), login_timeout),
86+
87+
gen_server2:enter_loop(?MODULE, [],
88+
rabbit_event:init_stats_timer(
89+
run_socket(control_throttle(
90+
#reader_state{socket = RealSocket,
91+
conn_name = ConnName,
92+
parse_state = ParseState,
93+
processor_state = ProcState,
94+
heartbeat_sup = SupHelperPid,
95+
heartbeat = {none, none},
96+
max_frame_size = MaxFrameSize,
97+
current_frame_size = 0,
98+
state = running,
99+
conserve_resources = false,
100+
recv_outstanding = false})), #reader_state.stats_timer),
101+
{backoff, 1000, 1000, 10000});
102+
{error, enotconn} ->
103+
rabbit_net:fast_close(RealSocket),
104+
terminate(shutdown, undefined);
68105
{error, Reason} ->
69-
rabbit_log_connection:error(
70-
"STOMP could not establish connection: ~s", [Reason]),
71-
{stop, Reason};
72-
{ok, Sock} ->
73-
RealSocket = rabbit_net:unwrap_socket(Sock),
74-
case rabbit_net:connection_string(Sock, inbound) of
75-
{ok, ConnStr} ->
76-
ConnName = rabbit_data_coercion:to_binary(ConnStr),
77-
ProcInitArgs = processor_args(Configuration, Sock),
78-
ProcState = rabbit_stomp_processor:initial_state(Configuration,
79-
ProcInitArgs),
80-
81-
rabbit_log_connection:info("accepting STOMP connection ~tp (~ts)",
82-
[self(), ConnName]),
83-
84-
ParseState = rabbit_stomp_frame:initial_state(),
85-
_ = register_resource_alarm(),
86-
87-
LoginTimeout = application:get_env(rabbitmq_stomp, login_timeout, 10_000),
88-
MaxFrameSize = application:get_env(rabbitmq_stomp, max_frame_size, ?DEFAULT_MAX_FRAME_SIZE),
89-
erlang:send_after(LoginTimeout, self(), login_timeout),
90-
91-
gen_server2:enter_loop(?MODULE, [],
92-
rabbit_event:init_stats_timer(
93-
run_socket(control_throttle(
94-
#reader_state{socket = RealSocket,
95-
conn_name = ConnName,
96-
parse_state = ParseState,
97-
processor_state = ProcState,
98-
heartbeat_sup = SupHelperPid,
99-
heartbeat = {none, none},
100-
max_frame_size = MaxFrameSize,
101-
current_frame_size = 0,
102-
state = running,
103-
conserve_resources = false,
104-
recv_outstanding = false})), #reader_state.stats_timer),
105-
{backoff, 1000, 1000, 10000});
106-
{error, enotconn} ->
107-
rabbit_net:fast_close(RealSocket),
108-
terminate(shutdown, undefined);
109-
{error, Reason} ->
110-
rabbit_net:fast_close(RealSocket),
111-
terminate({network_error, Reason}, undefined)
112-
end
106+
rabbit_net:fast_close(RealSocket),
107+
terminate({network_error, Reason}, undefined)
113108
end.
114109

110+
115111
handle_call({info, InfoItems}, _From, State) ->
116112
Infos = lists:map(
117113
fun(InfoItem) ->

‎deps/rabbitmq_stream/src/rabbit_stream_reader.erl‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,10 @@ init([KeepaliveSup,
136136
heartbeat := Heartbeat,
137137
transport := ConnTransport}]) ->
138138
process_flag(trap_exit, true),
139-
ProxyProtocolEnabled =
140-
application:get_env(rabbitmq_stream, proxy_protocol, false),
141-
%% Note:
142-
%% This function could return an error if the handshake times out.
143-
%% It is less likely to happen here as compared to MQTT, so
144-
%% crashing with a `badmatch` seems appropriate.
145-
{ok, Sock} = rabbit_networking:handshake(Ref, ProxyProtocolEnabled),
139+
{ok, Sock} =
140+
rabbit_networking:handshake(Ref,
141+
application:get_env(rabbitmq_stream,
142+
proxy_protocol, false)),
146143
RealSocket = rabbit_net:unwrap_socket(Sock),
147144
case rabbit_net:connection_string(Sock, inbound) of
148145
{ok, ConnStr} ->

0 commit comments

Comments
 (0)