Merge pull request #1151 from metabase/humanize-connection-error-messages

Humanize DB connection error messages 

.dir-locals.el

@@ -18,6 +18,7 @@
                               (catch-api-exceptions 0)
                               (check 1)
                               (checkp 1)
+                              (cond-let 0)
                               (conda 0)
                               (context 2)
                               (create-database-definition 1)

src/metabase/driver.clj

 (ns metabase.driver
   (:require clojure.java.classpath
+            [clojure.string :as s]
             [clojure.tools.logging :as log]
             [medley.core :as m]
             [metabase.db :refer [ins sel upd]]
@@ -108,13 +109,15 @@
   {:pre [(keyword? engine)
          (contains? (set (keys available-drivers)) engine)
          (map? details-map)]}
-  (try
-    (i/can-connect-with-details? (engine->driver engine) details-map)
-    (catch Throwable e
-      (log/error "Failed to connect to database:" (.getMessage e))
-      (when rethrow-exceptions
-        (throw e))
-      false)))
+  (let [driver (engine->driver engine)]
+    (try
+      (i/can-connect-with-details? driver details-map)
+      (catch Throwable e
+        (log/error "Failed to connect to database:" (.getMessage e))
+        (when rethrow-exceptions
+          (let [message (i/humanize-connection-error-message driver (.getMessage e))]
+            (throw (Exception. message))))
+        false))))
 
 (def ^{:arglists '([database])} sync-database!
   "Sync a `Database`, its `Tables`, and `Fields`."

src/metabase/driver/h2.clj

@@ -6,7 +6,7 @@
             [metabase.driver :as driver]
             (metabase.driver [generic-sql :as generic-sql, :refer [GenericSQLIDriverMixin GenericSQLISyncDriverTableFKsMixin
                                                                    GenericSQLISyncDriverFieldAvgLengthMixin GenericSQLISyncDriverFieldPercentUrlsMixin]]
-                             [interface :refer [IDriver ISyncDriverTableFKs ISyncDriverFieldAvgLength ISyncDriverFieldPercentUrls]])
+                             [interface :as i, :refer [IDriver ISyncDriverTableFKs ISyncDriverFieldAvgLength ISyncDriverFieldPercentUrls]])
             (metabase.driver.generic-sql [interface :refer [ISqlDriverDatabaseSpecific]]
                                          [util :refer [funcs]])
             [metabase.models.database :refer [Database]]))
@@ -186,6 +186,20 @@
                              :year    "DATEADD('YEAR',   %d,       NOW())")
                            amount)))
 
+(defn- humanize-connection-error-message [_ message]
+  (condp re-matches message
+    #"^A file path that is implicitly relative to the current working directory is not allowed in the database URL .*$"
+    (i/connection-error-messages :cannot-connect-check-host-and-port)
+
+    #"^Database .* not found .*$"
+    (i/connection-error-messages :cannot-connect-check-host-and-port)
+
+    #"^Wrong user name or password .*$"
+    (i/connection-error-messages :username-or-password-incorrect)
+
+    #".*" ; default
+    message))
+
 
 (defrecord H2Driver [])
 
@@ -196,7 +210,9 @@
                                :date-interval                       date-interval
                                :unix-timestamp->timestamp           unix-timestamp->timestamp}
   ;; Override the generic SQL implementation of wrap-process-query-middleware so we can block unsafe native queries (see above)
-  IDriver                     (assoc GenericSQLIDriverMixin :wrap-process-query-middleware wrap-process-query-middleware)
+  IDriver                     (assoc GenericSQLIDriverMixin
+                                     :humanize-connection-error-message humanize-connection-error-message
+                                     :wrap-process-query-middleware     wrap-process-query-middleware)
   ISyncDriverTableFKs         GenericSQLISyncDriverTableFKsMixin
   ISyncDriverFieldAvgLength   GenericSQLISyncDriverFieldAvgLengthMixin
   ISyncDriverFieldPercentUrls GenericSQLISyncDriverFieldPercentUrlsMixin)

