feat: refactor error components and add database issue code (#10473)

* feat: refactor error components and add database issue code * Apply suggestions from code review Co-authored-by: John Bodley <4567245+john-bodley@users.noreply.github.com> Co-authored-by: John Bodley <4567245+john-bodley@users.noreply.github.com>
2026-04-27 03:55:47 +00:00 · 2020-08-06 13:22:24 -07:00
parent 62b873e3da
commit 2055ecc1ba
35 changed files with 384 additions and 185 deletions
--- a/superset-frontend/src/components/ErrorMessage/DatabaseErrorMessage.tsx
+++ b/superset-frontend/src/components/ErrorMessage/DatabaseErrorMessage.tsx
@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { t, tn } from '@superset-ui/translation';
+
+import { ErrorMessageComponentProps } from './types';
+import IssueCode from './IssueCode';
+import ErrorAlert from './ErrorAlert';
+
+interface DatabaseErrorExtra {
+  owners?: string[];
+  issue_codes: {
+    code: number;
+    message: string;
+  }[];
+  engine_name: string | null;
+}
+
+function DatabaseErrorMessage({
+  error,
+  source = 'dashboard',
+}: ErrorMessageComponentProps<DatabaseErrorExtra>) {
+  const { extra, level, message } = error;
+
+  const isVisualization = ['dashboard', 'explore'].includes(source);
+
+  const body = (
+    <>
+      <p>
+        {t('This may be triggered by:')}
+        <br />
+        {extra.issue_codes
+          .map<React.ReactNode>(issueCode => <IssueCode {...issueCode} />)
+          .reduce((prev, curr) => [prev, <br />, curr])}
+      </p>
+      {isVisualization && extra.owners && (
+        <>
+          <br />
+          <p>
+            {tn(
+              'Please reach out to the Chart Owner for assistance.',
+              'Please reach out to the Chart Owners for assistance.',
+              extra.owners.length,
+            )}
+          </p>
+          <p>
+            {tn(
+              'Chart Owner: %s',
+              'Chart Owners: %s',
+              extra.owners.length,
+              extra.owners.join(', '),
+            )}
+          </p>
+        </>
+      )}
+    </>
+  );
+
+  const copyText = `${message}
+${t('This may be triggered by:')}
+${extra.issue_codes.map(issueCode => issueCode.message).join('\n')}`;
+
+  return (
+    <ErrorAlert
+      title={t('%s Error', extra.engine_name || t('DB Engine'))}
+      subtitle={message}
+      level={level}
+      source={source}
+      copyText={copyText}
+      body={body}
+    />
+  );
+}
+
+export default DatabaseErrorMessage;
--- a/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
+++ b/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { useState, ReactNode } from 'react';
+import { Modal } from 'react-bootstrap';
+import { styled, supersetTheme } from '@superset-ui/style';
+import { t } from '@superset-ui/translation';
+import { noOp } from 'src/utils/common';
+import Button from 'src/views/CRUD/dataset/Button';
+
+import Icon from '../Icon';
+import { ErrorLevel, ErrorSource } from './types';
+import CopyToClipboard from '../CopyToClipboard';
+
+const ErrorAlertDiv = styled.div<{ level: ErrorLevel }>`
+  align-items: center;
+  background-color: ${({ level, theme }) => theme.colors[level].light2};
+  border-radius: ${({ theme }) => theme.borderRadius}px;
+  border: 1px solid ${({ level, theme }) => theme.colors[level].base};
+  color: ${({ level, theme }) => theme.colors[level].dark2};
+  padding: ${({ theme }) => 2 * theme.gridUnit}px;
+  width: 100%;
+
+  .top-row {
+    display: flex;
+    justify-content: space-between;
+  }
+
+  .error-body {
+    padding-top: ${({ theme }) => theme.gridUnit}px;
+    padding-left: ${({ theme }) => 8 * theme.gridUnit}px;
+  }
+
+  .icon {
+    margin-right: ${({ theme }) => 2 * theme.gridUnit}px;
+  }
+
+  .link {
+    color: ${({ level, theme }) => theme.colors[level].dark2};
+    text-decoration: underline;
+  }
+`;
+
+const ErrorModal = styled(Modal)<{ level: ErrorLevel }>`
+  color: ${({ level, theme }) => theme.colors[level].dark2};
+
+  .icon {
+    margin-right: ${({ theme }) => 2 * theme.gridUnit}px;
+  }
+
+  .header {
+    align-items: center;
+    background-color: ${({ level, theme }) => theme.colors[level].light2};
+    display: flex;
+    justify-content: space-between;
+    font-size: ${({ theme }) => theme.typography.sizes.l}px;
+
+    // Remove clearfix hack as Superset is only used on modern browsers
+    ::before,
+    ::after {
+      content: unset;
+    }
+  }
+`;
+
+const LeftSideContent = styled.div`
+  align-items: center;
+  display: flex;
+`;
+
+interface ErrorAlertProps {
+  body: ReactNode;
+  copyText?: string;
+  level: ErrorLevel;
+  source?: ErrorSource;
+  subtitle: ReactNode;
+  title: ReactNode;
+}
+
+export default function ErrorAlert({
+  body,
+  copyText,
+  level,
+  source = 'dashboard',
+  subtitle,
+  title,
+}: ErrorAlertProps) {
+  const [isModalOpen, setIsModalOpen] = useState(false);
+  const [isBodyExpanded, setIsBodyExpanded] = useState(false);
+
+  const isExpandable = ['explore', 'sqllab'].includes(source);
+
+  return (
+    <ErrorAlertDiv level={level}>
+      <div className="top-row">
+        <LeftSideContent>
+          <Icon
+            className="icon"
+            name={level === 'error' ? 'error' : 'warning'}
+            color={supersetTheme.colors[level].base}
+          />
+          <strong>{title}</strong>
+        </LeftSideContent>
+        {!isExpandable && (
+          <a href="#" className="link" onClick={() => setIsModalOpen(true)}>
+            {t('See More')}
+          </a>
+        )}
+      </div>
+      {isExpandable ? (
+        <div className="error-body">
+          <p>{subtitle}</p>
+          {body && (
+            <>
+              {!isBodyExpanded && (
+                <a
+                  href="#"
+                  className="link"
+                  onClick={() => setIsBodyExpanded(true)}
+                >
+                  {t('See More')}
+                </a>
+              )}
+              {isBodyExpanded && (
+                <>
+                  <br />
+                  {body}
+                  <a
+                    href="#"
+                    className="link"
+                    onClick={() => setIsBodyExpanded(false)}
+                  >
+                    {t('See Less')}
+                  </a>
+                </>
+              )}
+            </>
+          )}
+        </div>
+      ) : (
+        <ErrorModal
+          level={level}
+          show={isModalOpen}
+          onHide={() => setIsModalOpen(false)}
+        >
+          <Modal.Header className="header">
+            <LeftSideContent>
+              <Icon
+                className="icon"
+                name={level === 'error' ? 'error' : 'warning'}
+                color={supersetTheme.colors[level].base}
+              />
+              <div className="title">{title}</div>
+            </LeftSideContent>
+            <span
+              role="button"
+              tabIndex={0}
+              onClick={() => setIsModalOpen(false)}
+            >
+              <Icon name="close" />
+            </span>
+          </Modal.Header>
+          <Modal.Body>
+            <p>{subtitle}</p>
+            <br />
+            {body}
+          </Modal.Body>
+          <Modal.Footer>
+            {copyText && (
+              <CopyToClipboard
+                text={copyText}
+                shouldShowText={false}
+                wrapped={false}
+                copyNode={<Button onClick={noOp}>{t('Copy Message')}</Button>}
+              />
+            )}
+            <Button bsStyle="primary" onClick={() => setIsModalOpen(false)}>
+              {t('Close')}
+            </Button>
+          </Modal.Footer>
+        </ErrorModal>
+      )}
+    </ErrorAlertDiv>
+  );
+}
--- a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx
+++ b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx
@@ -16,13 +16,12 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import React, { useState } from 'react';
-// @ts-ignore
-import { Alert, Collapse } from 'react-bootstrap';
+import React from 'react';
 import { t } from '@superset-ui/translation';

 import getErrorMessageComponentRegistry from './getErrorMessageComponentRegistry';
 import { SupersetError, ErrorSource } from './types';
+import ErrorAlert from './ErrorAlert';

 type Props = {
  error?: SupersetError;
@@ -39,8 +38,6 @@ export default function ErrorMessageWithStackTrace({
  stackTrace,
  source,
 }: Props) {
-  const [showStackTrace, setShowStackTrace] = useState(false);
-
  // Check if a custom error message component was registered for this message
  if (error) {
    const ErrorMessageComponent = getErrorMessageComponentRegistry().get(
@@ -51,25 +48,26 @@ export default function ErrorMessageWithStackTrace({
    }
  }

-  // Fallback to the default error message renderer
  return (
-    <div className={`stack-trace-container${stackTrace ? ' has-trace' : ''}`}>
-      <Alert
-        bsStyle="warning"
-        onClick={() => setShowStackTrace(!showStackTrace)}
-      >
-        {message || t('An error occurred.')}
-        {link && (
-          <a href={link} target="_blank" rel="noopener noreferrer">
-            (Request Access)
-          </a>
-        )}
-      </Alert>
-      {stackTrace && (
-        <Collapse in={showStackTrace}>
-          <pre>{stackTrace}</pre>
-        </Collapse>
-      )}
-    </div>
+    <ErrorAlert
+      level="warning"
+      title={t('Unexpected Error')}
+      subtitle={message}
+      copyText={message}
+      source={source}
+      body={
+        link || stackTrace ? (
+          <>
+            {link && (
+              <a href={link} target="_blank" rel="noopener noreferrer">
+                (Request Access)
+              </a>
+            )}
+            <br />
+            {stackTrace && <pre>{stackTrace}</pre>}
+          </>
+        ) : undefined
+      }
+    />
  );
 }
--- a/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx
+++ b/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx
@@ -16,73 +16,12 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import React, { useState } from 'react';
-import { Modal } from 'react-bootstrap';
-import { styled, supersetTheme } from '@superset-ui/style';
+import React from 'react';
 import { t, tn } from '@superset-ui/translation';

-import { noOp } from 'src/utils/common';
-import Button from 'src/views/CRUD/dataset/Button';
-import Icon from '../Icon';
 import { ErrorMessageComponentProps } from './types';
-import CopyToClipboard from '../CopyToClipboard';
 import IssueCode from './IssueCode';
-
-const ErrorAlert = styled.div`
-  align-items: center;
-  background-color: ${({ theme }) => theme.colors.error.light2};
-  border-radius: ${({ theme }) => theme.borderRadius}px;
-  border: 1px solid ${({ theme }) => theme.colors.error.base};
-  color: ${({ theme }) => theme.colors.error.dark2};
-  padding: ${({ theme }) => 2 * theme.gridUnit}px;
-  width: 100%;
-
-  .top-row {
-    display: flex;
-    justify-content: space-between;
-  }
-
-  .error-body {
-    padding-top: ${({ theme }) => theme.gridUnit}px;
-    padding-left: ${({ theme }) => 8 * theme.gridUnit}px;
-  }
-
-  .icon {
-    margin-right: ${({ theme }) => 2 * theme.gridUnit}px;
-  }
-
-  .link {
-    color: ${({ theme }) => theme.colors.error.dark2};
-    text-decoration: underline;
-  }
-`;
-
-const ErrorModal = styled(Modal)`
-  color: ${({ theme }) => theme.colors.error.dark2};
-
-  .icon {
-    margin-right: ${({ theme }) => 2 * theme.gridUnit}px;
-  }
-
-  .header {
-    align-items: center;
-    background-color: ${({ theme }) => theme.colors.error.light2};
-    display: flex;
-    justify-content: space-between;
-    font-size: ${({ theme }) => theme.typography.sizes.l}px;
-
-    // Remove clearfix hack as Superset is only used on modern browsers
-    ::before,
-    ::after {
-      content: unset;
-    }
-  }
-`;
-
-const LeftSideContent = styled.div`
-  align-items: center;
-  display: flex;
-`;
+import ErrorAlert from './ErrorAlert';

 interface TimeoutErrorExtra {
  issue_codes: {
@@ -97,21 +36,14 @@ function TimeoutErrorMessage({
  error,
  source,
 }: ErrorMessageComponentProps<TimeoutErrorExtra>) {
-  const [isModalOpen, setIsModalOpen] = useState(false);
-  const [isMessageExpanded, setIsMessageExpanded] = useState(false);
-  const { extra } = error;
+  const { extra, level } = error;

  const isVisualization = (['dashboard', 'explore'] as (
    | string
    | undefined
  )[]).includes(source);

-  const isExpandable = (['explore', 'sqllab'] as (
-    | string
-    | undefined
-  )[]).includes(source);
-
-  const title = isVisualization
+  const subtitle = isVisualization
    ? tn(
        'We’re having trouble loading this visualization. Queries are set to timeout after %s second.',
        'We’re having trouble loading this visualization. Queries are set to timeout after %s seconds.',
@@ -125,7 +57,7 @@ function TimeoutErrorMessage({
        extra.timeout,
      );

-  const message = (
+  const body = (
    <>
      <p>
        {t('This may be triggered by:')}
@@ -157,91 +89,19 @@ function TimeoutErrorMessage({
    </>
  );

-  const copyText = `${title}
+  const copyText = `${subtitle}
 ${t('This may be triggered by:')}
 ${extra.issue_codes.map(issueCode => issueCode.message).join('\n')}`;

  return (
-    <ErrorAlert>
-      <div className="top-row">
-        <LeftSideContent>
-          <Icon
-            className="icon"
-            name="error"
-            color={supersetTheme.colors.error.base}
-          />
-          <strong>{t('Timeout Error')}</strong>
-        </LeftSideContent>
-        {!isExpandable && (
-          <a href="#" className="link" onClick={() => setIsModalOpen(true)}>
-            {t('See More')}
-          </a>
-        )}
-      </div>
-      {isExpandable ? (
-        <div className="error-body">
-          <p>{title}</p>
-          {!isMessageExpanded && (
-            <a
-              href="#"
-              className="link"
-              onClick={() => setIsMessageExpanded(true)}
-            >
-              {t('See More')}
-            </a>
-          )}
-          {isMessageExpanded && (
-            <>
-              <br />
-              {message}
-              <a
-                href="#"
-                className="link"
-                onClick={() => setIsMessageExpanded(false)}
-              >
-                {t('See Less')}
-              </a>
-            </>
-          )}
-        </div>
-      ) : (
-        <ErrorModal show={isModalOpen} onHide={() => setIsModalOpen(false)}>
-          <Modal.Header className="header">
-            <LeftSideContent>
-              <Icon
-                className="icon"
-                name="error"
-                color={supersetTheme.colors.error.base}
-              />
-              <div className="title">{t('Timeout Error')}</div>
-            </LeftSideContent>
-            <span
-              role="button"
-              tabIndex={0}
-              onClick={() => setIsModalOpen(false)}
-            >
-              <Icon name="close" />
-            </span>
-          </Modal.Header>
-          <Modal.Body>
-            <p>{title}</p>
-            <br />
-            {message}
-          </Modal.Body>
-          <Modal.Footer>
-            <CopyToClipboard
-              text={copyText}
-              shouldShowText={false}
-              wrapped={false}
-              copyNode={<Button onClick={noOp}>{t('Copy Message')}</Button>}
-            />
-            <Button bsStyle="primary" onClick={() => setIsModalOpen(false)}>
-              {t('Close')}
-            </Button>
-          </Modal.Footer>
-        </ErrorModal>
-      )}
-    </ErrorAlert>
+    <ErrorAlert
+      title={t('Timeout Error')}
+      subtitle={subtitle}
+      level={level}
+      source={source}
+      copyText={copyText}
+      body={body}
+    />
  );
 }