diff --git a/src/metabase/query_processor/middleware/catch_exceptions.clj b/src/metabase/query_processor/middleware/catch_exceptions.clj
index dd28df493fbeb2244743fb0a4fd7d7bc3269a88b..06e67124caa458f5896645e2c75be0077ecde8bf 100644
--- a/src/metabase/query_processor/middleware/catch_exceptions.clj
+++ b/src/metabase/query_processor/middleware/catch_exceptions.clj
@@ -154,9 +154,15 @@
(letfn [(raisef* [e context]
;; format the Exception and return it
(let [formatted-exception (format-exception* query e @extra-info)]
- (log/error (str (trs "Error processing query: {0}" (:error format-exception))
+ (log/error (str (trs "Error processing query: {0}"
+ (or (:error formatted-exception)
+ ;; log in server locale, respond in user locale
+ (trs "Error running query")))
"\n" (u/pprint-to-str formatted-exception)))
- (qp.context/resultf formatted-exception context)))]
+ ;; ensure always a message on the error otherwise FE thinks query was successful. (#23258, #23281)
+ (qp.context/resultf (update formatted-exception
+ :error (fnil identity (trs "Error running query")))
+ context)))]
(try
(qp query rff (assoc context :raisef raisef*))
(catch Throwable e
diff --git a/test/metabase/query_processor/middleware/catch_exceptions_test.clj b/test/metabase/query_processor/middleware/catch_exceptions_test.clj
index f1145a999ac4d0fa4d15b8aca4eca2d76b77f102..f4cc08fd5bb3c088a33c6581e5c0c87de43e6c96 100644
--- a/test/metabase/query_processor/middleware/catch_exceptions_test.clj
+++ b/test/metabase/query_processor/middleware/catch_exceptions_test.clj
@@ -104,37 +104,50 @@
:metadata
(update :stacktrace boolean)))))))
-(deftest include-query-execution-info-test
- (testing "Should include info from QueryExecution if added to the thrown/raised Exception"
+(deftest catch-exceptions-test
+ (testing "include-query-execution-info-test"
+ (testing "Should include info from QueryExecution if added to the thrown/raised Exception"
+ (tu.log/suppress-output
+ (is (= {:status :failed
+ :class java.lang.Exception
+ :error "Something went wrong"
+ :stacktrace true
+ :card_id 300
+ :json_query {}
+ :row_count 0
+ :data {:cols []}
+ :a 100
+ :b 200}
+ (-> (mt/test-qp-middleware catch-exceptions/catch-exceptions
+ {} {} []
+ {:runf (fn [_ _ context]
+ (qp.context/raisef (ex-info "Something went wrong."
+ {:query-execution {:a 100
+ :b 200
+ :card_id 300
+ ;; these keys should all get removed
+ :result_rows 400
+ :hash 500
+ :executor_id 500
+ :dashboard_id 700
+ :pulse_id 800
+ :native 900}}
+ (Exception. "Something went wrong"))
+ context))})
+ :metadata
+ (update :stacktrace boolean)))))))
+ (testing "Should always include :error (#23258, #23281)"
(tu.log/suppress-output
- (is (= {:status :failed
- :class java.lang.Exception
- :error "Something went wrong"
- :stacktrace true
- :card_id 300
- :json_query {}
- :row_count 0
- :data {:cols []}
- :a 100
- :b 200}
- (-> (mt/test-qp-middleware catch-exceptions/catch-exceptions
- {} {} []
- {:runf (fn [_ _ context]
- (qp.context/raisef (ex-info "Something went wrong."
- {:query-execution {:a 100
- :b 200
- :card_id 300
- ;; these keys should all get removed
- :result_rows 400
- :hash 500
- :executor_id 500
- :dashboard_id 700
- :pulse_id 800
- :native 900}}
- (Exception. "Something went wrong"))
- context))})
- :metadata
- (update :stacktrace boolean)))))))
+ (testing "Uses error message if present"
+ (is (= "Something went wrong"
+ (-> (fn [] (throw (Exception. "Something went wrong")))
+ (catch-exceptions)
+ :error))))
+ (testing "Has a default if no message on error"
+ (is (= "Error running query"
+ (-> (fn [] (throw (ex-info nil {})))
+ (catch-exceptions)
+ :error)))))))
(deftest permissions-test
(data/with-temp-copy-of-db