diff --git a/superset/databases/api.py b/superset/databases/api.py index b61b1f7881079..07253a5548eb1 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -75,10 +75,12 @@ from superset.databases.utils import get_table_metadata from superset.db_engine_specs import get_available_engine_specs from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.exceptions import SupersetErrorsException from superset.extensions import security_manager from superset.models.core import Database from superset.superset_typing import FlaskResponse from superset.utils.core import error_msg_from_exception, parse_js_uri_path_item +from superset.views.base import json_errors_response from superset.views.base_api import ( BaseSupersetModelRestApi, requires_form_data, @@ -224,7 +226,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): log_to_statsd=False, ) @requires_json - def post(self) -> Response: + def post(self) -> FlaskResponse: """Creates a new Database --- post: @@ -281,6 +283,8 @@ def post(self) -> Response: return self.response_422(message=ex.normalized_messages()) except DatabaseConnectionFailedError as ex: return self.response_422(message=str(ex)) + except SupersetErrorsException as ex: + return json_errors_response(errors=ex.errors, status=ex.status) except DatabaseCreateFailedError as ex: logger.error( "Error creating model %s: %s", diff --git a/superset/databases/commands/create.py b/superset/databases/commands/create.py index 469dda706a800..4dc8e8eda49e0 100644 --- a/superset/databases/commands/create.py +++ b/superset/databases/commands/create.py @@ -31,6 +31,7 @@ ) from superset.databases.commands.test_connection import TestConnectionDatabaseCommand from superset.databases.dao import DatabaseDAO +from superset.exceptions import SupersetErrorsException from superset.extensions import db, event_logger, security_manager logger = logging.getLogger(__name__) @@ -46,6 +47,13 @@ def run(self) -> Model: try: # Test connection before starting create transaction TestConnectionDatabaseCommand(self._properties).run() + except SupersetErrorsException as ex: + event_logger.log_with_context( + action=f"db_creation_failed.{ex.__class__.__name__}", + engine=self._properties.get("sqlalchemy_uri", "").split(":")[0], + ) + # So we can show the original message + raise ex except Exception as ex: event_logger.log_with_context( action=f"db_creation_failed.{ex.__class__.__name__}", diff --git a/superset/databases/commands/test_connection.py b/superset/databases/commands/test_connection.py index d7f7d90e49220..167b5657fbfdc 100644 --- a/superset/databases/commands/test_connection.py +++ b/superset/databases/commands/test_connection.py @@ -29,13 +29,16 @@ from superset.databases.commands.exceptions import ( DatabaseSecurityUnsafeError, DatabaseTestConnectionDriverError, - DatabaseTestConnectionFailedError, DatabaseTestConnectionUnexpectedError, ) from superset.databases.dao import DatabaseDAO from superset.databases.utils import make_url_safe from superset.errors import ErrorLevel, SupersetErrorType -from superset.exceptions import SupersetSecurityException, SupersetTimeoutException +from superset.exceptions import ( + SupersetErrorsException, + SupersetSecurityException, + SupersetTimeoutException, +) from superset.extensions import event_logger from superset.models.core import Database @@ -49,6 +52,7 @@ def __init__(self, data: Dict[str, Any]): def run(self) -> None: # pylint: disable=too-many-statements self.validate() + ex_str = "" uri = self._properties.get("sqlalchemy_uri", "") if self._model and uri == self._model.safe_sqlalchemy_uri(): uri = self._model.sqlalchemy_uri_decrypted @@ -117,10 +121,13 @@ def ping(engine: Engine) -> bool: level=ErrorLevel.ERROR, extra={"sqlalchemy_uri": database.sqlalchemy_uri}, ) from ex - except Exception: # pylint: disable=broad-except + except Exception as ex: # pylint: disable=broad-except alive = False + # So we stop losing the original message if any + ex_str = str(ex) + if not alive: - raise DBAPIError(None, None, None) + raise DBAPIError(ex_str or None, None, None) # Log succesful connection test with engine event_logger.log_with_context( @@ -145,7 +152,7 @@ def ping(engine: Engine) -> bool: ) # check for custom errors (wrong username, wrong password, etc) errors = database.db_engine_spec.extract_errors(ex, context) - raise DatabaseTestConnectionFailedError(errors) from ex + raise SupersetErrorsException(errors) from ex except SupersetSecurityException as ex: event_logger.log_with_context( action=f"test_connection_error.{ex.__class__.__name__}", diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py index 8f59af945d28c..59ed8e1bc9e1f 100644 --- a/superset/db_engine_specs/bigquery.py +++ b/superset/db_engine_specs/bigquery.py @@ -46,8 +46,8 @@ CONNECTION_DATABASE_PERMISSIONS_REGEX = re.compile( - "Access Denied: Project User does not have bigquery.jobs.create " - + "permission in project (?P.+?)" + "Access Denied: Project (?P.+?): User does not have " + + "bigquery.jobs.create permission in project (?P.+?)" ) TABLE_DOES_NOT_EXIST_REGEX = re.compile( @@ -154,9 +154,12 @@ class BigQueryEngineSpec(BaseEngineSpec): custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = { CONNECTION_DATABASE_PERMISSIONS_REGEX: ( __( - "We were unable to connect to your database. Please " - "confirm that your service account has the Viewer " - "and Job User roles on the project." + "Unable to connect. Verify that the following roles are set " + 'on the service account: "BigQuery Data Viewer", ' + '"BigQuery Metadata Viewer", "BigQuery Job User" ' + "and the following permissions are set " + '"bigquery.readsessions.create", ' + '"bigquery.readsessions.getData"' ), SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR, {}, diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py index 4bd80279a0274..9698c7e42a664 100644 --- a/tests/integration_tests/databases/api_tests.py +++ b/tests/integration_tests/databases/api_tests.py @@ -525,11 +525,55 @@ def test_create_database_conn_fail(self): self.login(username="admin") response = self.client.post(uri, json=database_data) response_data = json.loads(response.data.decode("utf-8")) - expected_response = { - "message": "Connection failed, please check your connection settings" + superset_error_mysql = SupersetError( + message='Either the username "superset" or the password is incorrect.', + error_type="CONNECTION_ACCESS_DENIED_ERROR", + level="error", + extra={ + "engine_name": "MySQL", + "invalid": ["username", "password"], + "issue_codes": [ + { + "code": 1014, + "message": ( + "Issue 1014 - Either the username or the password is wrong." + ), + }, + { + "code": 1015, + "message": ( + "Issue 1015 - Issue 1015 - Either the database is spelled incorrectly or does not exist." + ), + }, + ], + }, + ) + superset_error_postgres = SupersetError( + message='The password provided for username "superset" is incorrect.', + error_type="CONNECTION_INVALID_PASSWORD_ERROR", + level="error", + extra={ + "engine_name": "PostgreSQL", + "invalid": ["username", "password"], + "issue_codes": [ + { + "code": 1013, + "message": ( + "Issue 1013 - The password provided when connecting to a database is not valid." + ), + } + ], + }, + ) + expected_response_mysql = {"errors": [dataclasses.asdict(superset_error_mysql)]} + expected_response_postgres = { + "errors": [dataclasses.asdict(superset_error_postgres)] } - self.assertEqual(response.status_code, 422) - self.assertEqual(response_data, expected_response) + self.assertEqual(response.status_code, 500) + if example_db.backend == "mysql": + self.assertEqual(response_data, expected_response_mysql) + else: + self.assertEqual(response_data, expected_response_postgres) def test_update_database(self): """ @@ -1516,7 +1560,7 @@ def test_test_connection_failed_invalid_hostname( url = "api/v1/database/test_connection/" rv = self.post_assert_metric(url, data, "test_connection") - assert rv.status_code == 422 + assert rv.status_code == 500 assert rv.headers["Content-Type"] == "application/json; charset=utf-8" response = json.loads(rv.data.decode("utf-8")) expected_response = {"errors": [dataclasses.asdict(superset_error)]} diff --git a/tests/integration_tests/databases/commands_tests.py b/tests/integration_tests/databases/commands_tests.py index ed8eb43cc788e..4426fa756ff55 100644 --- a/tests/integration_tests/databases/commands_tests.py +++ b/tests/integration_tests/databases/commands_tests.py @@ -32,7 +32,6 @@ DatabaseNotFoundError, DatabaseSecurityUnsafeError, DatabaseTestConnectionDriverError, - DatabaseTestConnectionFailedError, DatabaseTestConnectionUnexpectedError, ) from superset.databases.commands.export import ExportDatabasesCommand @@ -683,7 +682,7 @@ def test_connection_do_ping_exception( json_payload = {"sqlalchemy_uri": db_uri} command_without_db_name = TestConnectionDatabaseCommand(json_payload) - with pytest.raises(DatabaseTestConnectionFailedError) as excinfo: + with pytest.raises(SupersetErrorsException) as excinfo: command_without_db_name.run() assert ( excinfo.value.errors[0].error_type @@ -757,7 +756,7 @@ def test_connection_db_api_exc( json_payload = {"sqlalchemy_uri": db_uri} command_without_db_name = TestConnectionDatabaseCommand(json_payload) - with pytest.raises(DatabaseTestConnectionFailedError) as excinfo: + with pytest.raises(SupersetErrorsException) as excinfo: command_without_db_name.run() assert str(excinfo.value) == ( "Connection failed, please check your connection settings" diff --git a/tests/integration_tests/db_engine_specs/bigquery_tests.py b/tests/integration_tests/db_engine_specs/bigquery_tests.py index 32e3d1f2067e5..1825f9587cc80 100644 --- a/tests/integration_tests/db_engine_specs/bigquery_tests.py +++ b/tests/integration_tests/db_engine_specs/bigquery_tests.py @@ -246,11 +246,11 @@ def test_df_to_sql(self, mock_get_engine): ) def test_extract_errors(self): - msg = "403 POST https://bigquery.googleapis.com/bigquery/v2/projects/test-keel-310804/jobs?prettyPrint=false: Access Denied: Project User does not have bigquery.jobs.create permission in project profound-keel-310804" + msg = "403 POST https://bigquery.googleapis.com/bigquery/v2/projects/test-keel-310804/jobs?prettyPrint=false: Access Denied: Project profound-keel-310804: User does not have bigquery.jobs.create permission in project profound-keel-310804" result = BigQueryEngineSpec.extract_errors(Exception(msg)) assert result == [ SupersetError( - message="We were unable to connect to your database. Please confirm that your service account has the Viewer and Job User roles on the project.", + message='Unable to connect. Verify that the following roles are set on the service account: "BigQuery Data Viewer", "BigQuery Metadata Viewer", "BigQuery Job User" and the following permissions are set "bigquery.readsessions.create", "bigquery.readsessions.getData"', error_type=SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR, level=ErrorLevel.ERROR, extra={