src/metabase/driver/interface.clj

@@ -16,6 +16,16 @@
    to scan millions of values at any rate."
   10000)
 
+(def ^:const connection-error-messages
+  "Generic error messages that drivers should return in their implementation of `humanize-connection-error-message`."
+  {:cannot-connect-check-host-and-port "Hmm, we couldn't connect to the database. Make sure your host and port settings are correct."
+   :database-name-incorrect            "Looks like the database name is incorrect."
+   :invalid-hostname                   "It looks like your host is invalid. Please double-check it and try again."
+   :password-incorrect                 "Looks like your password is incorrect."
+   :password-required                  "Looks like you forgot to enter your password."
+   :username-incorrect                 "Looks like your username is incorrect."
+   :username-or-password-incorrect     "Looks like the username or password is incorrect."})
+
 ;; ## IDriver Protocol
 
 (defprotocol IDriver
@@ -33,10 +43,14 @@
        (can-connect? driver (sel :one Database :id 1))")
   (can-connect-with-details? [this details-map]
     "Check whether we can connect to a database and performa a simple query.
-     Returns true if we can, otherwise returns false or throws an Exception.
+     Returns true if we can, otherwise returns `false` or throws an `Exception`.
 
        (can-connect-with-details? driver {:engine :postgres, :dbname \"book\", ...})")
 
+  (humanize-connection-error-message ^String [this ^String message]
+    "Return a humanized (user-facing) version of an connection error message string.
+     Generic error messages are provided in the constant `connection-error-messages`; return one of these whenever possible.")
+
   ;; Syncing
   (sync-in-context [this database do-sync-fn]
     "This function is basically around-advice for `sync-database!` and `sync-table!` operations.

src/metabase/driver/mongo.clj

@@ -12,12 +12,13 @@
                     [db :as mdb]
                     [query :as mq])
             [metabase.driver :as driver]
-            [metabase.driver.interface :refer :all]
+            [metabase.driver.interface :as i, :refer [IDriver ISyncDriverFieldNestedFields]]
             (metabase.driver.mongo [query-processor :as qp]
                                    [util :refer [*mongo-connection* with-mongo-connection values->base-type]])
             [metabase.util :as u]))
 
-(declare driver)
+(declare driver
+         field-values-lazy-seq)
 
 ;;; ### Driver Helper Fns
 
@@ -26,7 +27,7 @@
   [table]
   (with-mongo-connection [^com.mongodb.DB conn @(:db table)]
     (->> (mc/find-maps conn (:name table))
-         (take max-sync-lazy-seq-results)
+         (take i/max-sync-lazy-seq-results)
          (map keys)
          (map set)
          (reduce set/union))))
@@ -42,81 +43,103 @@
 
 ;;; ## MongoDriver
 
-(defrecord MongoDriver []
-  IDriver
-;;; ### Connection
-  (can-connect? [_ database]
-    (with-mongo-connection [^com.mongodb.DB conn database]
-      (= (-> (cmd/db-stats conn)
-             (conv/from-db-object :keywordize)
-             :ok)
-         1.0)))
+(defn- can-connect? [_ database]
+  (with-mongo-connection [^com.mongodb.DB conn database]
+    (= (-> (cmd/db-stats conn)
+           (conv/from-db-object :keywordize)
+           :ok)
+       1.0)))
 
-  (can-connect-with-details? [this details]
-    (can-connect? this {:details details}))
+(defn- can-connect-with-details? [this details]
+  (can-connect? this {:details details}))
 
-;;; ### QP
-  (wrap-process-query-middleware [_ qp]
-    (fn [query]
-      (with-mongo-connection [^com.mongodb.DB conn (:database query)]
-        (qp query))))
+(defn- humanize-connection-error-message [_ message]
+  (condp re-matches message
+    #"^Timed out after \d+ ms while waiting for a server .*$"
+    (i/connection-error-messages :cannot-connect-check-host-and-port)
 
-  (process-query [_ query]
-    (qp/process-and-run query))
+    #"^host and port should be specified in host:port format$"
+    (i/connection-error-messages :invalid-hostname)
+
+    #"^Password can not be null when the authentication mechanism is unspecified$"
+    (i/connection-error-messages :password-required)
+
+    #".*" ; default
+    message))
+
+(defn- wrap-process-query-middleware [_ qp]
+  (fn [query]
+    (with-mongo-connection [^com.mongodb.DB conn (:database query)]
+      (qp query))))
+
+(defn- process-query [_ query]
+  (qp/process-and-run query))
 
 ;;; ### Syncing
-  (sync-in-context [_ database do-sync-fn]
-      (with-mongo-connection [_ database]
-        (do-sync-fn)))
-
-  (active-table-names [_ database]
-    (with-mongo-connection [^com.mongodb.DB conn database]
-      (-> (mdb/get-collection-names conn)
-          (set/difference #{"system.indexes"}))))
-
-  (active-column-names->type [_ table]
-    (with-mongo-connection [_ @(:db table)]
-      (into {} (for [column-name (table->column-names table)]
-                 {(name column-name)
-                  (field->base-type {:name                      (name column-name)
-                                     :table                     (delay table)
-                                     :qualified-name-components (delay [(:name table) (name column-name)])})}))))
-
-  (table-pks [_ _]
-    #{"_id"})
-
-  (field-values-lazy-seq [_ {:keys [qualified-name-components table], :as field}]
-    (assert (and (map? field)
-                 (delay? qualified-name-components)
-                 (delay? table))
-            (format "Field is missing required information:\n%s" (u/pprint-to-str 'red field)))
-    (lazy-seq
-     (assert *mongo-connection*
-             "You must have an open Mongo connection in order to get lazy results with field-values-lazy-seq.")
-     (let [table           @table
-           name-components (rest @qualified-name-components)]
-       (assert (seq name-components))
-       (map #(get-in % (map keyword name-components))
-            (mq/with-collection *mongo-connection* (:name table)
-              (mq/fields [(apply str (interpose "." name-components))]))))))
-
-  ISyncDriverFieldNestedFields
-  (active-nested-field-name->type [this field]
-    ;; Build a map of nested-field-key -> type -> count
-    ;; TODO - using an atom isn't the *fastest* thing in the world (but is the easiest); consider alternate implementation
-    (let [field->type->count (atom {})]
-      (doseq [val (take max-sync-lazy-seq-results (field-values-lazy-seq this field))]
-        (when (map? val)
-          (doseq [[k v] val]
-            (swap! field->type->count update-in [k (type v)] #(if % (inc %) 1)))))
-      ;; (seq types) will give us a seq of pairs like [java.lang.String 500]
-      (->> @field->type->count
-           (m/map-vals (fn [type->count]
-                         (->> (seq type->count)                 ; convert to pairs of [type count]
-                              (sort-by second)                  ; source by count
-                              last                              ; take last item (highest count)
-                              first                             ; keep just the type
-                              driver/class->base-type)))))))    ; get corresponding Field base_type
+(defn- sync-in-context [_ database do-sync-fn]
+  (with-mongo-connection [_ database]
+    (do-sync-fn)))
+
+(defn- active-table-names [_ database]
+  (with-mongo-connection [^com.mongodb.DB conn database]
+    (-> (mdb/get-collection-names conn)
+        (set/difference #{"system.indexes"}))))
+
+(defn- active-column-names->type [_ table]
+  (with-mongo-connection [_ @(:db table)]
+    (into {} (for [column-name (table->column-names table)]
+               {(name column-name)
+                (field->base-type {:name                      (name column-name)
+                                   :table                     (delay table)
+                                   :qualified-name-components (delay [(:name table) (name column-name)])})}))))
+
+(defn- field-values-lazy-seq [_ {:keys [qualified-name-components table], :as field}]
+  (assert (and (map? field)
+               (delay? qualified-name-components)
+               (delay? table))
+    (format "Field is missing required information:\n%s" (u/pprint-to-str 'red field)))
+  (lazy-seq
+   (assert *mongo-connection*
+     "You must have an open Mongo connection in order to get lazy results with field-values-lazy-seq.")
+   (let [table           @table
+         name-components (rest @qualified-name-components)]
+     (assert (seq name-components))
+     (map #(get-in % (map keyword name-components))
+          (mq/with-collection *mongo-connection* (:name table)
+            (mq/fields [(apply str (interpose "." name-components))]))))))
+
+(defn- active-nested-field-name->type [this field]
+  ;; Build a map of nested-field-key -> type -> count
+  ;; TODO - using an atom isn't the *fastest* thing in the world (but is the easiest); consider alternate implementation
+  (let [field->type->count (atom {})]
+    (doseq [val (take i/max-sync-lazy-seq-results (field-values-lazy-seq this field))]
+      (when (map? val)
+        (doseq [[k v] val]
+          (swap! field->type->count update-in [k (type v)] #(if % (inc %) 1)))))
+    ;; (seq types) will give us a seq of pairs like [java.lang.String 500]
+    (->> @field->type->count
+         (m/map-vals (fn [type->count]
+                       (->> (seq type->count)             ; convert to pairs of [type count]
+                            (sort-by second)              ; source by count
+                            last                          ; take last item (highest count)
+                            first                         ; keep just the type
+                            driver/class->base-type))))))
+
+
+(defrecord MongoDriver [])
+
+(extend MongoDriver
+  IDriver                      {:can-connect?                      can-connect?
+                                :can-connect-with-details?         can-connect-with-details?
+                                :humanize-connection-error-message humanize-connection-error-message
+                                :wrap-process-query-middleware     wrap-process-query-middleware
+                                :process-query                     process-query
+                                :sync-in-context                   sync-in-context
+                                :active-table-names                active-table-names
+                                :active-column-names->type         active-column-names->type
+                                :table-pks                         (constantly #{"_id"})
+                                :field-values-lazy-seq             field-values-lazy-seq}
+  ISyncDriverFieldNestedFields {:active-nested-field-name->type active-nested-field-name->type})
 
 (def driver
   "Concrete instance of the MongoDB driver."

src/metabase/driver/mysql.clj

@@ -8,8 +8,8 @@
                        [utils :as utils])
             (metabase.driver [generic-sql :as generic-sql, :refer [GenericSQLIDriverMixin GenericSQLISyncDriverTableFKsMixin
                                                                    GenericSQLISyncDriverFieldAvgLengthMixin GenericSQLISyncDriverFieldPercentUrlsMixin]]
-                             [interface :refer [IDriver ISyncDriverTableFKs ISyncDriverFieldAvgLength ISyncDriverFieldPercentUrls
-                                                ISyncDriverSpecificSyncField driver-specific-sync-field!]])
+                             [interface :as i, :refer [IDriver ISyncDriverTableFKs ISyncDriverFieldAvgLength ISyncDriverFieldPercentUrls
+                                                       ISyncDriverSpecificSyncField driver-specific-sync-field!]])
             (metabase.driver.generic-sql [interface :refer [ISqlDriverDatabaseSpecific]]
                                          [util :refer [funcs]])))
 
@@ -132,6 +132,23 @@
                              :year    "DATE_ADD(NOW(), INTERVAL %d YEAR)")
                            amount)))
 
+(defn- humanize-connection-error-message [_ message]
+  (condp re-matches message
+        #"^Communications link failure\s+The last packet sent successfully to the server was 0 milliseconds ago. The driver has not received any packets from the server.$"
+        (i/connection-error-messages :cannot-connect-check-host-and-port)
+
+        #"^Unknown database .*$"
+        (i/connection-error-messages :database-name-incorrect)
+
+        #"Access denied for user.*$"
+        (i/connection-error-messages :username-or-password-incorrect)
+
+        #"Must specify port after ':' in connection string"
+        (i/connection-error-messages :invalid-hostname)
+
+        #".*" ; default
+        message))
+
 (defrecord MySQLDriver [])
 
 (extend MySQLDriver
@@ -141,7 +158,8 @@
                                :date                                date
                                :date-interval                       date-interval
                                :timezone->set-timezone-sql          timezone->set-timezone-sql}
-  IDriver                     GenericSQLIDriverMixin
+  IDriver                     (assoc GenericSQLIDriverMixin
+                                     :humanize-connection-error-message humanize-connection-error-message)
   ISyncDriverTableFKs         GenericSQLISyncDriverTableFKsMixin
   ISyncDriverFieldAvgLength   GenericSQLISyncDriverFieldAvgLengthMixin
   ISyncDriverFieldPercentUrls GenericSQLISyncDriverFieldPercentUrlsMixin)

src/metabase/driver/postgres.clj

@@ -12,8 +12,8 @@
             [metabase.driver :as driver]
             (metabase.driver [generic-sql :as generic-sql, :refer [GenericSQLIDriverMixin GenericSQLISyncDriverTableFKsMixin
                                                                    GenericSQLISyncDriverFieldAvgLengthMixin GenericSQLISyncDriverFieldPercentUrlsMixin]]
-                             [interface :refer [IDriver ISyncDriverTableFKs ISyncDriverFieldAvgLength ISyncDriverFieldPercentUrls
-                                                ISyncDriverSpecificSyncField]])
+                             [interface :as i, :refer [IDriver ISyncDriverTableFKs ISyncDriverFieldAvgLength ISyncDriverFieldPercentUrls
+                                                       ISyncDriverSpecificSyncField]])
             [metabase.driver.generic-sql :as generic-sql]
             (metabase.driver.generic-sql [interface :refer [ISqlDriverDatabaseSpecific]]
                                          [util :refer [with-jdbc-metadata]])))
@@ -153,6 +153,30 @@
                              :year    "(NOW() + INTERVAL '%d year')")
                            amount)))
 
+(defn- humanize-connection-error-message [_ message]
+  (condp re-matches message
+    #"^FATAL: database \".*\" does not exist$"
+    (i/connection-error-messages :database-name-incorrect)
+
+    #"^No suitable driver found for.*$"
+    (i/connection-error-messages :invalid-hostname)
+
+    #"^Connection refused. Check that the hostname and port are correct and that the postmaster is accepting TCP/IP connections.$"
+    (i/connection-error-messages :cannot-connect-check-host-and-port)
+
+    #"^FATAL: role \".*\" does not exist$"
+    (i/connection-error-messages :username-incorrect)
+
+    #"^FATAL: password authentication failed for user.*$"
+    (i/connection-error-messages :password-incorrect)
+
+    #"^FATAL: .*$" ; all other FATAL messages: strip off the 'FATAL' part, capitalize, and add a period
+    (let [[_ message] (re-matches #"^FATAL: (.*$)" message)]
+      (str (s/capitalize message) \.))
+
+    #".*" ; default
+    message))
+
 (defrecord PostgresDriver [])
 
 (extend PostgresDriver
@@ -163,7 +187,8 @@
                                 :date-interval                       date-interval
                                 :timezone->set-timezone-sql          timezone->set-timezone-sql}
   ISyncDriverSpecificSyncField {:driver-specific-sync-field!         driver-specific-sync-field!}
-  IDriver                      GenericSQLIDriverMixin
+  IDriver                      (assoc GenericSQLIDriverMixin
+                                      :humanize-connection-error-message humanize-connection-error-message)
   ISyncDriverTableFKs          GenericSQLISyncDriverTableFKsMixin
   ISyncDriverFieldAvgLength    GenericSQLISyncDriverFieldAvgLengthMixin
   ISyncDriverFieldPercentUrls  GenericSQLISyncDriverFieldPercentUrlsMixin)