From aa125a50f93d4d0c2f1875bab1d42fea7685d5ba Mon Sep 17 00:00:00 2001
From: crwen <37775582+crwen@users.noreply.github.com>
Date: Mon, 11 Mar 2024 03:38:36 +0000
Subject: [PATCH] refactor: make http api returns non-200 status code (#3473)

* refactor: make http api returns non-200 status code

* recover some code
---
 src/servers/src/http/error_result.rs | 37 ++++++++++++++++++++++++++--
 tests-integration/tests/http.rs      | 15 +++++------
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/src/servers/src/http/error_result.rs b/src/servers/src/http/error_result.rs
index 57a4bd6981..c92042f323 100644
--- a/src/servers/src/http/error_result.rs
+++ b/src/servers/src/http/error_result.rs
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use axum::http::HeaderValue;
+use axum::http::{HeaderValue, StatusCode as HttpStatusCode};
 use axum::response::{IntoResponse, Response};
 use axum::Json;
 use common_error::ext::ErrorExt;
@@ -93,6 +93,39 @@ impl IntoResponse for ErrorResponse {
             &GREPTIME_DB_HEADER_EXECUTION_TIME,
             HeaderValue::from(execution_time),
         );
-        resp
+        let status = StatusCode::from_u32(code).unwrap_or(StatusCode::Unknown);
+        let status_code = match status {
+            StatusCode::Success | StatusCode::Cancelled => HttpStatusCode::OK,
+            StatusCode::Unsupported
+            | StatusCode::InvalidArguments
+            | StatusCode::InvalidSyntax
+            | StatusCode::RequestOutdated
+            | StatusCode::RegionAlreadyExists
+            | StatusCode::TableColumnExists
+            | StatusCode::TableAlreadyExists
+            | StatusCode::RegionNotFound
+            | StatusCode::DatabaseNotFound
+            | StatusCode::TableNotFound
+            | StatusCode::TableColumnNotFound => HttpStatusCode::BAD_REQUEST,
+            StatusCode::PermissionDenied
+            | StatusCode::AuthHeaderNotFound
+            | StatusCode::InvalidAuthHeader
+            | StatusCode::UserNotFound
+            | StatusCode::UnsupportedPasswordType
+            | StatusCode::UserPasswordMismatch
+            | StatusCode::RegionReadonly => HttpStatusCode::UNAUTHORIZED,
+            StatusCode::AccessDenied => HttpStatusCode::FORBIDDEN,
+            StatusCode::Internal
+            | StatusCode::Unexpected
+            | StatusCode::Unknown
+            | StatusCode::RegionNotReady
+            | StatusCode::RegionBusy
+            | StatusCode::RateLimited
+            | StatusCode::StorageUnavailable
+            | StatusCode::RuntimeResourcesExhausted
+            | StatusCode::PlanQuery
+            | StatusCode::EngineExecuteQuery => HttpStatusCode::INTERNAL_SERVER_ERROR,
+        };
+        (status_code, resp).into_response()
     }
 }
diff --git a/tests-integration/tests/http.rs b/tests-integration/tests/http.rs
index 7f59e1dfc6..32feebe161 100644
--- a/tests-integration/tests/http.rs
+++ b/tests-integration/tests/http.rs
@@ -124,7 +124,7 @@ pub async fn test_sql_api(store_type: StorageType) {
     let (app, mut guard) = setup_test_http_app_with_frontend(store_type, "sql_api").await;
     let client = TestClient::new(app);
     let res = client.get("/v1/sql").send().await;
-    assert_eq!(res.status(), StatusCode::OK);
+    assert_eq!(res.status(), StatusCode::BAD_REQUEST);
 
     let body = serde_json::from_str::<ErrorResponse>(&res.text().await).unwrap();
     assert_eq!(body.code(), 1004);
@@ -252,11 +252,12 @@ pub async fn test_sql_api(store_type: StorageType) {
         .get("/v1/sql?sql=select cpu, ts from demo limit 1;select cpu, ts from demo2 where ts > 0;")
         .send()
         .await;
-    assert_eq!(res.status(), StatusCode::OK);
+    assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR);
 
-    let _body = serde_json::from_str::<ErrorResponse>(&res.text().await).unwrap();
+    let body = serde_json::from_str::<ErrorResponse>(&res.text().await).unwrap();
     // TODO(shuiyisong): fix this when return source err msg to client side
-    // assert!(body.error().contains("Table not found"));
+    assert!(body.error().contains("Table not found"));
+    assert_eq!(body.code(), ErrorCode::PlanQuery as u32);
 
     // test database given
     let res = client
@@ -280,7 +281,7 @@ pub async fn test_sql_api(store_type: StorageType) {
         .get("/v1/sql?db=notfound&sql=select cpu, ts from demo limit 1")
         .send()
         .await;
-    assert_eq!(res.status(), StatusCode::OK);
+    assert_eq!(res.status(), StatusCode::BAD_REQUEST);
     let body = serde_json::from_str::<ErrorResponse>(&res.text().await).unwrap();
     assert_eq!(body.code(), ErrorCode::DatabaseNotFound as u32);
 
@@ -306,7 +307,7 @@ pub async fn test_sql_api(store_type: StorageType) {
         .get("/v1/sql?db=notfound2-schema&sql=select cpu, ts from demo limit 1")
         .send()
         .await;
-    assert_eq!(res.status(), StatusCode::OK);
+    assert_eq!(res.status(), StatusCode::BAD_REQUEST);
     let body = serde_json::from_str::<ErrorResponse>(&res.text().await).unwrap();
     assert_eq!(body.code(), ErrorCode::DatabaseNotFound as u32);
 
@@ -315,7 +316,7 @@ pub async fn test_sql_api(store_type: StorageType) {
         .get("/v1/sql?db=greptime-schema&sql=select cpu, ts from demo limit 1")
         .send()
         .await;
-    assert_eq!(res.status(), StatusCode::OK);
+    assert_eq!(res.status(), StatusCode::BAD_REQUEST);
     let body = serde_json::from_str::<ErrorResponse>(&res.text().await).unwrap();
     assert_eq!(body.code(), ErrorCode::DatabaseNotFound as u32);