POST /api/meta/dataset returns a 400 w/ error message (instead of 200 w/

nothing) when an error occurs in QP

src/metabase/api/meta/dataset.clj

@@ -5,6 +5,8 @@
             [metabase.driver :as driver]))
 
 (defendpoint POST "/" [:as {:keys [body]}]
-  (driver/dataset-query body {:executed_by *current-user-id*}))
+  (let [{:keys [status] :as response} (driver/dataset-query body {:executed_by *current-user-id*})]
+    {:status (if (= status :completed) 200 400)
+     :body response}))
 
 (define-routes)

src/metabase/driver.clj

@@ -79,10 +79,10 @@
 
   Possible caller-options include:
 
-    :executed_by [int]               (user_id of caller)
-    :saved_query [{}]                (dictionary representing Query model)
-    :synchronously [true|false]      (default true)
-    :cache_result [true|false]       (default false)"
+  :executed_by [int]               (user_id of caller)
+  :saved_query [{}]                (dictionary representing Query model)
+  :synchronously [true|false]      (default true)
+  :cache_result [true|false]       (default false)"
   {:arglists '([query caller-options])}
   [query {:keys [executed_by synchronously saved_query]
           :or {synchronously true}
@@ -135,9 +135,9 @@
 
 (defn query-fail
   "Save QueryExecution state and construct a failed query response"
-  [query-execution msg]
-  (let [updates {:status "failed"
-                 :error msg
+  [query-execution error-message]
+  (let [updates {:status :failed
+                 :error error-message
                  :finished_at (util/new-sql-timestamp)
                  :running_time (- (System/currentTimeMillis) (:start_time_millis query-execution))}]
     ;; record our query execution and format response
@@ -146,7 +146,8 @@
         (merge updates)
         (save-query-execution)
         ;; this is just for the response for clien
-        (assoc :row_count 0
+        (assoc :error error-message
+               :row_count 0
                :data {:rows []
                       :cols []
                       :columns []}))))

src/metabase/driver/generic_sql/native.clj

@@ -47,6 +47,7 @@
                             columns first-row)}})
        (catch java.sql.SQLException e
          {:status :failed
-          :error (->> (.getMessage e)     ; error message comes back like 'Column "ZID" not found; SQL statement: ... [error-code]
-                      (re-find #"^(.*);") ; the user already knows the SQL, and error code is meaningless
-                      second)})))         ; so just return the part of the exception that is relevant
+          :error (or (->> (.getMessage e)     ; error message comes back like 'Column "ZID" not found; SQL statement: ... [error-code]' sometimes
+                          (re-find #"^(.*);") ; the user already knows the SQL, and error code is meaningless
+                          second)             ; so just return the part of the exception that is relevant
+                     (.getMessage e))})))

src/metabase/driver/generic_sql/query_processor.clj

@@ -54,9 +54,10 @@
          (annotate/annotate query))
     (catch java.sql.SQLException e
       {:status :failed
-       :error (->> (.getMessage e)                       ; error message comes back like "Error message ... [status-code]
-                   (re-find  #"(?s)(^.*)\s+\[[\d-]+\]$") ; status code isn't useful and makes unit tests hard to write so strip it off
-                   second)})))                           ; (?s) = Pattern.DOTALL - tell regex `.` to match newline characters as well
+       :error (or (->> (.getMessage e)                       ; error message comes back like "Error message ... [status-code]" sometimes
+                       (re-find  #"(?s)(^.*)\s+\[[\d-]+\]$") ; status code isn't useful and makes unit tests hard to write so strip it off
+                       second)
+                  (.getMessage e))})))                       ; (?s) = Pattern.DOTALL - tell regex `.` to match newline characters as well
 
 
 (defn process-and-run

src/metabase/models/query_execution.clj

@@ -45,17 +45,18 @@
    :result_rows
    :query_id])
 
-(defmethod pre-insert QueryExecution [_ {:keys [json_query] :as query-execution}]
-  (assoc query-execution :json_query (if (string? json_query) json_query
-                                        (json/write-str json_query))))
+(defmethod pre-insert QueryExecution [_ {:keys [status json_query] :as query-execution}]
+  (cond-> (assoc query-execution :json_query (if (string? json_query) json_query
+                                                 (json/write-str json_query)))
+    status (assoc :status (name status))))
 
 (defmethod post-select QueryExecution [_ {:keys [query_id result_rows] :as query-execution}]
   (-> query-execution
-    (realize-json :json_query :result_data)
-    (assoc* :row_count (or result_rows 0) ; sadly we have 2 ways to reference the row count :(
-            :query (delay
-                    (check query_id 500 "Can't get execution: QueryExecution doesn't have a :query_id.")
-                    (sel :one Query :id query_id)))))
+      (realize-json :json_query :result_data)
+      (assoc* :row_count (or result_rows 0) ; sadly we have 2 ways to reference the row count :(
+              :query (delay
+                      (check query_id 500 "Can't get execution: QueryExecution doesn't have a :query_id.")
+                      (sel :one Query :id query_id)))))
 
 
 (defn build-response