You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/08/05 00:26:06 UTC

(superset) branch aminghadersohi/sqllab-oauth2-table-preview-retry created (now bdd0ca890bd)

This is an automated email from the ASF dual-hosted git repository.

aminghadersohi pushed a change to branch aminghadersohi/sqllab-oauth2-table-preview-retry
in repository https://gitbox.apache.org/repos/asf/superset.git


      at bdd0ca890bd fix(sqllab): don't let a generic metadata error mask an OAuth2 redirect

This branch includes the following new commits:

     new bdd0ca890bd fix(sqllab): don't let a generic metadata error mask an OAuth2 redirect

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: fix(sqllab): don't let a generic metadata error mask an OAuth2 redirect

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

aminghadersohi pushed a commit to branch aminghadersohi/sqllab-oauth2-table-preview-retry
in repository https://gitbox.apache.org/repos/asf/superset.git

commit bdd0ca890bde5ff90086f9a2b5310746f29db28a
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Wed Aug 5 00:25:29 2026 +0000

    fix(sqllab): don't let a generic metadata error mask an OAuth2 redirect
    
    Both table-metadata requests can fail at once. Always preferring
    metadataError meant a generic failure there (raw 500, timeout, network
    error) suppressed an actionable OAUTH2_REDIRECT payload coming from the
    extended-metadata request, hiding the authorization link and the retry.
    
    Select whichever request carries a structured errors[] payload instead,
    keeping the plain-message fallback for the case where neither does.
    
    Also adds the end-to-end assertion for the headline behavior: the
    completion broadcast repopulates the preview, rather than only checking
    that the link renders and the tag string is present in invalidateTags.
    
    Co-Authored-By: Claude <no...@anthropic.com>
---
 .../components/TablePreview/TablePreview.test.tsx  | 81 ++++++++++++++++++----
 .../src/SqlLab/components/TablePreview/index.tsx   | 14 ++--
 2 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/superset-frontend/src/SqlLab/components/TablePreview/TablePreview.test.tsx b/superset-frontend/src/SqlLab/components/TablePreview/TablePreview.test.tsx
index 072f27e70d0..32010ea9786 100644
--- a/superset-frontend/src/SqlLab/components/TablePreview/TablePreview.test.tsx
+++ b/superset-frontend/src/SqlLab/components/TablePreview/TablePreview.test.tsx
@@ -83,6 +83,17 @@ const mockedProps = {
   tableName: table.name,
 };
 
+const OAUTH2_TAB_ID = 'tab-1';
+const oauth2Error = {
+  message: 'The database is currently not authenticated',
+  error_type: ErrorTypeEnum.OAUTH2_REDIRECT,
+  level: 'warning',
+  extra: {
+    url: 'https://example.com/oauth2/authorize',
+    tab_id: OAUTH2_TAB_ID,
+  },
+};
+
 test('renders columns', async () => {
   const { getAllByTestId, queryByText } = render(
     <TablePreview {...mockedProps} />,
@@ -148,19 +159,7 @@ test('renders an OAuth2 authorization prompt when metadata errors with OAUTH2_RE
   fetchMock.removeRoutes();
   fetchMock.get(getTableMetadataEndpoint, {
     status: 403,
-    body: {
-      errors: [
-        {
-          message: 'The database is currently not authenticated',
-          error_type: ErrorTypeEnum.OAUTH2_REDIRECT,
-          level: 'warning',
-          extra: {
-            url: 'https://example.com/oauth2/authorize',
-            tab_id: 'tab-1',
-          },
-        },
-      ],
-    },
+    body: { errors: [oauth2Error] },
   });
   fetchMock.get(getExtraTableMetadataEndpoint, {});
 
@@ -177,6 +176,62 @@ test('renders an OAuth2 authorization prompt when metadata errors with OAUTH2_RE
   );
 });
 
+test('surfaces an OAUTH2_REDIRECT from the extended metadata request when the main request fails generically', async () => {
+  setupErrorMessages();
+  fetchMock.removeRoutes();
+  fetchMock.get(getTableMetadataEndpoint, {
+    status: 500,
+    body: { message: 'Something went wrong' },
+  });
+  fetchMock.get(getExtraTableMetadataEndpoint, {
+    status: 403,
+    body: { errors: [oauth2Error] },
+  });
+
+  render(<TablePreview {...mockedProps} />, { useRedux: true, initialState });
+
+  // The generic failure on the main request must not mask the actionable
+  // OAuth2 payload coming from the extended metadata request.
+  expect(
+    await screen.findByRole('link', { name: 'provide authorization' }),
+  ).toBeInTheDocument();
+});
+
+test('repopulates the preview once the OAuth2 dance completes', async () => {
+  setupErrorMessages();
+  fetchMock.removeRoutes();
+  fetchMock.get(getTableMetadataEndpoint, {
+    status: 403,
+    body: { errors: [oauth2Error] },
+  });
+  fetchMock.get(getExtraTableMetadataEndpoint, {});
+
+  const { getAllByTestId } = render(<TablePreview {...mockedProps} />, {
+    useRedux: true,
+    initialState,
+  });
+  await screen.findByRole('link', { name: 'provide authorization' });
+
+  // The second tab finished authorizing: the database now answers with the
+  // metadata, and the completion broadcast must drive the refetch.
+  fetchMock.removeRoutes();
+  fetchMock.get(getTableMetadataEndpoint, table);
+  fetchMock.get(getExtraTableMetadataEndpoint, {});
+  fireEvent(
+    window,
+    Object.assign(new Event('storage'), {
+      key: 'oauth2_auth_complete',
+      newValue: JSON.stringify({ tabId: OAUTH2_TAB_ID }),
+    }),
+  );
+
+  await waitFor(() =>
+    expect(getAllByTestId('mock-record-row')).toHaveLength(
+      table.columns.length,
+    ),
+  );
+});
+
 test('renders the error message when metadata fails without a structured error', async () => {
   fetchMock.removeRoutes();
   fetchMock.get(getTableMetadataEndpoint, {
diff --git a/superset-frontend/src/SqlLab/components/TablePreview/index.tsx b/superset-frontend/src/SqlLab/components/TablePreview/index.tsx
index 499c64a2810..6aab1e5deb6 100644
--- a/superset-frontend/src/SqlLab/components/TablePreview/index.tsx
+++ b/superset-frontend/src/SqlLab/components/TablePreview/index.tsx
@@ -268,13 +268,19 @@ const TablePreview: FC<Props> = ({ dbId, catalog, schema, tableName }) => {
     // OAuth2RedirectMessage.tsx (which invalidates the TableMetadatas tag).
     // Errors without a structured errors[] array (raw 500s, timeouts, network
     // failures) fall back to the plain message so it isn't swallowed.
-    const clientError = (metadataError ||
-      metadataExtrError) as ClientErrorObject;
-    const errorPayload = clientError?.errors?.[0];
+    // Both requests can fail at once, so pick whichever carries a structured
+    // payload rather than always preferring the main request — otherwise a
+    // generic failure there would mask an actionable OAUTH2_REDIRECT from the
+    // extended-metadata request.
+    const clientErrors = [metadataError, metadataExtrError].filter(
+      Boolean,
+    ) as ClientErrorObject[];
+    const errorPayload = clientErrors.find(({ errors }) => errors?.length)
+      ?.errors?.[0];
     return errorPayload ? (
       <ErrorMessageWithStackTrace error={errorPayload} source="crud" />
     ) : (
-      <Alert type="warning" message={clientError?.error} />
+      <Alert type="warning" message={clientErrors[0]?.error} />
     );
   }
   if (!data) {