Skip to content

Commit f0f7500

Browse files
mkuratczykmkuratczyk
authored
Revert "Log errors from ranch:handshake" (#12304)
This reverts commit 620fff2. It intoduced a regression in another area - a TCP health check, such as the default (with cluster-operator) readinessProbe, on a TLS-enabled instance would log a `rabbit_reader` crash every few seconds: ``` tls-server-0 rabbitmq 2024-09-13 09:03:13.010115+00:00 [error] <0.999.0> crasher: tls-server-0 rabbitmq 2024-09-13 09:03:13.010115+00:00 [error] <0.999.0> initial call: rabbit_reader:init/3 tls-server-0 rabbitmq 2024-09-13 09:03:13.010115+00:00 [error] <0.999.0> pid: <0.999.0> tls-server-0 rabbitmq 2024-09-13 09:03:13.010115+00:00 [error] <0.999.0> registered_name: [] tls-server-0 rabbitmq 2024-09-13 09:03:13.010115+00:00 [error] <0.999.0> exception error: no match of right hand side value {error, handshake_failed} tls-server-0 rabbitmq 2024-09-13 09:03:13.010115+00:00 [error] <0.999.0> in function rabbit_reader:init/3 (rabbit_reader.erl, line 171) ```
1 parent f78f14a commit f0f7500

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
@@ -547,7 +547,7 @@ failed_to_recv_proxy_header(Ref, Error) ->
547547
end,
548548
rabbit_log:debug(Msg, [Error]),
549549
% The following call will clean up resources then exit
550-
_ = catch ranch:handshake(Ref),
550+
_ = ranch:handshake(Ref),
551551
exit({shutdown, failed_to_recv_proxy_header}).
552552

553553
handshake(Ref, ProxyProtocolEnabled) ->
@@ -559,22 +559,14 @@ handshake(Ref, ProxyProtocolEnabled) ->
559559
{error, protocol_error, Error} ->
560560
failed_to_recv_proxy_header(Ref, Error);
561561
{ok, ProxyInfo} ->
562-
case catch ranch:handshake(Ref) of
563-
{'EXIT', normal} ->
564-
{error, handshake_failed};
565-
{ok, Sock} ->
566-
ok = tune_buffer_size(Sock),
567-
{ok, {rabbit_proxy_socket, Sock, ProxyInfo}}
568-
end
562+
{ok, Sock} = ranch:handshake(Ref),
563+
ok = tune_buffer_size(Sock),
564+
{ok, {rabbit_proxy_socket, Sock, ProxyInfo}}
569565
end;
570566
false ->
571-
case catch ranch:handshake(Ref) of
572-
{'EXIT', normal} ->
573-
{error, handshake_failed};
574-
{ok, Sock} ->
575-
ok = tune_buffer_size(Sock),
576-
{ok, Sock}
577-
end
567+
{ok, Sock} = ranch:handshake(Ref),
568+
ok = tune_buffer_size(Sock),
569+
{ok, Sock}
578570
end.
579571

580572
tune_buffer_size(Sock) ->

‎deps/rabbit/src/rabbit_reader.erl‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,6 @@ shutdown(Pid, Explanation) ->
158158
no_return().
159159
init(Parent, HelperSups, Ref) ->
160160
?LG_PROCESS_TYPE(reader),
161-
%% Note:
162-
%% This function could return an error if the handshake times out.
163-
%% It is less likely to happen here as compared to MQTT, so
164-
%% crashing with a `badmatch` seems appropriate.
165161
{ok, Sock} = rabbit_networking:handshake(Ref,
166162
application:get_env(rabbit, proxy_protocol, false)),
167163
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)