From dbfe92c537424f05d748e374bc84b3f302e4a8a3 Mon Sep 17 00:00:00 2001 From: Cam Saul <cammsaul@gmail.com> Date: Fri, 29 Jun 2018 10:28:55 -0700 Subject: [PATCH] Log probably-ignorable SQLException just to be safe :lock: --- src/metabase/driver/mongo.clj | 13 ++++++---- src/metabase/models/setting.clj | 5 +++- src/metabase/util.clj | 44 +-------------------------------- 3 files changed, 13 insertions(+), 49 deletions(-) diff --git a/src/metabase/driver/mongo.clj b/src/metabase/driver/mongo.clj index 5296b2d0db7..45f504ea3bc 100644 --- a/src/metabase/driver/mongo.clj +++ b/src/metabase/driver/mongo.clj @@ -66,14 +66,17 @@ (cond ;; 1. url? (and (string? field-value) - (u/url? field-value)) :type/URL + (u/url? field-value)) + :type/URL + ;; 2. json? (and (string? field-value) (or (.startsWith "{" field-value) - (.startsWith "[" field-value))) (when-let [j (u/try-apply json/parse-string field-value)] - (when (or (map? j) - (sequential? j)) - :type/SerializedJSON)))) + (.startsWith "[" field-value))) + (when-let [j (u/ignore-exceptions (json/parse-string field-value))] + (when (or (map? j) + (sequential? j)) + :type/SerializedJSON)))) (defn- find-nested-fields [field-value nested-fields] (loop [[k & more-keys] (keys field-value) diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index 61c6205e426..8aaa48168e5 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -34,6 +34,7 @@ [core :as core] [string :as str]] [clojure.core.memoize :as memoize] + [clojure.java.jdbc :as jdbc] [clojure.tools.logging :as log] [environ.core :as env] [honeysql.core :as hsql] @@ -126,7 +127,9 @@ ;; and ignore that Exception if one is thrown. (try (db/insert! Setting :key settings-last-updated-key, :value (hx/cast :text (hsql/raw "current_timestamp"))) - (catch java.sql.SQLException _))) + (catch java.sql.SQLException e + ;; go ahead and log the Exception anyway on the off chance that it *wasn't* just a race condition issue + (log/error (tru "Error inserting new Setting:") (with-out-str (jdbc/print-sql-exception-chain e)))))) ;; Now that we updated the value in the DB, go ahead and update our cached value as well, because we know about the ;; changes (swap! cache assoc settings-last-updated-key (db/select-one-field :value Setting :key settings-last-updated-key))) diff --git a/src/metabase/util.clj b/src/metabase/util.clj index 9d39e7ad08d..48984435a1e 100644 --- a/src/metabase/util.clj +++ b/src/metabase/util.clj @@ -4,9 +4,7 @@ [data :as data] [pprint :refer [pprint]] [string :as s]] - [clojure.java - [classpath :as classpath] - [jdbc :as jdbc]] + [clojure.java.classpath :as classpath] [clojure.math.numeric-tower :as math] [clojure.tools.logging :as log] [clojure.tools.namespace.find :as ns-find] @@ -15,7 +13,6 @@ [puppetlabs.i18n.core :as i18n :refer [trs]] [ring.util.codec :as codec]) (:import [java.net InetAddress InetSocketAddress Socket] - java.sql.SQLException [java.text Normalizer Normalizer$Form])) ;; This is the very first log message that will get printed. It's here because this is one of the very first @@ -290,45 +287,6 @@ :when (re-find #"metabase" s)] (s/replace s #"^metabase\." ""))))}) -(defn wrap-try-catch - "Returns a new function that wraps F in a `try-catch`. When an exception is caught, it is logged - with `log/error` and returns `nil`." - ([f] - (wrap-try-catch f nil)) - ([f f-name] - (let [exception-message (if f-name - (format "Caught exception in %s: " f-name) - "Caught exception: ")] - (fn [& args] - (try - (apply f args) - (catch SQLException e - (log/error (format-color 'red "%s\n%s\n%s" - exception-message - (with-out-str (jdbc/print-sql-exception-chain e)) - (pprint-to-str (filtered-stacktrace e))))) - (catch Throwable e - (log/error (format-color 'red "%s %s\n%s" - exception-message - (or (.getMessage e) e) - (pprint-to-str (filtered-stacktrace e)))))))))) - -(defn try-apply - "Like `apply`, but wraps F inside a `try-catch` block and logs exceptions caught. - (This is actaully more flexible than `apply` -- the last argument doesn't have to be - a sequence: - - (try-apply vector :a :b [:c :d]) -> [:a :b :c :d] - (apply vector :a :b [:c :d]) -> [:a :b :c :d] - (try-apply vector :a :b :c :d) -> [:a :b :c :d] - (apply vector :a :b :c :d) -> Not ok - :d is not a sequence - - This allows us to use `try-apply` in more situations than we'd otherwise be able to." - [^clojure.lang.IFn f & args] - (apply (wrap-try-catch f) (concat (butlast args) (if (sequential? (last args)) - (last args) - [(last args)])))) - (defn deref-with-timeout "Call `deref` on a FUTURE and throw an exception if it takes more than TIMEOUT-MS." [futur timeout-ms] -- GitLab