Ensure timezone detection doesn't cause sync to fail [ci drivers]

This commit wraps the timezone detections stuff in some try/catches so
that if there's an issue parsing one of the timestamps from the
database, it doesn't cause the whole sync process to fail.

If a failure happens, it's logged as a warning, the timezone field in
the database is not updated and the sync process proceeds.

Fixes #5869

src/metabase/driver.clj

@@ -196,24 +196,6 @@
   (current-db-time ^DateTime [this ^DatabaseInstance database]
     "Returns the current time and timezone from the perspective of `DATABASE`."))
 
-(defn create-db-time-formatter
-  "Creates a date formatter from `DATE-FORMAT-STR` that will preserve
-  the offset/timezone information. Results of this are threadsafe and
-  can safely be def'd"
-  [date-format-str]
-  (.withOffsetParsed ^DateTimeFormatter (tformat/formatter date-format-str)))
-
-(defn make-current-db-time-fn
-  "Takes a clj-time date formatter `DATE-FORMATTER` and a native query
-  for the current time. Returns a function that executes the query and
-  parses the date returned preserving it's timezone"
-  [date-formatter native-query]
-  (fn [driver database]
-    (some->> (execute-query driver {:database database, :native {:query native-query}})
-             :rows
-             ffirst
-             (tformat/parse date-formatter))))
-
 (def IDriverDefaultsMixin
   "Default implementations of `IDriver` methods marked *OPTIONAL*."
   {:date-interval                     (u/drop-first-arg u/relative-date)
@@ -289,6 +271,39 @@
       (when-not (empty? report-tz)
         report-tz))))
 
+(defn create-db-time-formatter
+  "Creates a date formatter from `DATE-FORMAT-STR` that will preserve
+  the offset/timezone information. Results of this are threadsafe and
+  can safely be def'd"
+  [date-format-str]
+  (.withOffsetParsed ^DateTimeFormatter (tformat/formatter date-format-str)))
+
+(defn make-current-db-time-fn
+  "Takes a clj-time date formatter `DATE-FORMATTER` and a native query
+  for the current time. Returns a function that executes the query and
+  parses the date returned preserving it's timezone"
+  [date-formatter native-query]
+  (fn [driver database]
+    (let [settings (when-let [report-tz (report-timezone-if-supported driver)]
+                     {:settings {:report-timezone report-tz}})
+          time-str (try
+                     (->> (merge settings {:database database, :native {:query native-query}})
+                          (execute-query driver)
+                          :rows
+                          ffirst)
+                     (catch Exception e
+                       (throw
+                        (Exception.
+                         (format "Error querying database '%s' for current time" (:name database)) e))))]
+      (try
+        (when time-str
+          (tformat/parse date-formatter time-str))
+        (catch Exception e
+          (throw
+           (Exception.
+            (format "Unable to parse date string '%s' for database engine '%s'"
+                    time-str (-> database :engine name)) e)))))))
+
 (defn class->base-type
   "Return the `Field.base_type` that corresponds to a given class returned by the DB.
    This is used to infer the types of results that come back from native queries."

src/metabase/sync/sync_metadata/sync_timezone.clj

 (ns metabase.sync.sync-metadata.sync-timezone
-  (:require [metabase.driver :as driver]
+  (:require [clojure.tools.logging :as log]
+            [metabase.driver :as driver]
             [metabase.models.database :refer [Database]]
             [metabase.sync.interface :as i]
             [schema.core :as s]
@@ -13,9 +14,12 @@
   "Query `DATABASE` for it's current time to determine it's
   timezone. Update that timezone if it's different."
   [database :- i/DatabaseInstance]
-  (let [tz-id (some-> database
-                      driver/->driver
-                      (driver/current-db-time database)
-                      extract-time-zone)]
-    (when-not (= tz-id (:timezone database))
-      (db/update! Database (:id database) {:timezone tz-id}))))
+  (try
+    (let [tz-id (some-> database
+                        driver/->driver
+                        (driver/current-db-time database)
+                        extract-time-zone)]
+      (when-not (= tz-id (:timezone database))
+        (db/update! Database (:id database) {:timezone tz-id})))
+    (catch Exception e
+      (log/warn e "Error syncing database timezone"))))

test/metabase/sync/sync_metadata/sync_timezone_test.clj

@@ -2,7 +2,9 @@
   (:require [clj-time.core :as time]
             [metabase.models.database :refer [Database]]
             [metabase.sync.sync-metadata.sync-timezone :as sync-tz]
-            [metabase.test.data :as data]
+            [metabase.test
+             [data :as data]
+             [util :as tu]]
             [metabase.test.data.datasets :as datasets]
             [metabase.util :as u]
             [toucan.db :as db]))
@@ -28,3 +30,18 @@
        (nil? tz-after-update)
        ;; Check that the value was set again after sync
        (boolean (time/time-zone-for-id (db-timezone db)))])))
+
+(datasets/expect-with-engines #{:postgres}
+  ["UTC" "UTC"]
+  (data/dataset test-data
+    (let [db (db/select-one Database [:name "test-data"])]
+      (sync-tz/sync-timezone! db)
+      [(db-timezone db)
+       ;; This call fails as the dates on PostgreSQL return 'AEST'
+       ;; for the time zone name. The exception is logged, but the
+       ;; timezone column should be left alone and processing should
+       ;; continue
+       (tu/with-temporary-setting-values [report-timezone "Australia/Sydney"]
+         (do
+           (sync-tz/sync-timezone! db)
+           (db-timezone db)))])))