feat: add extract_errors to Postgres (#13997)

* feat: add extract_errors to Postgres

* Add unit tests

* Fix lint

* Fix unit tests

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.

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',

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 @@ def assign_column_label(df: pd.DataFrame) -> Optional[pd.DataFrame]:
                 "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(

superset/databases/commands/exceptions.py

@@ -25,7 +25,7 @@
     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

superset/databases/commands/test_connection.py

@@ -20,23 +20,19 @@
 
 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
 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 @@ def __init__(self, user: User, data: Dict[str, Any]):
         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 @@ def run(self) -> None:
                 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__}",

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 @@ def _extract_error_message(cls, ex: Exception) -> str:
         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},
             )
         ]
 

superset/db_engine_specs/postgres.py

@@ -31,12 +31,14 @@
     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<username>.*?)" does not exist')
+INVALID_HOSTNAME_REGEX = re.compile(
+    'could not translate host name "(?P<hostname>.*?)" 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<hostname>.*?)" (\(.*?\) )?and accepting\s+TCP/IP '
+    r"connections on port (?P<port>.*?)\?"
+)
+CONNECTION_HOST_DOWN_REGEX = re.compile(
+    r"could not connect to server: (?P<reason>.*?)\s+Is the server running on "
+    r'host "(?P<hostname>.*?)" (\(.*?\) )?and accepting\s+TCP/IP '
+    r"connections on port (?P<port>.*?)\?"
+)
+
+
 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

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 @@ def get_function_names(cls, database: "Database") -> List[str]:
         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: