From c60a93db9c3b883d5a92bbd602b44cb873765238 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 8 Apr 2021 13:24:54 -0700 Subject: [PATCH] feat: add extract_errors to Postgres (#13997) * feat: add extract_errors to Postgres * Add unit tests * Fix lint * Fix unit tests --- .../pages/docs/Miscellaneous/issue_codes.mdx | 9 + .../src/components/ErrorMessage/types.ts | 6 +- superset/connectors/sqla/models.py | 5 +- superset/databases/commands/exceptions.py | 14 +- .../databases/commands/test_connection.py | 57 +------ superset/db_engine_specs/base.py | 30 +++- superset/db_engine_specs/postgres.py | 42 +++++ superset/db_engine_specs/presto.py | 54 +++--- superset/errors.py | 12 +- tests/databases/api_tests.py | 154 +++++------------- tests/db_engine_specs/postgres_tests.py | 93 ++++++++++- 11 files changed, 248 insertions(+), 228 deletions(-) diff --git a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx index cf259ab70..54e255f28 100644 --- a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx +++ b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx @@ -131,3 +131,12 @@ Superset encountered an unexpected error. Someething unexpected happened in the Superset backend. Please reach out to your administrator. + +## Issue 1012 + +``` +The username provided when connecting to a database is not valid. +``` + +The user provided a username that doesn't exist in the database. Please check +that the username is typed correctly and exists in the database. diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index 2ffc65558..dccb29aed 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -28,6 +28,10 @@ export const ErrorTypeEnum = { GENERIC_DB_ENGINE_ERROR: 'GENERIC_DB_ENGINE_ERROR', COLUMN_DOES_NOT_EXIST_ERROR: 'COLUMN_DOES_NOT_EXIST_ERROR', TABLE_DOES_NOT_EXIST_ERROR: 'TABLE_DOES_NOT_EXIST_ERROR', + TEST_CONNECTION_INVALID_USERNAME_ERROR: + 'TEST_CONNECTION_INVALID_USERNAME_ERROR', + TEST_CONNECTION_INVALID_HOSTNAME_ERROR: + 'TEST_CONNECTION_INVALID_HOSTNAME_ERROR', TEST_CONNECTION_PORT_CLOSED_ERROR: 'TEST_CONNECTION_PORT_CLOSED_ERROR', TEST_CONNECTION_HOST_DOWN_ERROR: 'TEST_CONNECTION_HOST_DOWN_ERROR', @@ -48,8 +52,6 @@ export const ErrorTypeEnum = { // Sqllab error MISSING_TEMPLATE_PARAMS_ERROR: 'MISSING_TEMPLATE_PARAMS_ERROR', - TEST_CONNECTION_INVALID_HOSTNAME_ERROR: - 'TEST_CONNECTION_INVALID_HOSTNAME_ERROR', // Generic errors GENERIC_COMMAND_ERROR: 'GENERIC_COMMAND_ERROR', diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 432430cc8..004cb23dd 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import dataclasses import json import logging import re @@ -1456,7 +1457,9 @@ class SqlaTable( # pylint: disable=too-many-public-methods,too-many-instance-at "Query %s on schema %s failed", sql, self.schema, exc_info=True ) db_engine_spec = self.database.db_engine_spec - errors = db_engine_spec.extract_errors(ex) + errors = [ + dataclasses.asdict(error) for error in db_engine_spec.extract_errors(ex) + ] error_message = utils.error_msg_from_exception(ex) return QueryResult( diff --git a/superset/databases/commands/exceptions.py b/superset/databases/commands/exceptions.py index d2105ca96..3205d08af 100644 --- a/superset/databases/commands/exceptions.py +++ b/superset/databases/commands/exceptions.py @@ -25,7 +25,7 @@ from superset.commands.exceptions import ( ImportFailedError, UpdateFailedError, ) -from superset.exceptions import SupersetErrorException +from superset.exceptions import SupersetErrorsException class DatabaseInvalidError(CommandInvalidError): @@ -117,26 +117,22 @@ class DatabaseDeleteFailedReportsExistError(DatabaseDeleteFailedError): message = _("There are associated alerts or reports") -class DatabaseTestConnectionFailedError(CommandException): +class DatabaseTestConnectionFailedError(SupersetErrorsException): status = 422 message = _("Connection failed, please check your connection settings") -class DatabaseSecurityUnsafeError(DatabaseTestConnectionFailedError): +class DatabaseSecurityUnsafeError(CommandInvalidError): message = _("Stopped an unsafe database connection") -class DatabaseTestConnectionDriverError(DatabaseTestConnectionFailedError): +class DatabaseTestConnectionDriverError(CommandInvalidError): message = _("Could not load database driver") -class DatabaseTestConnectionUnexpectedError(DatabaseTestConnectionFailedError): +class DatabaseTestConnectionUnexpectedError(CommandInvalidError): message = _("Unexpected error occurred, please check your logs for details") class DatabaseImportError(ImportFailedError): message = _("Import database failed for an unknown reason") - - -class DatabaseTestConnectionNetworkError(SupersetErrorException): - status = 400 diff --git a/superset/databases/commands/test_connection.py b/superset/databases/commands/test_connection.py index 605ecd498..7c2ce6fcf 100644 --- a/superset/databases/commands/test_connection.py +++ b/superset/databases/commands/test_connection.py @@ -20,7 +20,6 @@ from typing import Any, Dict, Optional from flask_appbuilder.security.sqla.models import User from flask_babel import gettext as _ -from sqlalchemy.engine.url import make_url from sqlalchemy.exc import DBAPIError, NoSuchModuleError from superset.commands.base import BaseCommand @@ -28,15 +27,12 @@ from superset.databases.commands.exceptions import ( DatabaseSecurityUnsafeError, DatabaseTestConnectionDriverError, DatabaseTestConnectionFailedError, - DatabaseTestConnectionNetworkError, DatabaseTestConnectionUnexpectedError, ) from superset.databases.dao import DatabaseDAO -from superset.errors import ErrorLevel, SupersetErrorType from superset.exceptions import SupersetSecurityException from superset.extensions import event_logger from superset.models.core import Database -from superset.utils.network import is_host_up, is_hostname_valid, is_port_open logger = logging.getLogger(__name__) @@ -47,53 +43,6 @@ class TestConnectionDatabaseCommand(BaseCommand): self._properties = data.copy() self._model: Optional[Database] = None - @staticmethod - def _diagnose(uri: str) -> None: - parsed_uri = make_url(uri) - if parsed_uri.host: - if not is_hostname_valid(parsed_uri.host): - raise DatabaseTestConnectionNetworkError( - error_type=SupersetErrorType.TEST_CONNECTION_INVALID_HOSTNAME_ERROR, - message=_( - 'Unable to resolve hostname "%(hostname)s".', - hostname=parsed_uri.host, - ), - level=ErrorLevel.ERROR, - extra={"hostname": parsed_uri.host}, - ) - - if parsed_uri.port: - if not is_port_open(parsed_uri.host, parsed_uri.port): - if is_host_up(parsed_uri.host): - raise DatabaseTestConnectionNetworkError( - error_type=( - SupersetErrorType.TEST_CONNECTION_PORT_CLOSED_ERROR - ), - message=_( - "The host %(host)s is up, but the port %(port)s is " - "closed.", - host=parsed_uri.host, - port=parsed_uri.port, - ), - level=ErrorLevel.ERROR, - extra={ - "hostname": parsed_uri.host, - "port": parsed_uri.port, - }, - ) - - raise DatabaseTestConnectionNetworkError( - error_type=SupersetErrorType.TEST_CONNECTION_HOST_DOWN_ERROR, - message=_( - "The host %(host)s might be down, ond can't be reached on " - "port %(port)s.", - host=parsed_uri.host, - port=parsed_uri.port, - ), - level=ErrorLevel.ERROR, - extra={"hostname": parsed_uri.host, "port": parsed_uri.port,}, - ) - def run(self) -> None: self.validate() uri = self._properties.get("sqlalchemy_uri", "") @@ -136,9 +85,9 @@ class TestConnectionDatabaseCommand(BaseCommand): action=f"test_connection_error.{ex.__class__.__name__}", engine=database.db_engine_spec.__name__, ) - # check if we have connectivity to the host, and if the port is open - self._diagnose(uri) - raise DatabaseTestConnectionFailedError() + # check for custom errors (wrong username, wrong password, etc) + errors = database.db_engine_spec.extract_errors(ex) + raise DatabaseTestConnectionFailedError(errors) except SupersetSecurityException as ex: event_logger.log_with_context( action=f"test_connection_error.{ex.__class__.__name__}", diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index e59bc8465..d92a0eb60 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -15,7 +15,6 @@ # specific language governing permissions and limitations # under the License. # pylint: disable=unused-argument -import dataclasses import hashlib import json import logging @@ -266,6 +265,7 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods max_column_name_length = 0 try_remove_schema_from_table_name = True # pylint: disable=invalid-name run_multiple_statements_as_one = False + custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType]] = {} @classmethod def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]: @@ -746,15 +746,27 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods return utils.error_msg_from_exception(ex) @classmethod - def extract_errors(cls, ex: Exception) -> List[Dict[str, Any]]: + def extract_errors(cls, ex: Exception) -> List[SupersetError]: + raw_message = cls._extract_error_message(ex) + + for regex, (message, error_type) in cls.custom_errors.items(): + match = regex.search(raw_message) + if match: + return [ + SupersetError( + error_type=error_type, + message=message % match.groupdict(), + level=ErrorLevel.ERROR, + extra={"engine_name": cls.engine_name}, + ) + ] + return [ - dataclasses.asdict( - SupersetError( - error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR, - message=cls._extract_error_message(ex), - level=ErrorLevel.ERROR, - extra={"engine_name": cls.engine_name}, - ) + SupersetError( + error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR, + message=cls._extract_error_message(ex), + level=ErrorLevel.ERROR, + extra={"engine_name": cls.engine_name}, ) ] diff --git a/superset/db_engine_specs/postgres.py b/superset/db_engine_specs/postgres.py index 957d5f76c..b3548a0a3 100644 --- a/superset/db_engine_specs/postgres.py +++ b/superset/db_engine_specs/postgres.py @@ -31,12 +31,14 @@ from typing import ( Union, ) +from flask_babel import gettext as __ from pytz import _FixedOffset # type: ignore from sqlalchemy.dialects.postgresql import ARRAY, DOUBLE_PRECISION, ENUM, JSON from sqlalchemy.dialects.postgresql.base import PGInspector from sqlalchemy.types import String, TypeEngine from superset.db_engine_specs.base import BaseEngineSpec +from superset.errors import SupersetErrorType from superset.exceptions import SupersetException from superset.utils import core as utils from superset.utils.core import ColumnSpec, GenericDataType @@ -53,6 +55,24 @@ class FixedOffsetTimezone(_FixedOffset): pass +# Regular expressions to catch custom errors +INVALID_USERNAME_REGEX = re.compile('role "(?P.*?)" does not exist') +INVALID_HOSTNAME_REGEX = re.compile( + 'could not translate host name "(?P.*?)" to address: ' + "nodename nor servname provided, or not known" +) +CONNECTION_PORT_CLOSED_REGEX = re.compile( + r"could not connect to server: Connection refused\s+Is the server " + r'running on host "(?P.*?)" (\(.*?\) )?and accepting\s+TCP/IP ' + r"connections on port (?P.*?)\?" +) +CONNECTION_HOST_DOWN_REGEX = re.compile( + r"could not connect to server: (?P.*?)\s+Is the server running on " + r'host "(?P.*?)" (\(.*?\) )?and accepting\s+TCP/IP ' + r"connections on port (?P.*?)\?" +) + + class PostgresBaseEngineSpec(BaseEngineSpec): """ Abstract class for Postgres 'like' databases """ @@ -71,6 +91,28 @@ class PostgresBaseEngineSpec(BaseEngineSpec): "P1Y": "DATE_TRUNC('year', {col})", } + custom_errors = { + INVALID_USERNAME_REGEX: ( + __('The username "%(username)s" does not exist.'), + SupersetErrorType.TEST_CONNECTION_INVALID_USERNAME_ERROR, + ), + INVALID_HOSTNAME_REGEX: ( + __('The hostname "%(hostname)s" cannot be resolved.'), + SupersetErrorType.TEST_CONNECTION_INVALID_HOSTNAME_ERROR, + ), + CONNECTION_PORT_CLOSED_REGEX: ( + __("Port %(port)s on hostname %(hostname)s refused the connection."), + SupersetErrorType.TEST_CONNECTION_PORT_CLOSED_ERROR, + ), + CONNECTION_HOST_DOWN_REGEX: ( + __( + "The host %(hostname)s might be down, and can't be " + "reached on port %(port)s" + ), + SupersetErrorType.TEST_CONNECTION_HOST_DOWN_ERROR, + ), + } + @classmethod def fetch_data( cls, cursor: Any, limit: Optional[int] = None diff --git a/superset/db_engine_specs/presto.py b/superset/db_engine_specs/presto.py index f9524aa61..bb4eefc6e 100644 --- a/superset/db_engine_specs/presto.py +++ b/superset/db_engine_specs/presto.py @@ -14,7 +14,6 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import dataclasses import logging import re import textwrap @@ -1133,54 +1132,41 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho return database.get_df("SHOW FUNCTIONS")["Function"].tolist() @classmethod - def extract_errors(cls, ex: Exception) -> List[Dict[str, Any]]: + def extract_errors(cls, ex: Exception) -> List[SupersetError]: raw_message = cls._extract_error_message(ex) column_match = re.search(COLUMN_NOT_RESOLVED_ERROR_REGEX, raw_message) if column_match: return [ - dataclasses.asdict( - SupersetError( - error_type=SupersetErrorType.COLUMN_DOES_NOT_EXIST_ERROR, - message=__( - 'We can\'t seem to resolve the column "%(column_name)s" at ' - "line %(location)s.", - column_name=column_match.group(2), - location=column_match.group(1), - ), - level=ErrorLevel.ERROR, - extra={"engine_name": cls.engine_name}, - ) + SupersetError( + error_type=SupersetErrorType.COLUMN_DOES_NOT_EXIST_ERROR, + message=__( + 'We can\'t seem to resolve the column "%(column_name)s" at ' + "line %(location)s.", + column_name=column_match.group(2), + location=column_match.group(1), + ), + level=ErrorLevel.ERROR, + extra={"engine_name": cls.engine_name}, ) ] table_match = re.search(TABLE_DOES_NOT_EXIST_ERROR_REGEX, raw_message) if table_match: return [ - dataclasses.asdict( - SupersetError( - error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, - message=__( - 'The table "%(table_name)s" does not exist. ' - "A valid table must be used to run this query.", - table_name=table_match.group(1), - ), - level=ErrorLevel.ERROR, - extra={"engine_name": cls.engine_name}, - ) - ) - ] - - return [ - dataclasses.asdict( SupersetError( - error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR, - message=cls._extract_error_message(ex), + error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, + message=__( + 'The table "%(table_name)s" does not exist. ' + "A valid table must be used to run this query.", + table_name=table_match.group(1), + ), level=ErrorLevel.ERROR, extra={"engine_name": cls.engine_name}, ) - ) - ] + ] + + return super().extract_errors(ex) @classmethod def is_readonly_query(cls, parsed_query: ParsedQuery) -> bool: diff --git a/superset/errors.py b/superset/errors.py index 13cd419df..9dfbede79 100644 --- a/superset/errors.py +++ b/superset/errors.py @@ -39,6 +39,8 @@ class SupersetErrorType(str, Enum): GENERIC_DB_ENGINE_ERROR = "GENERIC_DB_ENGINE_ERROR" COLUMN_DOES_NOT_EXIST_ERROR = "COLUMN_DOES_NOT_EXIST_ERROR" TABLE_DOES_NOT_EXIST_ERROR = "TABLE_DOES_NOT_EXIST_ERROR" + TEST_CONNECTION_INVALID_USERNAME_ERROR = "TEST_CONNECTION_INVALID_USERNAME_ERROR" + TEST_CONNECTION_INVALID_HOSTNAME_ERROR = "TEST_CONNECTION_INVALID_HOSTNAME_ERROR" TEST_CONNECTION_PORT_CLOSED_ERROR = "TEST_CONNECTION_PORT_CLOSED_ERROR" TEST_CONNECTION_HOST_DOWN_ERROR = "TEST_CONNECTION_HOST_DOWN_ERROR" @@ -58,7 +60,6 @@ class SupersetErrorType(str, Enum): # Sql Lab errors MISSING_TEMPLATE_PARAMS_ERROR = "MISSING_TEMPLATE_PARAMS_ERROR" - TEST_CONNECTION_INVALID_HOSTNAME_ERROR = "TEST_CONNECTION_INVALID_HOSTNAME_ERROR" # Generic errors GENERIC_COMMAND_ERROR = "GENERIC_COMMAND_ERROR" @@ -153,6 +154,15 @@ ERROR_TYPES_TO_ISSUE_CODES_MAPPING = { "message": _("Issue 1011 - Superset encountered an unexpected error."), }, ], + SupersetErrorType.TEST_CONNECTION_INVALID_USERNAME_ERROR: [ + { + "code": 1012, + "message": _( + "Issue 1012 - The username provided when " + "connecting to a database is not valid." + ), + }, + ], } diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py index 174bc29f6..cc360a09b 100644 --- a/tests/databases/api_tests.py +++ b/tests/databases/api_tests.py @@ -17,6 +17,7 @@ # isort:skip_file # pylint: disable=invalid-name, no-self-use, too-many-public-methods, too-many-arguments """Unit tests for Superset""" +import dataclasses import json from io import BytesIO from unittest import mock @@ -27,10 +28,12 @@ import pytest import yaml from sqlalchemy.engine.url import make_url +from sqlalchemy.exc import DBAPIError from sqlalchemy.sql import func from superset import db, security_manager from superset.connectors.sqla.models import SqlaTable +from superset.errors import SupersetError from superset.models.core import Database from superset.models.reports import ReportSchedule, ReportScheduleType from superset.utils.core import get_example_database, get_main_database @@ -895,16 +898,44 @@ class TestDatabaseApi(SupersetTestCase): app.config["PREVENT_UNSAFE_DB_CONNECTIONS"] = False - @mock.patch("superset.databases.commands.test_connection.is_hostname_valid",) - def test_test_connection_failed_invalid_hostname(self, mock_is_hostname_valid): + @mock.patch( + "superset.databases.commands.test_connection.DatabaseDAO.build_db_for_connection_test", + ) + @mock.patch("superset.databases.commands.test_connection.event_logger",) + def test_test_connection_failed_invalid_hostname( + self, mock_event_logger, mock_build_db + ): """ Database API: Test test connection failed due to invalid hostname """ - mock_is_hostname_valid.return_value = False + msg = 'psql: error: could not translate host name "locahost" to address: nodename nor servname provided, or not known' + mock_build_db.return_value.set_sqlalchemy_uri.side_effect = DBAPIError( + msg, None, None + ) + mock_build_db.return_value.db_engine_spec.__name__ = "Some name" + superset_error = SupersetError( + message='Unable to resolve hostname "locahost".', + error_type="TEST_CONNECTION_INVALID_HOSTNAME_ERROR", + level="error", + extra={ + "hostname": "locahost", + "issue_codes": [ + { + "code": 1007, + "message": ( + "Issue 1007 - The hostname provided can't be resolved." + ), + } + ], + }, + ) + mock_build_db.return_value.db_engine_spec.extract_errors.return_value = [ + superset_error + ] self.login("admin") data = { - "sqlalchemy_uri": "postgres://username:password@invalidhostname:12345/db", + "sqlalchemy_uri": "postgres://username:password@locahost:12345/db", "database_name": "examples", "impersonate_user": False, "server_cert": None, @@ -912,121 +943,10 @@ class TestDatabaseApi(SupersetTestCase): url = "api/v1/database/test_connection" rv = self.post_assert_metric(url, data, "test_connection") - assert rv.status_code == 400 + assert rv.status_code == 422 assert rv.headers["Content-Type"] == "application/json; charset=utf-8" response = json.loads(rv.data.decode("utf-8")) - expected_response = { - "errors": [ - { - "message": 'Unable to resolve hostname "invalidhostname".', - "error_type": "TEST_CONNECTION_INVALID_HOSTNAME_ERROR", - "level": "error", - "extra": { - "hostname": "invalidhostname", - "issue_codes": [ - { - "code": 1007, - "message": "Issue 1007 - The hostname provided can't be resolved.", - } - ], - }, - } - ] - } - assert response == expected_response - - @mock.patch("superset.databases.commands.test_connection.is_hostname_valid") - @mock.patch("superset.databases.commands.test_connection.is_port_open") - @mock.patch("superset.databases.commands.test_connection.is_host_up") - def test_test_connection_failed_closed_port( - self, mock_is_host_up, mock_is_port_open, mock_is_hostname_valid - ): - """ - Database API: Test test connection failed due to closed port. - """ - mock_is_hostname_valid.return_value = True - mock_is_port_open.return_value = False - mock_is_host_up.return_value = True - - self.login("admin") - data = { - "sqlalchemy_uri": "postgres://username:password@localhost:12345/db", - "database_name": "examples", - "impersonate_user": False, - "server_cert": None, - } - url = "api/v1/database/test_connection" - rv = self.post_assert_metric(url, data, "test_connection") - - assert rv.status_code == 400 - assert rv.headers["Content-Type"] == "application/json; charset=utf-8" - response = json.loads(rv.data.decode("utf-8")) - expected_response = { - "errors": [ - { - "message": "The host localhost is up, but the port 12345 is closed.", - "error_type": "TEST_CONNECTION_PORT_CLOSED_ERROR", - "level": "error", - "extra": { - "hostname": "localhost", - "port": 12345, - "issue_codes": [ - { - "code": 1008, - "message": "Issue 1008 - The port is closed.", - } - ], - }, - } - ] - } - assert response == expected_response - - @mock.patch("superset.databases.commands.test_connection.is_hostname_valid") - @mock.patch("superset.databases.commands.test_connection.is_port_open") - @mock.patch("superset.databases.commands.test_connection.is_host_up") - def test_test_connection_failed_host_down( - self, mock_is_host_up, mock_is_port_open, mock_is_hostname_valid - ): - """ - Database API: Test test connection failed due to host being down. - """ - mock_is_hostname_valid.return_value = True - mock_is_port_open.return_value = False - mock_is_host_up.return_value = False - - self.login("admin") - data = { - "sqlalchemy_uri": "postgres://username:password@localhost:12345/db", - "database_name": "examples", - "impersonate_user": False, - "server_cert": None, - } - url = "api/v1/database/test_connection" - rv = self.post_assert_metric(url, data, "test_connection") - - assert rv.status_code == 400 - assert rv.headers["Content-Type"] == "application/json; charset=utf-8" - response = json.loads(rv.data.decode("utf-8")) - expected_response = { - "errors": [ - { - "message": "The host localhost might be down, ond can't be reached on port 12345.", - "error_type": "TEST_CONNECTION_HOST_DOWN_ERROR", - "level": "error", - "extra": { - "hostname": "localhost", - "port": 12345, - "issue_codes": [ - { - "code": 1009, - "message": "Issue 1009 - The host might be down, and can't be reached on the provided port.", - } - ], - }, - } - ] - } + expected_response = {"errors": [dataclasses.asdict(superset_error)]} assert response == expected_response @pytest.mark.usefixtures( diff --git a/tests/db_engine_specs/postgres_tests.py b/tests/db_engine_specs/postgres_tests.py index 240677771..813cc498e 100644 --- a/tests/db_engine_specs/postgres_tests.py +++ b/tests/db_engine_specs/postgres_tests.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from textwrap import dedent from unittest import mock from sqlalchemy import column, literal_column @@ -21,6 +22,7 @@ from sqlalchemy.dialects import postgresql from superset.db_engine_specs import engines from superset.db_engine_specs.postgres import PostgresEngineSpec +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from tests.db_engine_specs.base_tests import TestDbEngineSpec from tests.fixtures.certificates import ssl_certificate from tests.fixtures.database import default_db_extra @@ -171,7 +173,9 @@ class TestPostgresDbEngineSpec(TestDbEngineSpec): ) sql = "SELECT * FROM birth_names" results = PostgresEngineSpec.estimate_statement_cost(sql, cursor) - self.assertEqual(results, {"Start-up cost": 0.00, "Total cost": 1537.91,}) + self.assertEqual( + results, {"Start-up cost": 0.00, "Total cost": 1537.91,}, + ) def test_estimate_statement_invalid_syntax(self): """ @@ -207,3 +211,90 @@ class TestPostgresDbEngineSpec(TestDbEngineSpec): {"Start-up cost": "10.0", "Total cost": "1537.0",}, ], ) + + def test_extract_errors(self): + """ + Test that custom error messages are extracted correctly. + """ + msg = 'psql: error: FATAL: role "testuser" does not exist' + result = PostgresEngineSpec.extract_errors(Exception(msg)) + assert result == [ + SupersetError( + error_type=SupersetErrorType.TEST_CONNECTION_INVALID_USERNAME_ERROR, + message='The username "testuser" does not exist.', + level=ErrorLevel.ERROR, + extra={"engine_name": "PostgreSQL"}, + ) + ] + + msg = 'psql: error: could not translate host name "locahost" to address: nodename nor servname provided, or not known' + result = PostgresEngineSpec.extract_errors(Exception(msg)) + assert result == [ + SupersetError( + error_type=SupersetErrorType.TEST_CONNECTION_INVALID_HOSTNAME_ERROR, + message='The hostname "locahost" cannot be resolved.', + level=ErrorLevel.ERROR, + extra={"engine_name": "PostgreSQL"}, + ) + ] + + msg = dedent( + """ +psql: error: could not connect to server: Connection refused + Is the server running on host "localhost" (::1) and accepting + TCP/IP connections on port 12345? +could not connect to server: Connection refused + Is the server running on host "localhost" (127.0.0.1) and accepting + TCP/IP connections on port 12345? + """ + ) + result = PostgresEngineSpec.extract_errors(Exception(msg)) + assert result == [ + SupersetError( + error_type=SupersetErrorType.TEST_CONNECTION_PORT_CLOSED_ERROR, + message="Port 12345 on hostname localhost refused the connection.", + level=ErrorLevel.ERROR, + extra={"engine_name": "PostgreSQL"}, + ) + ] + + msg = dedent( + """ +psql: error: could not connect to server: Operation timed out + Is the server running on host "example.com" (93.184.216.34) and accepting + TCP/IP connections on port 12345? + """ + ) + result = PostgresEngineSpec.extract_errors(Exception(msg)) + assert result == [ + SupersetError( + error_type=SupersetErrorType.TEST_CONNECTION_HOST_DOWN_ERROR, + message=( + "The host example.com might be down, " + "and can't be reached on port 12345" + ), + level=ErrorLevel.ERROR, + extra={"engine_name": "PostgreSQL"}, + ) + ] + + # response with IP only + msg = dedent( + """ +psql: error: could not connect to server: Operation timed out + Is the server running on host "93.184.216.34" and accepting + TCP/IP connections on port 12345? + """ + ) + result = PostgresEngineSpec.extract_errors(Exception(msg)) + assert result == [ + SupersetError( + error_type=SupersetErrorType.TEST_CONNECTION_HOST_DOWN_ERROR, + message=( + "The host 93.184.216.34 might be down, " + "and can't be reached on port 12345" + ), + level=ErrorLevel.ERROR, + extra={"engine_name": "PostgreSQL"}, + ) + ]