fix(dataset): retry metadata after OAuth2 authorization - #42581
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #42581 +/- ##
=======================================
Coverage 65.33% 65.34%
=======================================
Files 2803 2803
Lines 158490 158523 +33
Branches 36178 36190 +12
=======================================
+ Hits 103557 103589 +32
- Misses 52922 52923 +1
Partials 2011 2011
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
dosubot
Bot
added
authentication:sso
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
|
The flagged issue is correct. The current implementation relies on To resolve this, you should update the error handling logic to prioritize Here is a concise fix for the error handling block in 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 |
Code Review Agent Run #4d356dActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
rebenitez1802
left a comment
rebenitez1802
left a comment
There was a problem hiding this comment.
Request changes — one robustness regression to guard before merge.
Nice fix overall — the OAuth2 retry, the request-ID race guard, and the BroadcastChannel/storage dedup all look sound, and the coverage is good. One blocking item:
Catch-block fallback can pass message: undefined into a component that crashes on it.
When table_metadata rejects with a non-2xx JSON body that has no structured errors[] and no usable error/message (e.g. {} or { "errors": [] }), getClientErrorObject → parseErrorJson returns error: undefined — it's typed string, but the final return { ...error, error: error.error } propagates undefined. The catch fallback then builds { error_type: GENERIC_BACKEND_ERROR, extra: null, level: 'error', message: undefined }.
Because that fallback pins error_type: GENERIC_BACKEND_ERROR, ErrorMessageWithStackTrace routes it through the registry to DatabaseErrorMessage (setupErrorMessages.ts), which does message.split('\n') with no guard (DatabaseErrorMessage.tsx) → TypeError during render, blanking the dataset preview. Pre-PR, this same input rendered the benign "Unable to load columns" MessageContent, so it's a regression from a soft error state to a hard render crash.
One-line guard suggested inline below. Everything else I found was low-severity (test gaps for the stale-response/deselect paths and the second-tab_id dedup round; the errorMitigationFunction naming vs the onX convention; a duplicated generic-error literal) and is non-blocking — happy to share those separately if useful.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #24ac66Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
2 participants
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
SupersetClientcall 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.tsxfor error parsing and scoped retry, then followerrorMitigationFunctionthroughErrorMessageWithStackTraceintoOAuth2RedirectMessage.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
Automated validation:
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