From e78c2c2869d77f246e2d840f1b11d6c0a24911fb Mon Sep 17 00:00:00 2001
From: Stefan Strigler <stefan.strigler@emqx.io>
Date: Mon, 20 Feb 2023 17:07:36 +0100
Subject: [PATCH] fix: return 404 in case node is not found

---
 apps/emqx_management/src/emqx_mgmt_api_trace.erl | 16 ++++++++--------
 .../test/emqx_mgmt_api_trace_SUITE.erl           | 12 ++++++------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt_api_trace.erl b/apps/emqx_management/src/emqx_mgmt_api_trace.erl
index 594750d06..38ce9dcf2 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_trace.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_trace.erl
@@ -51,7 +51,6 @@
 
 -define(TO_BIN(_B_), iolist_to_binary(_B_)).
 -define(NOT_FOUND(N), {404, #{code => 'NOT_FOUND', message => ?TO_BIN([N, " NOT FOUND"])}}).
--define(BAD_REQUEST(C, M), {400, #{code => C, message => ?TO_BIN(M)}}).
 -define(SERVICE_UNAVAILABLE(C, M), {503, #{code => C, message => ?TO_BIN(M)}}).
 -define(TAGS, [<<"Trace">>]).
 
@@ -151,8 +150,9 @@ schema("/trace/:name/download") ->
                                 #{schema => #{type => "string", format => "binary"}}
                         }
                     },
-                400 => emqx_dashboard_swagger:error_codes(['NODE_ERROR'], <<"Node Not Found">>),
-                404 => emqx_dashboard_swagger:error_codes(['NOT_FOUND'], <<"Trace Name Not Found">>)
+                404 => emqx_dashboard_swagger:error_codes(
+                    ['NOT_FOUND', 'NODE_ERROR'], <<"Trace Name or Node Not Found">>
+                )
             }
         }
     };
@@ -188,10 +188,10 @@ schema("/trace/:name/log") ->
                         {meta, fields(bytes) ++ fields(position)}
                     ],
                 400 => emqx_dashboard_swagger:error_codes(
-                    ['BAD_REQUEST', 'NODE_ERROR'], <<"Bad input parameter">>
+                    ['BAD_REQUEST'], <<"Bad input parameter">>
                 ),
                 404 => emqx_dashboard_swagger:error_codes(
-                    ['NOT_FOUND'], <<"Trace Name Not Found">>
+                    ['NOT_FOUND', 'NODE_ERROR'], <<"Trace Name or Node Not Found">>
                 ),
                 503 => emqx_dashboard_swagger:error_codes(
                     ['SERVICE_UNAVAILABLE'], <<"Requested chunk size too big">>
@@ -509,7 +509,7 @@ download_trace_log(get, #{bindings := #{name := Name}, query_string := Query}) -
                     },
                     {200, Headers, {file_binary, ZipName, Binary}};
                 {error, not_found} ->
-                    ?BAD_REQUEST('NODE_ERROR', <<"Node not found">>)
+                    ?NOT_FOUND(<<"Node">>)
             end;
         {error, not_found} ->
             ?NOT_FOUND(Name)
@@ -602,10 +602,10 @@ stream_log_file(get, #{bindings := #{name := Name}, query_string := Query}) ->
                     }),
                     ?SERVICE_UNAVAILABLE('SERVICE_UNAVAILABLE', <<"Requested chunk size too big">>);
                 {badrpc, nodedown} ->
-                    ?BAD_REQUEST('NODE_ERROR', <<"Node not found">>)
+                    ?NOT_FOUND(<<"Node">>)
             end;
         {error, not_found} ->
-            ?BAD_REQUEST('NODE_ERROR', <<"Node not found">>)
+            ?NOT_FOUND(<<"Node">>)
     end.
 
 -spec get_trace_size() -> #{{node(), file:name_all()} => non_neg_integer()}.
diff --git a/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl
index 92b35db99..162d07aaa 100644
--- a/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl
@@ -223,12 +223,12 @@ t_log_file(_Config) ->
         ]},
         zip:table(Binary2)
     ),
-    {error, {_, 400, _}} =
+    {error, {_, 404, _}} =
         request_api(
             get,
-            api_path("trace/test_client_id/download?node=unknonwn_node")
+            api_path("trace/test_client_id/download?node=unknown_node")
         ),
-    {error, {_, 400, _}} =
+    {error, {_, 404, _}} =
         request_api(
             get,
             % known atom but unknown node
@@ -303,12 +303,12 @@ t_stream_log(_Config) ->
     meck:expect(file, read, 2, {error, enomem}),
     {error, {_, 503, _}} = request_api(get, Path),
     meck:unload(file),
-    {error, {_, 400, _}} =
+    {error, {_, 404, _}} =
         request_api(
             get,
-            api_path("trace/test_stream_log/log?node=unknonwn_node")
+            api_path("trace/test_stream_log/log?node=unknown_node")
         ),
-    {error, {_, 400, _}} =
+    {error, {_, 404, _}} =
         request_api(
             get,
             % known atom but not a node