diff --git a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx
index 9e0ea5935a0b4138b308ba1d027510ac17300d38..3ca8885c245588226e7234bd0d2eb3524aef093c 100644
--- a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx
+++ b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx
@@ -5,6 +5,7 @@ import { connect } from "react-redux";
import { t } from "ttag";
import _ from "underscore";
+import { updateIn } from "icepick";
import title from "metabase/hoc/Title";
@@ -118,6 +119,14 @@ class DatabaseEditApp extends Component {
[addingNewDatabase ? t`Add Database` : database.name],
];
+ const handleSubmit = async database => {
+ try {
+ await this.props.saveDatabase(database);
+ } catch (error) {
+ throw getSubmitError(error);
+ }
+ };
+
return (
<DatabaseEditRoot>
<Breadcrumbs className="py4" crumbs={crumbs} />
@@ -134,7 +143,7 @@ class DatabaseEditApp extends Component {
database={database}
form={Databases.forms.details}
formName={DATABASE_FORM_NAME}
- onSubmit={this.props.saveDatabase}
+ onSubmit={handleSubmit}
submitTitle={addingNewDatabase ? t`Save` : t`Save changes`}
submitButtonComponent={Button}
>
@@ -204,6 +213,16 @@ class DatabaseEditApp extends Component {
}
}
+const getSubmitError = error => {
+ if (_.isObject(error?.data?.errors)) {
+ return updateIn(error, ["data", "errors"], errors => ({
+ details: errors,
+ }));
+ }
+
+ return error;
+};
+
export default _.compose(
connect(mapStateToProps, mapDispatchToProps),
title(({ database }) => database && database.name),
diff --git a/frontend/src/metabase/components/form/CustomForm/CustomFormSubmit.tsx b/frontend/src/metabase/components/form/CustomForm/CustomFormSubmit.tsx
index 27392ffcdcddf5e58b91b9e5c71c4f067e2eee67..52300bff31f666d30dc9150ab4cef312b1108bf3 100644
--- a/frontend/src/metabase/components/form/CustomForm/CustomFormSubmit.tsx
+++ b/frontend/src/metabase/components/form/CustomForm/CustomFormSubmit.tsx
@@ -6,7 +6,7 @@ import ActionButton from "metabase/components/ActionButton";
import { FormLegacyContext, LegacyContextTypes } from "./types";
-interface CustomFormSubmitProps {
+export interface CustomFormSubmitProps {
children: React.ReactNode;
// ActionButton props
diff --git a/frontend/src/metabase/components/form/FormField/FormField.tsx b/frontend/src/metabase/components/form/FormField/FormField.tsx
index 4ca46489ae5f11604430f77be374924246c15046..e0586248812db6b014d1dcb325c0a8ded09e2b88 100644
--- a/frontend/src/metabase/components/form/FormField/FormField.tsx
+++ b/frontend/src/metabase/components/form/FormField/FormField.tsx
@@ -93,8 +93,8 @@ function FormField({
...props,
};
- const shouldHideError = !visited || active;
- const error = shouldHideError ? undefined : errorProp;
+ const shouldShowError = visited && !active;
+ const error = !shouldShowError ? undefined : errorProp;
return (
<FormFieldView
diff --git a/frontend/src/metabase/containers/Form.jsx b/frontend/src/metabase/containers/Form.jsx
index 63d7e69bd15f56c776d77bc0d2e19905e1a9593f..2d7621af14c6a20db3a4e267cb6971b22704e841 100644
--- a/frontend/src/metabase/containers/Form.jsx
+++ b/frontend/src/metabase/containers/Form.jsx
@@ -227,7 +227,10 @@ class Form extends React.Component {
const errorNames = Object.keys(error.data.errors);
const hasUnknownFields = errorNames.some(name => !fieldNames.has(name));
throw {
- _error: hasUnknownFields ? t`An error occurred` : null,
+ _error:
+ error.data?.message ||
+ error.message ||
+ (hasUnknownFields ? t`An error occurred` : null),
...error.data.errors,
};
} else if (error) {
diff --git a/frontend/src/metabase/setup/components/DatabaseStep/DatabaseStep.styled.tsx b/frontend/src/metabase/setup/components/DatabaseStep/DatabaseStep.styled.tsx
index d3fdda13c8402d09944a9f6a656923b8e6feb3aa..bcc4259d61742a7e76a001a2d4d2ae98eae9b5d1 100644
--- a/frontend/src/metabase/setup/components/DatabaseStep/DatabaseStep.styled.tsx
+++ b/frontend/src/metabase/setup/components/DatabaseStep/DatabaseStep.styled.tsx
@@ -29,3 +29,9 @@ export const StepFormGroup = styled.div`
grid-template-columns: repeat(2, 1fr);
gap: 1rem;
`;
+
+export const FormActions = styled.div`
+ display: flex;
+ align-items: center;
+ justify-content: flex-end;
+`;
diff --git a/frontend/src/metabase/setup/components/DatabaseStep/DatabaseStep.tsx b/frontend/src/metabase/setup/components/DatabaseStep/DatabaseStep.tsx
index 1add100567619ab36a971aaa1e989e4d1379d5ab..d18365d7ce4be8cf54fcf8ca6483fe8661bdfa0c 100644
--- a/frontend/src/metabase/setup/components/DatabaseStep/DatabaseStep.tsx
+++ b/frontend/src/metabase/setup/components/DatabaseStep/DatabaseStep.tsx
@@ -2,6 +2,7 @@ import React from "react";
import { t } from "ttag";
import _ from "underscore";
import { updateIn } from "icepick";
+import Button from "metabase/core/components/Button";
import Users from "metabase/entities/users";
import Databases from "metabase/entities/databases";
import DriverWarning from "metabase/containers/DriverWarning";
@@ -14,6 +15,7 @@ import {
StepDescription,
StepFormGroup,
StepButton,
+ FormActions,
} from "./DatabaseStep.styled";
import { FormProps } from "./types";
@@ -78,12 +80,8 @@ const DatabaseStep = ({
engine={engine}
onSubmit={onDatabaseSubmit}
onEngineChange={onEngineChange}
+ onSkip={handleCancel}
/>
- <StepActions>
- <StepButton onClick={handleCancel}>
- {t`I'll add my data later`}
- </StepButton>
- </StepActions>
{isEmailConfigured && (
<SetupSection
title={t`Need help connecting to your data?`}
@@ -101,6 +99,7 @@ interface DatabaseFormProps {
engine?: string;
onSubmit: (database: DatabaseInfo) => void;
onEngineChange: (engine?: string) => void;
+ onSkip: () => void;
}
const DatabaseForm = ({
@@ -108,6 +107,7 @@ const DatabaseForm = ({
engine,
onSubmit,
onEngineChange,
+ onSkip,
}: DatabaseFormProps): JSX.Element => {
const handleSubmit = async (database: DatabaseInfo) => {
try {
@@ -127,14 +127,17 @@ const DatabaseForm = ({
formName="database"
database={database}
onSubmit={handleSubmit}
+ submitTitle={t`Connect database`}
>
{({
Form,
FormField,
- FormFooter,
+ FormSubmit,
+ FormMessage,
formFields,
values,
onChangeField,
+ submitTitle,
}: FormProps) => (
<Form>
<FormField name="engine" onChange={handleEngineChange} />
@@ -145,7 +148,19 @@ const DatabaseForm = ({
{_.reject(formFields, { name: "engine" }).map(({ name }) => (
<FormField key={name} name={name} />
))}
- {engine && <FormFooter submitTitle={t`Next`} />}
+ {engine ? (
+ <FormActions>
+ <FormMessage noPadding />
+ <Button type="button" onClick={onSkip}>{t`Skip`}</Button>
+ <FormSubmit className="ml2">{submitTitle}</FormSubmit>
+ </FormActions>
+ ) : (
+ <StepActions>
+ <StepButton onClick={onSkip}>
+ {t`I'll add my data later`}
+ </StepButton>
+ </StepActions>
+ )}
</Form>
)}
</Databases.Form>
diff --git a/frontend/src/metabase/setup/components/DatabaseStep/types.ts b/frontend/src/metabase/setup/components/DatabaseStep/types.ts
index 13ee3e11d662a36bf7e0ce6e0a2cf0d7001feee4..35ee7b5033c4285664091bf8ab92bd7b03869ba1 100644
--- a/frontend/src/metabase/setup/components/DatabaseStep/types.ts
+++ b/frontend/src/metabase/setup/components/DatabaseStep/types.ts
@@ -1,5 +1,9 @@
import { ComponentType } from "react";
+import { CustomFormMessageProps } from "metabase/components/form/CustomForm/CustomFormMessage";
+import { CustomFormSubmitProps } from "metabase/components/form/CustomForm/CustomFormSubmit";
+import { OptionalFormViewProps } from "metabase/components/form/CustomForm/types";
+
export interface FormField {
name: string;
}
@@ -8,9 +12,15 @@ export interface FormProps {
Form: ComponentType;
FormField: ComponentType<FormFieldProps>;
FormFooter: ComponentType<FormFooterProps>;
+ FormSubmit: React.ComponentType<
+ CustomFormSubmitProps & OptionalFormViewProps
+ >;
+ FormMessage: React.ComponentType<CustomFormMessageProps>;
formFields: FormField[];
values: FormValues;
onChangeField: (field: string, value: unknown) => void;
+ submitTitle: string;
+ error: string;
}
export interface FormValues {
diff --git a/frontend/test/metabase/scenarios/onboarding/setup/setup.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/setup/setup.cy.spec.js
index d8fa167a5fb5a7ec28a447027fa81aaf414de634..c4190ca23fcc21e151f7e664f352e1a330a8c52e 100644
--- a/frontend/test/metabase/scenarios/onboarding/setup/setup.cy.spec.js
+++ b/frontend/test/metabase/scenarios/onboarding/setup/setup.cy.spec.js
@@ -121,14 +121,14 @@ describe("scenarios > setup", () => {
cy.findByText("Show more options").click();
cy.findByText("H2").click();
cy.findByLabelText("Display name").type("Metabase H2");
- cy.findByText("Next")
+ cy.findByText("Connect database")
.closest("button")
.should("be.disabled");
const dbFilename = "frontend/test/__runner__/empty.db";
const dbPath = Cypress.config("fileServerFolder") + "/" + dbFilename;
cy.findByLabelText("Connection String").type(`file:${dbPath}`);
- cy.findByText("Next")
+ cy.findByText("Connect database")
.closest("button")
.should("not.be.disabled")
.click();
diff --git a/frontend/test/metabase/scenarios/smoketest/admin.cy.spec.js b/frontend/test/metabase/scenarios/smoketest/admin.cy.spec.js
index 783fac8be76f2d8e7f9ea5af9294660f5a8f5d28..5fda0455ada90e92a2d49938726d7892e36e55e0 100644
--- a/frontend/test/metabase/scenarios/smoketest/admin.cy.spec.js
+++ b/frontend/test/metabase/scenarios/smoketest/admin.cy.spec.js
@@ -60,7 +60,7 @@ describe("metabase-smoketest > admin", () => {
const dbFilename = "frontend/test/__runner__/empty.db";
const dbPath = Cypress.config("fileServerFolder") + "/" + dbFilename;
cy.findByLabelText("Connection String").type(`file:${dbPath}`);
- cy.findByText("Next").click();
+ cy.findByText("Connect database").click();
// Turns off anonymous data collection
cy.findByLabelText(
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo.clj b/modules/drivers/mongo/src/metabase/driver/mongo.clj
index 43239f9b3435fe473dca0845ddadd715ed07f463..0684cbbb7f4b5a9f9cde109b02f87ef7b58dcf58 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo.clj
@@ -59,25 +59,25 @@
[_ message]
(condp re-matches message
#"^Timed out after \d+ ms while waiting for a server .*$"
- (driver.common/connection-error-messages :cannot-connect-check-host-and-port)
+ :cannot-connect-check-host-and-port
#"^host and port should be specified in host:port format$"
- (driver.common/connection-error-messages :invalid-hostname)
+ :invalid-hostname
#"^Password can not be null when the authentication mechanism is unspecified$"
- (driver.common/connection-error-messages :password-required)
+ :password-required
#"^org.apache.sshd.common.SshException: No more authentication methods available$"
- (driver.common/connection-error-messages :ssh-tunnel-auth-fail)
+ :ssh-tunnel-auth-fail
#"^java.net.ConnectException: Connection refused$"
- (driver.common/connection-error-messages :ssh-tunnel-connection-fail)
+ :ssh-tunnel-connection-fail
#".*javax.net.ssl.SSLHandshakeException: PKIX path building failed.*"
- (driver.common/connection-error-messages :certificate-not-trusted)
+ :certificate-not-trusted
#".*MongoSocketReadException: Prematurely reached end of stream.*"
- (driver.common/connection-error-messages :requires-ssl)
+ :requires-ssl
#".*" ; default
message))
diff --git a/modules/drivers/presto/src/metabase/driver/presto.clj b/modules/drivers/presto/src/metabase/driver/presto.clj
index 4b99ea5b41bfadbabf1c8b3d6935f5cfbdd5acc1..8bebb1abe85e9d316ac269a8b9efc3319452eb48 100644
--- a/modules/drivers/presto/src/metabase/driver/presto.clj
+++ b/modules/drivers/presto/src/metabase/driver/presto.clj
@@ -7,7 +7,6 @@
[clojure.tools.logging :as log]
[medley.core :as m]
[metabase.driver :as driver]
- [metabase.driver.common :as driver.common]
[metabase.driver.presto-common :as presto-common]
[metabase.driver.sql-jdbc.sync.describe-database :as sql-jdbc.describe-database]
[metabase.driver.sql.util :as sql.u]
@@ -249,13 +248,13 @@
[_ message]
(condp re-matches message
#"^java.net.ConnectException: Connection refused.*$"
- (driver.common/connection-error-messages :cannot-connect-check-host-and-port)
+ :cannot-connect-check-host-and-port
#"^clojure.lang.ExceptionInfo: Catalog .* does not exist.*$"
- (driver.common/connection-error-messages :database-name-incorrect)
+ :database-name-incorrect
#"^java.net.UnknownHostException.*$"
- (driver.common/connection-error-messages :invalid-hostname)
+ :invalid-hostname
#".*" ; default
message))
diff --git a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
index 62ea99a08c55aa1f955746e9a5113b830d850c24..9e0aea04082225445cc3ee54583331abee619ca8 100644
--- a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
+++ b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
@@ -35,7 +35,7 @@
(log/spy :error (type message))
(condp re-matches message
#"(?s).*Object does not exist.*$"
- (driver.common/connection-error-messages :database-name-incorrect)
+ :database-name-incorrect
#"(?s).*" ; default - the Snowflake errors have a \n in them
message))
diff --git a/src/metabase/api/database.clj b/src/metabase/api/database.clj
index 567a84ddea4769afe28015a868387c39cebc0f86..6cb115ca8db42c2db0d79894e94f65283386d5b8 100644
--- a/src/metabase/api/database.clj
+++ b/src/metabase/api/database.clj
@@ -499,18 +499,11 @@
;;; ----------------------------------------------- POST /api/database -----------------------------------------------
-(defn- invalid-connection-response [field m]
- ;; work with the new {:field error-message} format but be backwards-compatible with the UI as it exists right now
- {:valid false
- field m
- :message m})
-
(defn test-database-connection
"Try out the connection details for a database and useful error message if connection fails, returns `nil` if
connection succeeds."
- [engine {:keys [host port] :as details}, & {:keys [invalid-response-handler log-exception]
- :or {invalid-response-handler invalid-connection-response
- log-exception true}}]
+ [engine {:keys [host port] :as details}, & {:keys [log-exception]
+ :or {log-exception true}}]
{:pre [(some? engine)]}
(let [engine (keyword engine)
details (assoc details :engine engine)]
@@ -520,22 +513,26 @@
nil
(and host port (u/host-port-up? host port))
- (invalid-response-handler :dbname (tru "Connection to ''{0}:{1}'' successful, but could not connect to DB."
- host port))
+ {:message (tru "Connection to ''{0}:{1}'' successful, but could not connect to DB."
+ host port)}
(and host (u/host-up? host))
- (invalid-response-handler :port (tru "Connection to host ''{0}'' successful, but port {1} is invalid."
- host port))
+ {:message (tru "Connection to host ''{0}'' successful, but port {1} is invalid."
+ host port)
+ :errors {:port (deferred-tru "check your port settings")}}
host
- (invalid-response-handler :host (tru "Host ''{0}'' is not reachable" host))
+ {:message (tru "Host ''{0}'' is not reachable" host)
+ :errors {:host (deferred-tru "check your host settings")}}
:else
- (invalid-response-handler :db (tru "Unable to connect to database.")))
+ {:message (tru "Unable to connect to database.")})
(catch Throwable e
(when (and log-exception (not (some->> e ex-cause ex-data ::driver/can-connect-message?)))
(log/error e (trs "Cannot connect to Database")))
- (invalid-response-handler :dbname (.getMessage e))))))
+ (if (-> e ex-data :message)
+ (ex-data e)
+ {:message (.getMessage e)})))))
;; TODO - Just make `:ssl` a `feature`
(defn- supports-ssl?
@@ -565,7 +562,8 @@
;; Opportunistic SSL
details-with-ssl
;; Try with original parameters
- (test-database-connection engine details)
+ (some-> (test-database-connection engine details)
+ (assoc :valid false))
details)))
(api/defendpoint POST "/"
@@ -612,7 +610,7 @@
api/*current-user-id*
{:database engine, :source :setup})
{:status 400
- :body details-or-error}))))
+ :body (dissoc details-or-error :valid)}))))
(api/defendpoint POST "/validate"
"Validate that we can connect to a database given a set of details."
diff --git a/src/metabase/api/setup.clj b/src/metabase/api/setup.clj
index 8d152ff748c4044f27bd4eb2aa2d063711af24b8..098743ea72ec695b4d1f273d9d08c703e10cd79e 100644
--- a/src/metabase/api/setup.clj
+++ b/src/metabase/api/setup.clj
@@ -172,16 +172,14 @@
[:as {{{:keys [engine details]} :details, token :token} :body}]
{token SetupToken
engine DBEngineString}
- (let [engine (keyword engine)
- invalid-response (fn [field m] {:status 400, :body (if (#{:dbname :port :host} field)
- {:errors {field m}}
- {:message m})})
- error-or-nil (api.database/test-database-connection engine details :invalid-response-handler invalid-response)]
+ (let [engine (keyword engine)
+ error-or-nil (api.database/test-database-connection engine details)]
(when error-or-nil
(snowplow/track-event! ::snowplow/database-connection-failed
nil
{:database engine, :source :setup})
- error-or-nil)))
+ {:status 400
+ :body error-or-nil})))
;;; Admin Checklist
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index ba7ae9d66bfbc28d1ddd94b98119d3bc1d913a36..0a059649b919e0d2b4b56bb3183dc0b2ece28036 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -511,9 +511,10 @@
(defmulti humanize-connection-error-message
"Return a humanized (user-facing) version of an connection error message.
- Generic error messages are provided in [[metabase.driver.common/connection-error-messages]]; return one of these
- whenever possible.
- Error messages can be strings, or localized strings, as returned by [[metabase.util.i18n/trs]] and
+ Generic error messages provided in [[metabase.driver.util/connection-error-messages]]; should be returned
+ as keywords whenever possible. This provides for both unified error messages and categories which let us point
+ users to the erroneous input fields.
+ Error messages can also be strings, or localized strings, as returned by [[metabase.util.i18n/trs]] and
`metabase.util.i18n/tru`."
{:arglists '([this message])}
dispatch-on-initialized-driver
diff --git a/src/metabase/driver/common.clj b/src/metabase/driver/common.clj
index 75e19acc79fed86a3b1931ea30871c061daf0eec..bd70a52c702a694136629e39f020ca018b4d735f 100644
--- a/src/metabase/driver/common.clj
+++ b/src/metabase/driver/common.clj
@@ -17,50 +17,6 @@
org.joda.time.DateTime
org.joda.time.format.DateTimeFormatter))
-(def connection-error-messages
- "Generic error messages that drivers should return in their implementation
- of [[metabase.driver/humanize-connection-error-message]]."
- {:cannot-connect-check-host-and-port
- (str (deferred-tru "Hmm, we couldn''t connect to the database.")
- " "
- (deferred-tru "Make sure your host and port settings are correct"))
-
- :ssh-tunnel-auth-fail
- (str (deferred-tru "We couldn''t connect to the ssh tunnel host.")
- " "
- (deferred-tru "Check the username, password."))
-
- :ssh-tunnel-connection-fail
- (str (deferred-tru "We couldn''t connect to the ssh tunnel host.")
- " "
- (deferred-tru "Check the hostname and port."))
-
- :database-name-incorrect
- (deferred-tru "Looks like the database name is incorrect.")
-
- :invalid-hostname
- (str (deferred-tru "It looks like your host is invalid.")
- " "
- (deferred-tru "Please double-check it and try again."))
-
- :password-incorrect
- (deferred-tru "Looks like your password is incorrect.")
-
- :password-required
- (deferred-tru "Looks like you forgot to enter your password.")
-
- :username-incorrect
- (deferred-tru "Looks like your username is incorrect.")
-
- :username-or-password-incorrect
- (deferred-tru "Looks like the username or password is incorrect.")
-
- :certificate-not-trusted
- (deferred-tru "Server certificate not trusted - did you specify the correct SSL certificate chain?")
-
- :requires-ssl
- (deferred-tru "Server appears to require SSL - please enable SSL above")})
-
;; TODO - we should rename these from `default-*-details` to `default-*-connection-property`
(def default-host-details
diff --git a/src/metabase/driver/h2.clj b/src/metabase/driver/h2.clj
index d6ad5dc56cf16462c4e95fc9bc444bdcbdfb0cd9..10b9807212abf2af241848d63b79fd3e0a523167 100644
--- a/src/metabase/driver/h2.clj
+++ b/src/metabase/driver/h2.clj
@@ -108,13 +108,13 @@
[_ message]
(condp re-matches message
#"^A file path that is implicitly relative to the current working directory is not allowed in the database URL .*$"
- (driver.common/connection-error-messages :cannot-connect-check-host-and-port)
+ :cannot-connect-check-host-and-port
#"^Database .* not found .*$"
- (driver.common/connection-error-messages :cannot-connect-check-host-and-port)
+ :cannot-connect-check-host-and-port
#"^Wrong user name or password .*$"
- (driver.common/connection-error-messages :username-or-password-incorrect)
+ :username-or-password-incorrect
#".*" ; default
message))
diff --git a/src/metabase/driver/mysql.clj b/src/metabase/driver/mysql.clj
index c830f00e50c121869af5d53bdc719672152139d8..3357324c8a89d838cbb3623394527bb208875e40 100644
--- a/src/metabase/driver/mysql.clj
+++ b/src/metabase/driver/mysql.clj
@@ -123,16 +123,16 @@
[_ 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.$"
- (driver.common/connection-error-messages :cannot-connect-check-host-and-port)
+ :cannot-connect-check-host-and-port
#"^Unknown database .*$"
- (driver.common/connection-error-messages :database-name-incorrect)
+ :database-name-incorrect
#"Access denied for user.*$"
- (driver.common/connection-error-messages :username-or-password-incorrect)
+ :username-or-password-incorrect
#"Must specify port after ':' in connection string"
- (driver.common/connection-error-messages :invalid-hostname)
+ :invalid-hostname
#".*" ; default
message))
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index fa58664044c6e29324788951ec53a0f7fcb3defa..93407c9d1a844f537a4b7af1ce61c022cfaa957b 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -71,19 +71,19 @@
[_ message]
(condp re-matches message
#"^FATAL: database \".*\" does not exist$"
- (driver.common/connection-error-messages :database-name-incorrect)
+ :database-name-incorrect
#"^No suitable driver found for.*$"
- (driver.common/connection-error-messages :invalid-hostname)
+ :invalid-hostname
#"^Connection refused. Check that the hostname and port are correct and that the postmaster is accepting TCP/IP connections.$"
- (driver.common/connection-error-messages :cannot-connect-check-host-and-port)
+ :cannot-connect-check-host-and-port
#"^FATAL: role \".*\" does not exist$"
- (driver.common/connection-error-messages :username-incorrect)
+ :username-incorrect
#"^FATAL: password authentication failed for user.*$"
- (driver.common/connection-error-messages :password-incorrect)
+ :password-incorrect
#"^FATAL: .*$" ; all other FATAL messages: strip off the 'FATAL' part, capitalize, and add a period
(let [[_ message] (re-matches #"^FATAL: (.*$)" message)]
diff --git a/src/metabase/driver/util.clj b/src/metabase/driver/util.clj
index 3e20771d0a49278d119c5fab5321562915aafa8d..877639718a6a1f2d1e341f2d767b4dad989a23af 100644
--- a/src/metabase/driver/util.clj
+++ b/src/metabase/driver/util.clj
@@ -11,7 +11,7 @@
[metabase.public-settings.premium-features :as premium-features]
[metabase.query-processor.error-type :as qp.error-type]
[metabase.util :as u]
- [metabase.util.i18n :refer [trs]]
+ [metabase.util.i18n :refer [deferred-tru trs]]
[toucan.db :as db])
(:import java.io.ByteArrayInputStream
[java.security.cert CertificateFactory X509Certificate]
@@ -20,6 +20,75 @@
javax.net.SocketFactory
[javax.net.ssl SSLContext TrustManagerFactory X509TrustManager]))
+(def ^:private connection-error-messages
+ "Generic error messages that drivers should return in their implementation
+ of [[metabase.driver/humanize-connection-error-message]]."
+ {:cannot-connect-check-host-and-port
+ {:message [(deferred-tru "Hmm, we couldn''t connect to the database.")
+ " "
+ (deferred-tru "Make sure your Host and Port settings are correct")]
+ :errors {:host (deferred-tru "check your host settings")
+ :port (deferred-tru "check your port settings")}}
+
+ :ssh-tunnel-auth-fail
+ {:message [(deferred-tru "We couldn''t connect to the SSH tunnel host.")
+ " "
+ (deferred-tru "Check the Username and Password.")]
+ :errors {:tunnel-user (deferred-tru "check your username")
+ :tunnel-pass (deferred-tru "check your password")}}
+
+ :ssh-tunnel-connection-fail
+ {:message [(deferred-tru "We couldn''t connect to the SSH tunnel host.")
+ " "
+ (deferred-tru "Check the Host and Port.")]
+ :errors {:tunnel-host (deferred-tru "check your host settings")
+ :tunnel-port (deferred-tru "check your port settings")}}
+
+ :database-name-incorrect
+ {:message (deferred-tru "Looks like the Database name is incorrect.")
+ :errors {:dbname (deferred-tru "check your database name settings")}}
+
+ :invalid-hostname
+ {:message [(deferred-tru "It looks like your Host is invalid.")
+ " "
+ (deferred-tru "Please double-check it and try again.")]
+ :errors {:host (deferred-tru "check your host settings")}}
+
+ :password-incorrect
+ {:message (deferred-tru "Looks like your Password is incorrect.")
+ :errors {:password (deferred-tru "check your password")}}
+
+ :password-required
+ {:message (deferred-tru "Looks like you forgot to enter your Password.")
+ :errors {:password (deferred-tru "check your password")}}
+
+ :username-incorrect
+ {:message (deferred-tru "Looks like your Username is incorrect.")
+ :errors {:user (deferred-tru "check your username")}}
+
+ :username-or-password-incorrect
+ {:message (deferred-tru "Looks like the Username or Password is incorrect.")
+ :errors {:user (deferred-tru "check your username")
+ :password (deferred-tru "check your password")}}
+
+ :certificate-not-trusted
+ {:message (deferred-tru "Server certificate not trusted - did you specify the correct SSL certificate chain?")}
+
+ :requires-ssl
+ {:message (deferred-tru "Server appears to require SSL - please enable SSL below")
+ :errors {:ssl (deferred-tru "please enable SSL")}}})
+
+(defn- force-tr [text-or-vector]
+ (if (vector? text-or-vector)
+ (apply str text-or-vector)
+ (str text-or-vector)))
+
+(defn- tr-connection-error-messages [error-type-kw]
+ (when-let [message (connection-error-messages error-type-kw)]
+ (cond-> message
+ (contains? message :message) (update :message force-tr)
+ (contains? message :errors) (update :errors update-vals force-tr))))
+
(comment mdb.connection/keep-me) ; used for [[memoize/ttl]]
;; This is normally set via the env var `MB_DB_CONNECTION_TIMEOUT_MS`
@@ -36,6 +105,11 @@
3000
10000))
+(defn- connection-error? [^Throwable throwable]
+ (and (some? throwable)
+ (or (instance? java.net.ConnectException throwable)
+ (recur (.getCause throwable)))))
+
(defn can-connect-with-details?
"Check whether we can connect to a database with `driver` and `details-map` and perform a basic query such as `SELECT
1`. Specify optional param `throw-exceptions` if you want to handle any exceptions thrown yourself (e.g., so you
@@ -52,10 +126,18 @@
;; actually if we are going to `throw-exceptions` we'll rethrow the original but attempt to humanize the message
;; first
(catch Throwable e
- (throw (if-let [message (some->> (.getMessage e)
- (driver/humanize-connection-error-message driver)
- str)]
- (Exception. message e)
+ (throw (if-let [humanized-message (some->> (.getMessage e)
+ (driver/humanize-connection-error-message driver))]
+ (let [error-data (cond
+ (keyword? humanized-message)
+ (tr-connection-error-messages humanized-message)
+
+ (connection-error? e)
+ (tr-connection-error-messages :cannot-connect-check-host-and-port)
+
+ :else
+ {:message humanized-message})]
+ (ex-info (str (:message error-data)) error-data e))
e))))
(try
(can-connect-with-details? driver details-map :throw-exceptions)
diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj
index dfe0e9ae6116b9236e6331e93f795770de039b8c..b1ec229c41dff5849cd7524cad85aaa5512d834c 100644
--- a/test/metabase/api/database_test.clj
+++ b/test/metabase/api/database_test.clj
@@ -174,7 +174,29 @@
:cache_field_values monthly-schedule}})]
(is (:let-user-control-scheduling details))
(is (= "monthly" (-> cache_field_values_schedule u.cron/cron-string->schedule-map :schedule_type)))
- (is (= "monthly" (-> metadata_sync_schedule u.cron/cron-string->schedule-map :schedule_type)))))))
+ (is (= "monthly" (-> metadata_sync_schedule u.cron/cron-string->schedule-map :schedule_type)))))
+ (testing "well known connection errors are reported properly"
+ (let [dbname (mt/random-name)
+ exception (Exception. (format "FATAL: database \"%s\" does not exist" dbname))]
+ (is (= {:errors {:dbname "check your database name settings"},
+ :message "Looks like the Database name is incorrect."}
+ (with-redefs [driver/can-connect? (fn [& _] (throw exception))]
+ (mt/user-http-request :crowberto :post 400 "database"
+ {:name dbname
+ :engine "postgres"
+ :details {:host "localhost", :port 5432
+ :dbname "fakedb", :user "rastacan"}}))))))
+ (testing "unknown connection errors are reported properly"
+ (let [exception (Exception. "Unknown driver message" (java.net.ConnectException. "Failed!"))]
+ (is (= {:errors {:host "check your host settings"
+ :port "check your port settings"}
+ :message "Hmm, we couldn't connect to the database. Make sure your Host and Port settings are correct"}
+ (with-redefs [driver/available? (constantly true)
+ driver/can-connect? (fn [& _] (throw exception))]
+ (mt/user-http-request :crowberto :post 400 "database"
+ {:name (mt/random-name)
+ :engine (u/qualified-name ::test-driver)
+ :details {:db "my_db"}}))))))))
(deftest delete-database-test
(testing "DELETE /api/database/:id"
@@ -199,8 +221,7 @@
update! (fn [expected-status-code]
(mt/user-http-request :crowberto :put expected-status-code (format "database/%d" db-id) updates))]
(testing "Should check that connection details are valid on save"
- (is (= false
- (:valid (update! 400)))))
+ (is (string? (:message (update! 400)))))
(testing "If connection details are valid, we should be able to update the Database"
(with-redefs [driver/can-connect? (constantly true)]
(is (= nil
@@ -772,8 +793,9 @@
(testing "invalid database connection details"
(testing "calling test-connection-details directly"
- (is (= {:dbname "Hmm, we couldn't connect to the database. Make sure your host and port settings are correct"
- :message "Hmm, we couldn't connect to the database. Make sure your host and port settings are correct"
+ (is (= {:errors {:host "check your host settings"
+ :port "check your port settings"}
+ :message "Hmm, we couldn't connect to the database. Make sure your Host and Port settings are correct"
:valid false}
(#'api.database/test-connection-details "h2" {:db "ABC"}))))
diff --git a/test/metabase/api/setup_test.clj b/test/metabase/api/setup_test.clj
index 23203a92df257bd8ec8b6da8a75428d01ebfaf2e..8e097481424407367fe0005f0429e5ac844ba42c 100644
--- a/test/metabase/api/setup_test.clj
+++ b/test/metabase/api/setup_test.clj
@@ -371,7 +371,9 @@
(client/client :post 400 "setup/validate" {:token (setup/setup-token)}))))
(testing "should validate that database connection works"
- (is (= {:errors {:dbname "Hmm, we couldn't connect to the database. Make sure your host and port settings are correct"}}
+ (is (= {:errors {:host "check your host settings"
+ :port "check your port settings"}
+ :message "Hmm, we couldn't connect to the database. Make sure your Host and Port settings are correct"}
(client/client :post 400 "setup/validate" {:token (setup/setup-token)
:details {:engine "h2"
:details {:db "file:///tmp/fake.db"}}}))))