From d625f5f111ac64313c9dcbd07b89457c1a9796af Mon Sep 17 00:00:00 2001
From: Beto Dealmeida <roberto@dealmeida.net>
Date: Wed, 16 Jun 2021 12:58:14 -0700
Subject: [PATCH] feat: show rich error messages on past failed queries
 (#15158)

* feat: store SIP-40 error payload with queries

* Set errors in query on load
---
 .../SqlLab/components/SouthPane/SouthPane.tsx |  3 +++
 superset/db_engine_specs/sqlite.py            | 16 +++++++++++++++-
 superset/sql_lab.py                           | 12 +++++++++++-
 superset/views/core.py                        | 19 ++++++++++---------
 4 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.tsx b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.tsx
index f084fc4384..80f9e2d798 100644
--- a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.tsx
+++ b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.tsx
@@ -118,6 +118,9 @@ export default function SouthPane({
     }
     let results;
     if (latestQuery) {
+      if (latestQuery?.extra?.errors) {
+        latestQuery.errors = latestQuery.extra.errors;
+      }
       if (
         isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) &&
         latestQuery.state === 'success' &&
diff --git a/superset/db_engine_specs/sqlite.py b/superset/db_engine_specs/sqlite.py
index 07706d2d8e..cf2f0f9e23 100644
--- a/superset/db_engine_specs/sqlite.py
+++ b/superset/db_engine_specs/sqlite.py
@@ -14,12 +14,15 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+import re
 from datetime import datetime
-from typing import List, Optional, TYPE_CHECKING
+from typing import Any, Dict, List, Optional, Pattern, Tuple, TYPE_CHECKING
 
+from flask_babel import gettext as __
 from sqlalchemy.engine.reflection import Inspector
 
 from superset.db_engine_specs.base import BaseEngineSpec
+from superset.errors import SupersetErrorType
 from superset.utils import core as utils
 
 if TYPE_CHECKING:
@@ -27,6 +30,9 @@ if TYPE_CHECKING:
     from superset.models.core import Database
 
 
+COLUMN_DOES_NOT_EXIST_REGEX = re.compile("no such column: (?P<column_name>.+)")
+
+
 class SqliteEngineSpec(BaseEngineSpec):
     engine = "sqlite"
     engine_name = "SQLite"
@@ -53,6 +59,14 @@ class SqliteEngineSpec(BaseEngineSpec):
         "1969-12-28T00:00:00Z/P1W": "DATE({col}, 'weekday 0', '-7 days')",
     }
 
+    custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = {
+        COLUMN_DOES_NOT_EXIST_REGEX: (
+            __('We can\'t seem to resolve the column "%(column_name)s"'),
+            SupersetErrorType.COLUMN_DOES_NOT_EXIST_ERROR,
+            {},
+        ),
+    }
+
     @classmethod
     def epoch_to_dttm(cls) -> str:
         return "datetime({col}, 'unixepoch')"
diff --git a/superset/sql_lab.py b/superset/sql_lab.py
index 09cfc417c9..e6f3e5428a 100644
--- a/superset/sql_lab.py
+++ b/superset/sql_lab.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 logging
 import uuid
 from contextlib import closing
@@ -93,8 +94,17 @@ def handle_query_error(
     query.error_message = msg
     query.status = QueryStatus.FAILED
     query.tmp_table_name = None
+
+    # extract DB-specific errors (invalid column, eg)
+    errors = [
+        dataclasses.asdict(error)
+        for error in query.database.db_engine_spec.extract_errors(msg)
+    ]
+    if errors:
+        query.set_extra_json_key("errors", errors)
+
     session.commit()
-    payload.update({"status": query.status, "error": msg})
+    payload.update({"status": query.status, "error": msg, "errors": errors})
     if troubleshooting_link:
         payload["link"] = troubleshooting_link
     return payload
diff --git a/superset/views/core.py b/superset/views/core.py
index 219e81eed8..0d7ea13015 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -73,6 +73,7 @@ from superset.dashboards.dao import DashboardDAO
 from superset.databases.dao import DatabaseDAO
 from superset.databases.filters import DatabaseFilter
 from superset.datasets.commands.exceptions import DatasetNotFoundError
+from superset.errors import SupersetError
 from superset.exceptions import (
     CacheLoadError,
     CertificateException,
@@ -2427,15 +2428,15 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
             raise SupersetGenericDBErrorException(utils.error_msg_from_exception(ex))
 
         if data.get("status") == QueryStatus.FAILED:
-            msg = data["error"]
-            query = _session.query(Query).filter_by(id=query_id).one()
-            database = query.database
-            db_engine_spec = database.db_engine_spec
-            errors = db_engine_spec.extract_errors(msg)
-            _session.close()
-            if errors:
-                raise SupersetErrorsException(errors)
-            raise SupersetGenericDBErrorException(msg)
+            # new error payload with rich context
+            if data["errors"]:
+                raise SupersetErrorsException(
+                    [SupersetError(**params) for params in data["errors"]]
+                )
+
+            # old string-only error message
+            raise SupersetGenericDBErrorException(data["error"])
+
         return json_success(payload)
 
     @has_access_api