You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/07/29 20:55:02 UTC
[PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
aminghadersohi opened a new pull request, #42581:
URL: https://github.com/apache/superset/pull/42581
### SUMMARY
The dataset-creation metadata preview now preserves structured API errors and renders them through the registered error-message system, so OAuth2 authorization failures display the existing authorization prompt instead of a generic empty error state.
After a matching OAuth2 completion notification, the preview retries only its original table-metadata request. This keeps the existing raw `SupersetClient` call rather than migrating a single request to RTK Query, avoiding a larger change and broad cache invalidation of unrelated metadata requests. The shared OAuth2 component now accepts the existing optional mitigation callback and deduplicates the BroadcastChannel and storage notifications emitted for the same tab.
The blast radius is limited to dataset creation and the optional callback path in the shared OAuth2 message; SQL Lab, Explore, dashboard, and default CRUD retry behavior are unchanged when no callback is supplied. Request IDs prevent stale or unmounted requests from updating the preview. Rollback is a revert of this commit.
Review guidance: start with `DatasetPanel/index.tsx` for error parsing and scoped retry, then follow `errorMitigationFunction` through `ErrorMessageWithStackTrace` into `OAuth2RedirectMessage`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not included because a local Superset backend was unavailable. The UI uses the existing OAuth2 authorization alert, and React Testing Library coverage verifies the authorization link and successful automatic retry.
### TESTING INSTRUCTIONS
1. Configure a database that uses OAuth2 authentication.
2. In the dataset-creation flow, select a table whose metadata request requires authorization.
3. Verify the authorization alert and link are displayed.
4. Complete authorization in the opened tab and verify the selected table's columns load automatically without reselecting the table.
Automated validation:
```bash
cd superset-frontend
npm run test -- \
src/components/ErrorMessage/ErrorMessageWithStackTrace.test.tsx \
src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx \
src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.test.tsx \
src/features/datasets/AddDataset/DatasetPanel/DatasetPanelWrapper.test.tsx \
src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.subdirectory.test.tsx \
src/features/datasets/DatasetLayout/DatasetLayout.test.tsx
```
All 6 suites and 37 tests pass. Scoped Prettier, Oxlint, custom frontend rules, and stylelint checks pass. The targeted type-check hook could not complete locally because the clean worktree lacks built workspace declaration outputs (TS6305); CI builds those package outputs.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42581:
URL: https://github.com/apache/superset/pull/42581#issuecomment-5123452542
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42581?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `80.26316%` with `15 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 65.34%. Comparing base ([`bdaa535`](https://app.codecov.io/gh/apache/superset/commit/bdaa53575f19ce009726c1d2d3ede4d9dd49b959?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`987bdf8`](https://app.codecov.io/gh/apache/superset/commit/987bdf83054a67eebc5c1d6ea002068dba318ca8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 6 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/42581?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...eatures/datasets/AddDataset/DatasetPanel/index.tsx](https://app.codecov.io/gh/apache/superset/pull/42581?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Ffeatures%2Fdatasets%2FAddDataset%2FDatasetPanel%2Findex.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVzL2RhdGFzZXRzL0FkZERhdGFzZXQvRGF0YXNldFBhbmVsL2luZGV4LnRzeA==) | 72.72% | [15 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42581 +/- ##
==========================================
+ Coverage 64.73% 65.34% +0.60%
==========================================
Files 2803 2803
Lines 158465 158497 +32
Branches 36170 36179 +9
==========================================
+ Hits 102590 103567 +977
+ Misses 53861 52919 -942
+ Partials 2014 2011 -3
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42581/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/42581/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.47% <80.26%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42581?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42581:
URL: https://github.com/apache/superset/pull/42581#issuecomment-5123283597
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42581:
URL: https://github.com/apache/superset/pull/42581#discussion_r3679964570
##########
superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx:
##########
@@ -103,13 +104,18 @@ export function OAuth2RedirectMessage({
);
const dispatch = useDispatch();
+ const lastHandledTabIdRef = useRef<string>();
useEffect(() => {
const handleOAuthComplete = (tabId?: string) => {
- if (tabId !== extra.tab_id) {
+ if (tabId !== extra.tab_id || tabId === lastHandledTabIdRef.current) {
return;
}
- if (source === 'sqllab' && query) {
+ lastHandledTabIdRef.current = tabId;
+
+ if (errorMitigationFunction) {
+ errorMitigationFunction();
+ } else if (source === 'sqllab' && query) {
dispatch(reRunQuery(query));
Review Comment:
**Suggestion:** The tab ID is marked as handled before confirming that any mitigation or rerun can actually execute. For SQL Lab, `query` can be temporarily unavailable when the OAuth completion arrives; the event is then consumed without dispatching `reRunQuery`, and the matching BroadcastChannel/storage duplicate is suppressed as well. If the query becomes available afterward, the component never retries. Record the tab ID only after a valid action is scheduled, or otherwise retry when the required state becomes available. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ SQL Lab OAuth completion may not rerun the failed query.
- ⚠️ Users must manually rerun after authorization.
- ⚠️ BroadcastChannel and storage retries are both suppressed.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d998bb84f5d14956be9dbb0834ebcedf&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d998bb84f5d14956be9dbb0834ebcedf&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx
**Line:** 111:119
**Comment:**
*Logic Error: The tab ID is marked as handled before confirming that any mitigation or rerun can actually execute. For SQL Lab, `query` can be temporarily unavailable when the OAuth completion arrives; the event is then consumed without dispatching `reRunQuery`, and the matching BroadcastChannel/storage duplicate is suppressed as well. If the query becomes available afterward, the component never retries. Record the tab ID only after a valid action is scheduled, or otherwise retry when the required state becomes available.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42581&comment_hash=a89d0094c93b515d48b8956b10a6a59c31a62a34b61d9c60a1b2135d7728e637&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42581&comment_hash=a89d0094c93b515d48b8956b10a6a59c31a62a34b61d9c60a1b2135d7728e637&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "rebenitez1802 (via GitHub)" <gi...@apache.org>.
rebenitez1802 commented on code in PR #42581:
URL: https://github.com/apache/superset/pull/42581#discussion_r3684856223
##########
superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx:
##########
@@ -78,74 +87,127 @@ const DatasetPanelWrapper = ({
}: IDatasetPanelWrapperProps) => {
const [columnList, setColumnList] = useState<ITableColumn[]>([]);
const [loading, setLoading] = useState(false);
- const [hasError, setHasError] = useState(false);
- const tableNameRef = useRef(tableName);
+ const [error, setError] = useState<SupersetError>();
+ const requestIdRef = useRef(0);
+ const currentRequestRef = useRef<TableMetadataRequest>();
+ const supportsSchemas = database?.supports_schemas;
- const getTableMetadata = async (props: IColumnProps) => {
- const { dbId, tableName, schema } = props;
- setLoading(true);
- setHasColumns?.(false);
- const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
- name: tableName,
- catalog,
- schema,
- })}`;
- try {
- const response = await SupersetClient.get({
- endpoint: path,
- });
+ const getTableMetadata = useCallback(
+ async (props: TableMetadataRequest) => {
+ const { dbId, tableName, catalog, schema } = props;
+ requestIdRef.current += 1;
+ const requestId = requestIdRef.current;
+ setLoading(true);
+ setColumnList([]);
+ setError(undefined);
+ setHasColumns?.(false);
+ const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
+ name: tableName,
+ catalog,
+ schema,
+ })}`;
+ try {
+ const response = await SupersetClient.get({
+ endpoint: path,
+ });
+
+ if (requestId !== requestIdRef.current) {
+ return;
+ }
- if (isIDatabaseTable(response?.json)) {
- const table: IDatabaseTable = response.json as IDatabaseTable;
- /**
- * The user is able to click other table columns while the http call for last selected table column is made
- * This check ensures we process the response that matches the last selected table name and ignore the others
- */
- if (table.name === tableNameRef.current) {
+ const table = isIDatabaseTable(response?.json)
+ ? (response.json as IDatabaseTable)
+ : undefined;
+ if (table?.name === tableName) {
setColumnList(table.columns);
setHasColumns?.(table.columns.length > 0);
- setHasError(false);
- }
- } else {
- setColumnList([]);
- setHasColumns?.(false);
- setHasError(true);
- addDangerToast(
- t(
- 'The API response from %s does not match the IDatabaseTable interface.',
- path,
- ),
- );
- logging.error(
- t(
+ setError(undefined);
+ } else {
+ const message = t(
'The API response from %s does not match the IDatabaseTable interface.',
path,
- ),
+ );
+ setColumnList([]);
+ setHasColumns?.(false);
+ setError({
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error',
+ message,
+ });
+ addDangerToast(message);
+ logging.error(message);
+ }
+ } catch (caughtError) {
+ const clientError = await getClientErrorObject(
+ caughtError as Parameters<typeof getClientErrorObject>[0],
);
+
+ if (requestId === requestIdRef.current) {
+ const parsedError = clientError.errors?.[0] ?? {
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error' as const,
+ message: clientError.error,
Review Comment:
`clientError.error` is `undefined` at runtime when the rejected body has no `errors[]` and no usable `error`/`message` (e.g. a 500 with body `{}` or `{ "errors": [] }`) — `parseErrorJson` returns `error: error.error` → `undefined`. Since this fallback pins `error_type: GENERIC_BACKEND_ERROR`, it renders through `DatabaseErrorMessage`, whose `message.split('\n')` throws on `undefined`. Guarantee a string so a generic backend failure shows an error instead of crashing the preview:
```suggestion
message: clientError.error ?? t('Unable to load columns for the selected table.'),
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42581:
URL: https://github.com/apache/superset/pull/42581#issuecomment-5134754538
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | fdea5b3306b6a95d2355dd4430d59658ede42568 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a6b97656eff5a0008f22715 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-42581--superset-docs-preview.netlify.app](https://deploy-preview-42581--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctBDoMgEADAv-y5RCQB2_7AZ6yyFJLKknU9GOPf5TaXuaAJb03hC_CCRbCuuRu3Un8ZI8nOuQwRFXdSswqhFq6G8dDsjJDK2V-k9udzjn0GDMtnCj5QSh6tte_k3DR6uB8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42581--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #42581:
URL: https://github.com/apache/superset/pull/42581
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #42581:
URL: https://github.com/apache/superset/pull/42581#discussion_r3685254528
##########
superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx:
##########
@@ -78,74 +87,127 @@ const DatasetPanelWrapper = ({
}: IDatasetPanelWrapperProps) => {
const [columnList, setColumnList] = useState<ITableColumn[]>([]);
const [loading, setLoading] = useState(false);
- const [hasError, setHasError] = useState(false);
- const tableNameRef = useRef(tableName);
+ const [error, setError] = useState<SupersetError>();
+ const requestIdRef = useRef(0);
+ const currentRequestRef = useRef<TableMetadataRequest>();
+ const supportsSchemas = database?.supports_schemas;
- const getTableMetadata = async (props: IColumnProps) => {
- const { dbId, tableName, schema } = props;
- setLoading(true);
- setHasColumns?.(false);
- const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
- name: tableName,
- catalog,
- schema,
- })}`;
- try {
- const response = await SupersetClient.get({
- endpoint: path,
- });
+ const getTableMetadata = useCallback(
+ async (props: TableMetadataRequest) => {
+ const { dbId, tableName, catalog, schema } = props;
+ requestIdRef.current += 1;
+ const requestId = requestIdRef.current;
+ setLoading(true);
+ setColumnList([]);
+ setError(undefined);
+ setHasColumns?.(false);
+ const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
+ name: tableName,
+ catalog,
+ schema,
+ })}`;
+ try {
+ const response = await SupersetClient.get({
+ endpoint: path,
+ });
+
+ if (requestId !== requestIdRef.current) {
+ return;
+ }
- if (isIDatabaseTable(response?.json)) {
- const table: IDatabaseTable = response.json as IDatabaseTable;
- /**
- * The user is able to click other table columns while the http call for last selected table column is made
- * This check ensures we process the response that matches the last selected table name and ignore the others
- */
- if (table.name === tableNameRef.current) {
+ const table = isIDatabaseTable(response?.json)
+ ? (response.json as IDatabaseTable)
+ : undefined;
+ if (table?.name === tableName) {
setColumnList(table.columns);
setHasColumns?.(table.columns.length > 0);
- setHasError(false);
- }
- } else {
- setColumnList([]);
- setHasColumns?.(false);
- setHasError(true);
- addDangerToast(
- t(
- 'The API response from %s does not match the IDatabaseTable interface.',
- path,
- ),
- );
- logging.error(
- t(
+ setError(undefined);
+ } else {
+ const message = t(
'The API response from %s does not match the IDatabaseTable interface.',
path,
- ),
+ );
+ setColumnList([]);
+ setHasColumns?.(false);
+ setError({
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error',
+ message,
+ });
+ addDangerToast(message);
+ logging.error(message);
+ }
+ } catch (caughtError) {
+ const clientError = await getClientErrorObject(
+ caughtError as Parameters<typeof getClientErrorObject>[0],
);
+
+ if (requestId === requestIdRef.current) {
+ const parsedError = clientError.errors?.[0] ?? {
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error' as const,
+ message: clientError.error,
+ };
Review Comment:
Fixed in fdea5b3306. The fallback now prefers the parsed error, message, and status text, then guarantees a localized “Unable to load columns for the selected table.” string. I also added a regression test using an HTTP 500 `{}` response with the registered `DatabaseErrorMessage` to verify the preview renders the fallback instead of crashing.
##########
superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx:
##########
@@ -103,13 +104,18 @@ export function OAuth2RedirectMessage({
);
const dispatch = useDispatch();
+ const lastHandledTabIdRef = useRef<string>();
useEffect(() => {
const handleOAuthComplete = (tabId?: string) => {
- if (tabId !== extra.tab_id) {
+ if (tabId !== extra.tab_id || tabId === lastHandledTabIdRef.current) {
return;
}
- if (source === 'sqllab' && query) {
+ lastHandledTabIdRef.current = tabId;
+
+ if (errorMitigationFunction) {
+ errorMitigationFunction();
+ } else if (source === 'sqllab' && query) {
dispatch(reRunQuery(query));
Review Comment:
Fixed in fdea5b3306. The tab ID is now recorded only after a mitigation/rerun/invalidation action can execute; when SQL Lab has no query yet, the completion is left unconsumed. Added coverage where BroadcastChannel arrives before the query is available and the storage notification succeeds after the query appears, plus a dual-transport exactly-once assertion.
##########
superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx:
##########
@@ -78,74 +87,127 @@ const DatasetPanelWrapper = ({
}: IDatasetPanelWrapperProps) => {
const [columnList, setColumnList] = useState<ITableColumn[]>([]);
const [loading, setLoading] = useState(false);
- const [hasError, setHasError] = useState(false);
- const tableNameRef = useRef(tableName);
+ const [error, setError] = useState<SupersetError>();
+ const requestIdRef = useRef(0);
+ const currentRequestRef = useRef<TableMetadataRequest>();
+ const supportsSchemas = database?.supports_schemas;
- const getTableMetadata = async (props: IColumnProps) => {
- const { dbId, tableName, schema } = props;
- setLoading(true);
- setHasColumns?.(false);
- const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
- name: tableName,
- catalog,
- schema,
- })}`;
- try {
- const response = await SupersetClient.get({
- endpoint: path,
- });
+ const getTableMetadata = useCallback(
+ async (props: TableMetadataRequest) => {
+ const { dbId, tableName, catalog, schema } = props;
+ requestIdRef.current += 1;
+ const requestId = requestIdRef.current;
+ setLoading(true);
+ setColumnList([]);
+ setError(undefined);
+ setHasColumns?.(false);
+ const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
+ name: tableName,
+ catalog,
+ schema,
+ })}`;
+ try {
+ const response = await SupersetClient.get({
+ endpoint: path,
+ });
+
+ if (requestId !== requestIdRef.current) {
+ return;
+ }
- if (isIDatabaseTable(response?.json)) {
- const table: IDatabaseTable = response.json as IDatabaseTable;
- /**
- * The user is able to click other table columns while the http call for last selected table column is made
- * This check ensures we process the response that matches the last selected table name and ignore the others
- */
- if (table.name === tableNameRef.current) {
+ const table = isIDatabaseTable(response?.json)
+ ? (response.json as IDatabaseTable)
+ : undefined;
+ if (table?.name === tableName) {
setColumnList(table.columns);
setHasColumns?.(table.columns.length > 0);
- setHasError(false);
- }
- } else {
- setColumnList([]);
- setHasColumns?.(false);
- setHasError(true);
- addDangerToast(
- t(
- 'The API response from %s does not match the IDatabaseTable interface.',
- path,
- ),
- );
- logging.error(
- t(
+ setError(undefined);
+ } else {
+ const message = t(
'The API response from %s does not match the IDatabaseTable interface.',
path,
- ),
+ );
+ setColumnList([]);
+ setHasColumns?.(false);
+ setError({
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error',
+ message,
+ });
+ addDangerToast(message);
+ logging.error(message);
+ }
+ } catch (caughtError) {
+ const clientError = await getClientErrorObject(
+ caughtError as Parameters<typeof getClientErrorObject>[0],
);
+
+ if (requestId === requestIdRef.current) {
+ const parsedError = clientError.errors?.[0] ?? {
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error' as const,
+ message: clientError.error,
Review Comment:
Fixed in fdea5b3306. The generic fallback now always supplies a string (`clientError.error || clientError.message || clientError.statusText ||` the localized column-loading fallback). Added an HTTP 500 `{}` regression test through `DatabaseErrorMessage` to ensure this path renders without throwing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42581:
URL: https://github.com/apache/superset/pull/42581#issuecomment-5136452532
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42581:
URL: https://github.com/apache/superset/pull/42581#discussion_r3679962932
##########
superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx:
##########
@@ -78,74 +87,127 @@ const DatasetPanelWrapper = ({
}: IDatasetPanelWrapperProps) => {
const [columnList, setColumnList] = useState<ITableColumn[]>([]);
const [loading, setLoading] = useState(false);
- const [hasError, setHasError] = useState(false);
- const tableNameRef = useRef(tableName);
+ const [error, setError] = useState<SupersetError>();
+ const requestIdRef = useRef(0);
+ const currentRequestRef = useRef<TableMetadataRequest>();
+ const supportsSchemas = database?.supports_schemas;
- const getTableMetadata = async (props: IColumnProps) => {
- const { dbId, tableName, schema } = props;
- setLoading(true);
- setHasColumns?.(false);
- const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
- name: tableName,
- catalog,
- schema,
- })}`;
- try {
- const response = await SupersetClient.get({
- endpoint: path,
- });
+ const getTableMetadata = useCallback(
+ async (props: TableMetadataRequest) => {
+ const { dbId, tableName, catalog, schema } = props;
+ requestIdRef.current += 1;
+ const requestId = requestIdRef.current;
+ setLoading(true);
+ setColumnList([]);
+ setError(undefined);
+ setHasColumns?.(false);
+ const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
+ name: tableName,
+ catalog,
+ schema,
+ })}`;
+ try {
+ const response = await SupersetClient.get({
+ endpoint: path,
+ });
+
+ if (requestId !== requestIdRef.current) {
+ return;
+ }
- if (isIDatabaseTable(response?.json)) {
- const table: IDatabaseTable = response.json as IDatabaseTable;
- /**
- * The user is able to click other table columns while the http call for last selected table column is made
- * This check ensures we process the response that matches the last selected table name and ignore the others
- */
- if (table.name === tableNameRef.current) {
+ const table = isIDatabaseTable(response?.json)
+ ? (response.json as IDatabaseTable)
+ : undefined;
+ if (table?.name === tableName) {
setColumnList(table.columns);
setHasColumns?.(table.columns.length > 0);
- setHasError(false);
- }
- } else {
- setColumnList([]);
- setHasColumns?.(false);
- setHasError(true);
- addDangerToast(
- t(
- 'The API response from %s does not match the IDatabaseTable interface.',
- path,
- ),
- );
- logging.error(
- t(
+ setError(undefined);
+ } else {
+ const message = t(
'The API response from %s does not match the IDatabaseTable interface.',
path,
- ),
+ );
+ setColumnList([]);
+ setHasColumns?.(false);
+ setError({
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error',
+ message,
+ });
+ addDangerToast(message);
+ logging.error(message);
+ }
+ } catch (caughtError) {
+ const clientError = await getClientErrorObject(
+ caughtError as Parameters<typeof getClientErrorObject>[0],
);
+
+ if (requestId === requestIdRef.current) {
+ const parsedError = clientError.errors?.[0] ?? {
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error' as const,
+ message: clientError.error,
+ };
Review Comment:
**Suggestion:** The fallback error uses `clientError.error` directly, but `getClientErrorObject` can produce a response without an `error` value, such as a network failure or an empty/non-JSON response. In that case `parsedError.message` becomes undefined and the error panel renders without a useful explanation. Use the parsed message/status text and a final localized fallback, as the existing CRUD error handling does. [error handling]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dataset metadata errors can display without explanatory text.
- ⚠️ Users receive poor feedback for empty JSON responses.
- ⚠️ Dataset creation troubleshooting becomes more difficult.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=30ddfe3146f942c1838cdc8d9a55021b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=30ddfe3146f942c1838cdc8d9a55021b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx
**Line:** 147:152
**Comment:**
*Error Handling: The fallback error uses `clientError.error` directly, but `getClientErrorObject` can produce a response without an `error` value, such as a network failure or an empty/non-JSON response. In that case `parsedError.message` becomes undefined and the error panel renders without a useful explanation. Use the parsed message/status text and a final localized fallback, as the existing CRUD error handling does.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42581&comment_hash=80e7bf52768fcdca08ef409d9140681036d9bd872fa9c4e6c8cc92a81e497aca&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42581&comment_hash=80e7bf52768fcdca08ef409d9140681036d9bd872fa9c4e6c8cc92a81e497aca&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42581:
URL: https://github.com/apache/superset/pull/42581#issuecomment-5126444506
<!-- Bito Reply -->
The flagged issue is correct. The current implementation relies on `clientError.error` as a fallback, which can be undefined if `getClientErrorObject` returns a response without an explicit error field (e.g., network failures or non-JSON responses).
To resolve this, you should update the error handling logic to prioritize `clientError.message` or `clientError.statusText` and provide a localized fallback string, similar to how CRUD error handling is implemented elsewhere in the codebase.
Here is a concise fix for the error handling block in `superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx`:
```tsx
const parsedError = clientError.errors?.[0] ?? {
error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
extra: null,
level: 'error' as const,
message: clientError.message || clientError.statusText || t('An unexpected error occurred'),
};
```
Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well?
**superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx**
```
const parsedError = clientError.errors?.[0] ?? {
error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
extra: null,
level: 'error' as const,
message: clientError.message || clientError.statusText || t('An unexpected error occurred'),
};
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset): retry metadata after OAuth2 authorization [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42581:
URL: https://github.com/apache/superset/pull/42581#issuecomment-5127914640
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org