From 012e0c5f7829110fe655a531246c5a497e312ed3 Mon Sep 17 00:00:00 2001
From: Alexander Solovyov <alexander@solovyov.net>
Date: Wed, 6 Nov 2024 18:18:54 +0200
Subject: [PATCH] [serdes] meaningful API status codes (#49610)
previously import would return 200 w/o regard if it was successful or not
fixes #49602
---
.../metabase_enterprise/serialization/api.clj | 23 +++++++++++++------
.../serialization/api_test.clj | 9 +++++++-
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/api.clj b/enterprise/backend/src/metabase_enterprise/serialization/api.clj
index 6dbd3f566af..c37cd482106 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/api.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/api.clj
@@ -125,13 +125,16 @@
{:additive *additive-logging*})]
(try ; try/catch inside logging to log errors
(log/infof "Serdes import, size %s" size)
- (let [cnt (u.compress/untgz file dst)
+ (let [cnt (try (u.compress/untgz file dst)
+ (catch Exception e
+ (throw (ex-info "Cannot unpack archive" {:status 422} e))))
path (find-serialization-dir dst)]
(when-not path
(throw (ex-info "No source dir detected. Please make sure the serialization files are in the top level dir."
- {:dst (.getPath dst)
- :count cnt
- :files (.listFiles dst)})))
+ {:status 400
+ :dst (.getPath dst)
+ :count cnt
+ :files (.listFiles dst)})))
(log/infof "In total %s entries unpacked, detected source dir: %s" cnt (.getName path))
(serdes/with-cache
(-> (v2.ingest/ingest-yaml (.getPath path))
@@ -142,6 +145,7 @@
(log/error e "Error during serialization")
(log/error (u/strip-error e "Error during serialization"))))))]
{:log-file log-file
+ :status (:status (ex-data @err))
:error-message (when @err
(u/strip-error @err nil))
:report report
@@ -253,6 +257,7 @@
(try
(let [start (System/nanoTime)
{:keys [log-file
+ status
error-message
report
callback]} (unpack&import (:tempfile file)
@@ -271,9 +276,13 @@
:error_count (count (:errors report))
:success (not error-message)
:error_message error-message})
- {:status 200
- :headers {"Content-Type" "text/plain"}
- :body (on-response! log-file callback)})
+ (if error-message
+ {:status (or status 500)
+ :headers {"Content-Type" "text/plain"}
+ :body (on-response! log-file callback)}
+ {:status 200
+ :headers {"Content-Type" "text/plain"}
+ :body (on-response! log-file callback)}))
(finally
(io/delete-file (:tempfile file)))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/api_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/api_test.clj
index 7c506aa00e8..cc306486f8e 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/api_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/api_test.clj
@@ -217,7 +217,7 @@
(assoc :collection_id "DoesNotExist")))))))]
(testing "ERROR /api/ee/serialization/import"
(let [res (binding [api.serialization/*additive-logging* false]
- (mt/user-http-request :crowberto :post 200 "ee/serialization/import"
+ (mt/user-http-request :crowberto :post 500 "ee/serialization/import"
{:request-options {:headers {"content-type" "multipart/form-data"}}}
{:file ba}))
log (slurp (io/input-stream res))]
@@ -262,6 +262,13 @@
"models" "Collection,Dashboard"}
(-> (snowplow-test/pop-event-data-and-user-id!) last :data))))))))
+ (testing "Client error /api/ee/serialization/import"
+ (let [res (mt/user-http-request :crowberto :post 422 "ee/serialization/import"
+ {:request-options {:headers {"content-type" "multipart/form-data"}}}
+ {:file (.getBytes "not an archive" "UTF-8")})
+ log (slurp (io/input-stream res))]
+ (is (re-find #"Cannot unpack archive" log))))
+
(mt/with-dynamic-redefs [serdes/extract-one (extract-one-error (:entity_id card)
(mt/dynamic-value serdes/extract-one))]
(testing "ERROR /api/ee/serialization/export"
--
GitLab