Gracefully handle ignored errors during sync (#45968)

src/metabase/sync.clj

@@ -34,6 +34,26 @@
      [:name       :string]
      [:steps      [:maybe [:sequential sync-util/StepNameWithMetadata]]]]]])
 
+(def ^:private phase->fn
+  {:metadata     sync-metadata/sync-db-metadata!
+   :analyze      analyze/analyze-db!
+   :field-values field-values/update-field-values!})
+
+(defn- scan-phases [scan]
+  (if (not= :full scan)
+    [:metadata]
+    [:metadata :analyze :field-values]))
+
+(defn- do-phase! [database phase]
+  (let [f      (phase->fn phase)
+        result (f database)]
+    (if (instance? Throwable result)
+      ;; do nothing if we're configured to just move on.
+      (when-not sync-util/*log-exceptions-and-continue?*
+        ;; but if we didn't expect any suppressed exceptions, rethrow it
+        (throw result))
+      (assoc result :name (name phase)))))
+
 (mu/defn sync-database! :- SyncDatabaseResults
   "Perform all the different sync operations synchronously for `database`.
 
@@ -49,10 +69,9 @@
     {:keys [scan], :or {scan :full}} :- [:maybe [:map
                                                  [:scan {:optional true} [:maybe [:enum :schema :full]]]]]]
    (sync-util/sync-operation :sync database (format "Sync %s" (sync-util/name-for-logging database))
-     (cond-> [(assoc (sync-metadata/sync-db-metadata! database) :name "metadata")]
-       (= scan :full)
-       (conj (assoc (analyze/analyze-db! database) :name "analyze")
-             (assoc (field-values/update-field-values! database) :name "field-values"))))))
+     (->> (scan-phases scan)
+          (keep (partial do-phase! database))
+          (doall)))))
 
 (mu/defn sync-table!
   "Perform all the different sync operations synchronously for a given `table`. Since often called on a sequence of

test/metabase/sync_test.clj

@@ -262,6 +262,21 @@
                 (is (= (expected-movie-table) movie))
                 (is (= (expected-studio-table) studio)))))))))
 
+(driver/register! ::sync-database-error-test)
+
+(defmethod driver/describe-database ::sync-database-error-test
+  [_driver _database]
+  (throw (Exception. "OOPS!")))
+
+(deftest sync-database!-error-test
+  (testing "Errors in sync-database! should be caught and handled correctly (#45848)"
+    (mt/with-temp [Database db {:engine ::sync-database-error-test}]
+      (binding [sync-util/*log-exceptions-and-continue?* true]
+        (let [results (sync/sync-database! db)]
+          (testing "Skips the metadata step"
+            (is (= ["analyze" "field-values"]
+                   (map :name results)))))))))
+
 ;; !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 ;; !!                                                                                                               !!
 ;; !! HEY! Your tests probably don't belong in this namespace! Put them in one appropriate to the specific part of  !!