codeant-ai-for-open-source[bot] commented on code in PR #37131:
URL: https://github.com/apache/superset/pull/37131#discussion_r3402725699
##########
superset-frontend/src/explore/exploreUtils/index.ts:
##########
@@ -396,11 +397,53 @@ export const exportChart = async ({
exportType: resultFormat,
});
} else {
- // SupersetClient.postForm calls getUrl({ endpoint }) internally, which
prepends
+ // Use AJAX blob download instead of form submission to enable error
handling.
+ // SupersetClient.postBlob calls getUrl({ endpoint }) internally, which
prepends
// appRoot — so the URL must NOT be pre-prefixed here.
- SupersetClient.postForm(url as string, {
- form_data: safeStringify(payload),
- });
+ try {
+ const response = await SupersetClient.postBlob(url as string, {
+ form_data: safeStringify(payload),
+ });
+
+ const extension = resultFormat === 'xlsx' ? 'xlsx' : resultFormat;
+ const timestamp = new Date()
+ .toISOString()
+ .replace(/[:.]/g, '-')
+ .slice(0, -5);
+ const filename = `chart_export_${timestamp}.${extension}`;
+
+ const blob = await response.blob();
+ downloadBlob(blob, filename);
Review Comment:
**🟠 Architect Review — HIGH**
The download filename is now derived solely from the requested resultFormat,
which ignores the actual response type; for multi-query table-like exports the
backend returns a ZIP (Content-Type application/zip and "zip" download
headers), so the frontend saves ZIP content with a .csv/.xlsx extension.
**Suggestion:** Inspect response headers (Content-Disposition/Content-Type)
before naming the file, using a .zip extension and/or server-provided filename
when the backend bundles multi-query results as a ZIP, and only fall back to
the resultFormat-derived extension when no explicit type is provided.
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bca7aeb4f3c24a91913cf98a7edb0bc6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=bca7aeb4f3c24a91913cf98a7edb0bc6&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 an **Architect / Logical Review** comment left during a code review.
These reviews are first-class, important findings — not optional suggestions.
Do NOT dismiss this as a 'big architectural change' just because the title says
architect review; most of these can be resolved with a small, localized fix
once the intent is understood.
**Path:** superset-frontend/src/explore/exploreUtils/index.ts
**Line:** 408:416
**Comment:**
*HIGH: The download filename is now derived solely from the requested
resultFormat, which ignores the actual response type; for multi-query
table-like exports the backend returns a ZIP (Content-Type application/zip and
"zip" download headers), so the frontend saves ZIP content with a .csv/.xlsx
extension.
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.
If a suggested approach is provided above, use it as the authoritative
instruction. If no explicit code suggestion is given, you MUST still draft and
apply your own minimal, localized fix — do not punt back with 'no suggestion
provided, review manually'. Keep the change as small as possible: add a guard
clause, gate on a loading state, reorder an await, wrap in a conditional, etc.
Do not refactor surrounding code or expand scope beyond the finding.
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>
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -406,46 +438,59 @@ export const useExploreAdditionalActionsMenu = (
streamingThreshold,
slice,
startExport,
+ handleExportError,
]);
- const exportCSVPivoted = useCallback(
- () =>
- canDownloadCSV
- ? exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'post_processed',
- resultFormat: 'csv',
- })
- : null,
- [canDownloadCSV, latestQueryFormData, ownState],
- );
+ const exportCSVPivoted = useCallback(async () => {
+ if (!canDownloadCSV) {
+ return null;
+ }
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'post_processed',
+ resultFormat: 'csv',
+ });
+ } catch (error) {
+ handleExportError(error);
+ }
+ return null;
+ }, [canDownloadCSV, latestQueryFormData, ownState, handleExportError]);
- const exportJson = useCallback(
- () =>
- canDownloadCSV
- ? exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'results',
- resultFormat: 'json',
- })
- : null,
- [canDownloadCSV, latestQueryFormData, ownState],
- );
+ const exportJson = useCallback(async () => {
+ if (!canDownloadCSV) {
+ return null;
+ }
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'results',
+ resultFormat: 'json',
+ });
+ } catch (error) {
+ handleExportError(error);
+ }
+ return null;
+ }, [canDownloadCSV, latestQueryFormData, ownState, handleExportError]);
- const exportExcel = useCallback(
- () =>
- canDownloadCSV
- ? exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'results',
- resultFormat: 'xlsx',
- })
- : null,
- [canDownloadCSV, latestQueryFormData, ownState],
- );
+ const exportExcel = useCallback(async () => {
+ if (!canDownloadCSV) {
+ return null;
+ }
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'results',
+ resultFormat: 'xlsx',
+ });
+ } catch (error) {
+ handleExportError(error);
+ }
+ return null;
+ }, [canDownloadCSV, latestQueryFormData, ownState, handleExportError]);
Review Comment:
**🟠 Architect Review — HIGH**
The error-handling coverage in Explore is incomplete: the "Export Current
View → Export to .CSV" server path calls exportChart(...) without awaiting or
wrapping it in try/catch, so HTTP 413 and other export failures reject the
Promise without showing the new user-friendly toast.
**Suggestion:** Mirror the pattern used by
exportCSV/exportCSVPivoted/exportJson/exportExcel by making the current-view
CSV server path async, awaiting exportChart, and routing any caught errors
through handleExportError so 413 and other failures consistently surface as
toasts.
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=06cd525223b94e2a8c48647a4f6d968b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=06cd525223b94e2a8c48647a4f6d968b&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 an **Architect / Logical Review** comment left during a code review.
These reviews are first-class, important findings — not optional suggestions.
Do NOT dismiss this as a 'big architectural change' just because the title says
architect review; most of these can be resolved with a small, localized fix
once the intent is understood.
**Path:**
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx
**Line:** 444:493
**Comment:**
*HIGH: The error-handling coverage in Explore is incomplete: the
"Export Current View → Export to .CSV" server path calls exportChart(...)
without awaiting or wrapping it in try/catch, so HTTP 413 and other export
failures reject the Promise without showing the new user-friendly toast.
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.
If a suggested approach is provided above, use it as the authoritative
instruction. If no explicit code suggestion is given, you MUST still draft and
apply your own minimal, localized fix — do not punt back with 'no suggestion
provided, review manually'. Keep the change as small as possible: add a guard
clause, gate on a loading state, reorder an await, wrap in a conditional, etc.
Do not refactor surrounding code or expand scope beyond the finding.
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>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
