Skip to content

Commit

Permalink
Merge pull request #12678 from kjellwinblad/kjell/dynamo_bridge/fix/e…
Browse files Browse the repository at this point in the history
…rror_log/EMQX-11934

fix: return error reason from dynamo connector status check
  • Loading branch information
kjellwinblad committed Mar 13, 2024
2 parents 5c59a7f + c90f4f5 commit 9d21372
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 17 deletions.
30 changes: 25 additions & 5 deletions apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,34 @@ on_batch_query(InstanceId, [{_ChannelId, _} | _] = Query, State) ->
on_batch_query(_InstanceId, Query, _State) ->
{error, {unrecoverable_error, {invalid_request, Query}}}.

on_get_status(_InstanceId, #{pool_name := Pool}) ->
health_check_timeout() ->
15000.

on_get_status(_InstanceId, #{pool_name := Pool} = State) ->
Health = emqx_resource_pool:health_check_workers(
Pool, {emqx_bridge_dynamo_connector_client, is_connected, []}
Pool,
{emqx_bridge_dynamo_connector_client, is_connected, [
emqx_resource_pool:health_check_timeout()
]},
health_check_timeout(),
#{return_values => true}
),
status_result(Health).
case Health of
{error, timeout} ->
{?status_connecting, State, <<"timeout_while_checking_connection">>};
{ok, Results} ->
status_result(Results, State)
end.

status_result(_Status = true) -> ?status_connected;
status_result(_Status = false) -> ?status_connecting.
status_result(Results, State) ->
case lists:filter(fun(Res) -> Res =/= true end, Results) of
[] when Results =:= [] ->
?status_connecting;
[] ->
?status_connected;
[{false, Error} | _] ->
{?status_connecting, State, Error}
end.

%%========================================================================================
%% Helper fns
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
%% API
-export([
start_link/1,
is_connected/1,
is_connected/2,
query/4
]).

Expand All @@ -27,20 +27,15 @@
-export([execute/2]).
-endif.

%% The default timeout for DynamoDB REST API calls is 10 seconds,
%% but this value for `gen_server:call` is 5s,
%% so we should pass the timeout to `gen_server:call`
-define(HEALTH_CHECK_TIMEOUT, 10000).

%%%===================================================================
%%% API
%%%===================================================================
is_connected(Pid) ->
is_connected(Pid, Timeout) ->
try
gen_server:call(Pid, is_connected, ?HEALTH_CHECK_TIMEOUT)
gen_server:call(Pid, is_connected, Timeout)
catch
_:_ ->
false
_:Error ->
{false, Error}
end.

query(Pid, Table, Query, Templates) ->
Expand Down Expand Up @@ -76,8 +71,8 @@ handle_call(is_connected, _From, State) ->
case erlcloud_ddb2:list_tables([{limit, 1}]) of
{ok, _} ->
true;
_ ->
false
Error ->
{false, Error}
end,
{reply, IsConnected, State};
handle_call({query, Table, Query, Templates}, _From, State) ->
Expand Down
1 change: 1 addition & 0 deletions changes/ee/fix-12678.en.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The DynamoDB connector now explicitly reports the error reason upon connection failure. This update addresses the previous limitation where connection failures did not result in any explanation.

0 comments on commit 9d21372

Please sign in to comment.