Better error handling when saving TaskHistory fails

src/metabase/models/task_history.clj

 (ns metabase.models.task-history
-  (:require [metabase.models.interface :as i]
+  (:require [clojure.tools.logging :as log]
+            [metabase.models.interface :as i]
             [metabase.util :as u]
             [metabase.util
              [date :as du]
+             [i18n :refer [trs]]
              [schema :as su]]
             [schema.core :as s]
             [toucan
@@ -20,9 +22,9 @@
   ;; the date that task finished, it deletes everything after that. As we continue to add TaskHistory entries, this
   ;; ensures we'll have a good amount of history for debugging/troubleshooting, but not grow too large and fill the
   ;; disk.
-  (when-let  [clean-before-date (db/select-one-field :ended_at TaskHistory {:limit    1
-                                                                            :offset   num-rows-to-keep
-                                                                            :order-by [[:ended_at :desc]]})]
+  (when-let [clean-before-date (db/select-one-field :ended_at TaskHistory {:limit    1
+                                                                           :offset   num-rows-to-keep
+                                                                           :order-by [[:ended_at :desc]]})]
     (db/simple-delete! TaskHistory :ended_at [:<= clean-before-date])))
 
 (u/strict-extend (class TaskHistory)
@@ -36,7 +38,7 @@
 
 (s/defn all
   "Return all TaskHistory entries, applying `limit` and `offset` if not nil"
-  [limit :- (s/maybe su/IntGreaterThanZero)
+  [limit  :- (s/maybe su/IntGreaterThanZero)
    offset :- (s/maybe su/IntGreaterThanOrEqualToZero)]
   (db/select TaskHistory (merge {:order-by [[:ended_at :desc]]}
                                 (when limit
@@ -58,11 +60,14 @@
 (defn- save-task-history! [start-time-ms info]
   (let [end-time-ms (System/currentTimeMillis)
         duration-ms (- end-time-ms start-time-ms)]
-    (db/insert! TaskHistory
-      (assoc info
-        :started_at (du/->Timestamp start-time-ms)
-        :ended_at   (du/->Timestamp end-time-ms)
-        :duration   duration-ms))))
+    (try
+      (db/insert! TaskHistory
+        (assoc info
+          :started_at (du/->Timestamp start-time-ms)
+          :ended_at   (du/->Timestamp end-time-ms)
+          :duration   duration-ms))
+      (catch Throwable e
+        (log/warn e (trs "Error saving task history"))))))
 
 (s/defn do-with-task-history
   "Impl for `with-task-history` macro; see documentation below."
@@ -85,6 +90,7 @@
   "Execute `body`, recording a TaskHistory entry when the task completes; if it failed to complete, records an entry
   containing information about the Exception. `info` should contain at least a name for the task (conventionally
   lisp-cased) as `:task`; see the `TaskHistoryInfo` schema in this namespace for other optional keys.
+
     (with-task-history {:task \"send-pulses\"}
       ...)"
   {:style/indent 1}

src/metabase/sync/util.clj

@@ -411,23 +411,26 @@
   [task-name :- su/NonBlankString
    database  :- i/DatabaseInstance
    {:keys [start-time end-time]} :- SyncOperationOrStepRunMetadata]
-  {:task task-name
-   :db_id (u/get-id database)
+  {:task       task-name
+   :db_id      (u/get-id database)
    :started_at (du/->Timestamp start-time)
-   :ended_at (du/->Timestamp end-time)
-   :duration (du/calculate-duration start-time end-time)})
+   :ended_at   (du/->Timestamp end-time)
+   :duration   (du/calculate-duration start-time end-time)})
 
 (s/defn ^:private store-sync-summary!
   [operation :- s/Str
    database  :- i/DatabaseInstance
    {:keys [steps] :as sync-md} :- SyncOperationMetadata]
-  (db/insert-many! TaskHistory
-    (cons (create-task-history operation database sync-md)
-          (for [[step-name step-info] steps
-                :let [task-details (dissoc step-info :start-time :end-time :log-summary-fn)]]
-            (assoc (create-task-history step-name database step-info)
-              :task_details (when (seq task-details)
-                              task-details))))))
+  (try
+    (db/insert-many! TaskHistory
+      (cons (create-task-history operation database sync-md)
+            (for [[step-name step-info] steps
+                  :let                  [task-details (dissoc step-info :start-time :end-time :log-summary-fn)]]
+              (assoc (create-task-history step-name database step-info)
+                :task_details (when (seq task-details)
+                                task-details)))))
+    (catch Throwable e
+      (log/warn e (trs "Error saving task history")))))
 
 (s/defn run-sync-operation
   "Run `sync-steps` and log a summary message"

src/metabase/task/task_history_cleanup.clj

@@ -9,7 +9,7 @@
             [metabase.util.i18n :refer [trs]]))
 
 (def ^:private history-rows-to-keep
-  "Maximum number of TaskHistory rows. This is not a `const` so that we can redef it in tests"
+  "Maximum number of TaskHistory rows."
   100000)
 
 (defn- task-history-cleanup! []