Skip to content

Commit

Permalink
rabbit_db_vhost: Bubble up database errors in delete/1
Browse files Browse the repository at this point in the history
We need to bubble up the error through the caller
`rabbit_vhost:delete/2`. The CLI calls `rabbit_vhost:delete/2` and
already handles the `{error, timeout}` but the management UI needs an
update so that an HTTP DELETE returns an error code when the deletion
times out.
  • Loading branch information
the-mikedavis committed Jul 22, 2024
1 parent 2a86dde commit 8399450
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 6 deletions.
7 changes: 4 additions & 3 deletions deps/rabbit/src/rabbit_db_vhost.erl
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,10 @@ with_fun_in_khepri_tx(VHostName, Thunk) ->
%% delete().
%% -------------------------------------------------------------------

-spec delete(VHostName) -> Existed when
-spec delete(VHostName) -> Ret when
VHostName :: vhost:name(),
Existed :: boolean().
Existed :: boolean(),
Ret :: Existed | rabbit_khepri:timeout_error().
%% @doc Deletes a virtual host record from the database.
%%
%% @returns a boolean indicating if the vhost existed or not. It throws an
Expand All @@ -470,7 +471,7 @@ delete_in_khepri(VHostName) ->
case rabbit_khepri:delete_or_fail(Path) of
ok -> true;
{error, {node_not_found, _}} -> false;
_ -> false
{error, _} = Err -> Err
end.

%% -------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion deps/rabbit/src/rabbit_vhost.erl
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,9 @@ delete(VHost, ActingUser) ->
[{name, VHost},
{user_who_performed_action, ActingUser}]);
false ->
{error, {no_such_vhost, VHost}}
{error, {no_such_vhost, VHost}};
{error, _} = Err ->
Err
end,
%% After vhost was deleted from the database, we try to stop vhost
%% supervisors on all the nodes.
Expand Down
18 changes: 16 additions & 2 deletions deps/rabbitmq_management/src/rabbit_mgmt_wm_vhost.erl
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,22 @@ accept_content(ReqData0, Context = #context{user = #user{username = Username}})

delete_resource(ReqData, Context = #context{user = #user{username = Username}}) ->
VHost = id(ReqData),
_ = rabbit_vhost:delete(VHost, Username),
{true, ReqData, Context}.
case rabbit_vhost:delete(VHost, Username) of
ok ->
{true, ReqData, Context};
{error, timeout} ->
rabbit_mgmt_util:internal_server_error(
timeout,
"Timed out waiting for the vhost to be deleted",
ReqData, Context);
{error, E} ->
Reason = iolist_to_binary(
io_lib:format(
"Error occurred while deleting vhost: ~tp",
[E])),
rabbit_mgmt_util:internal_server_error(
Reason, ReqData, Context)
end.

is_authorized(ReqData, Context) ->
rabbit_mgmt_util:is_authorized_admin(ReqData, Context).
Expand Down

0 comments on commit 8399450

Please sign in to comment.