You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2026/09/23 21:50:55 UTC

[PR] fix(native-filters): show the actual error instead of "Network error" when filter values fail to load [superset]

eschutho opened a new pull request, #44585:
URL: https://github.com/apache/superset/pull/44585

   ### SUMMARY
   When a dashboard native filter fails to load its values, `FilterValue` hands the error to `ErrorMessageWithStackTrace`. That component only uses a registered renderer when the error's `error_type` matches the error message registry. For anything else it renders the `fallback`, and that fallback was a hardcoded **"Network error — Network error while attempting to fetch resource"**. The actual error message was dropped.
   
   So any failure without a registered `error_type` looked like a network problem. For example, a ClickHouse query failed with a resource limit error (`TOO_MANY_ROWS_OR_BYTES`), where the response was `{"errors": [{"message": "..."}]}` with no `error_type`, and the filter showed "Network error". That sends anyone debugging the dashboard after connectivity, permissions or timeout problems when the real cause is a database error.
   
   This change keeps `ErrorMessageWithStackTrace` and the compact `ErrorAlert`, and makes the fallback use the real message when there is one:
   
   - The message is taken from `errors[0].message`, then `error`, then `message`, which covers every shape `getClientErrorObject` produces. Those are a JSON `{errors: [...]}` response, a plain `Error`, and a `Failed to fetch` `TypeError`. The last one already resolves to `{ error: 'Network error' }`, so real network failures still say so.
   - If a message exists, the title is `Cannot load filter`, a string already translated and used by `FiltersConfigForm` for the same situation. The generic "Network error" title and text are only used when there is no message at all.
   - The compact `ErrorAlert` shows only the title inline. The message goes in the tooltip and in a modal opened by clicking, so long database errors don't break the filter bar layout.
   
   The change is limited to this one render site.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   **Before:** the filter shows `Network error`, and the tooltip/modal says `Network error while attempting to fetch resource`, whatever the actual failure was.
   
   **After:** the filter shows `Cannot load filter`, and the tooltip/modal shows the actual error, e.g. `Error: Received ClickHouse exception, code: 396 ... (TOO_MANY_ROWS_OR_BYTES)`. If the failure has no message at all, it still shows `Network error`.
   
   ### TESTING INSTRUCTIONS
   Unit tests:
   ```
   cd superset-frontend
   npx jest src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.test.tsx
   ```
   New cases cover:
   1. an `errors: [{ message }]` payload with no `error_type`, which shows the database message and not "Network error";
   2. a `{ error: '...' }` object with no `errors` array, which shows that message;
   3. no message at all, which still shows the "Network error" fallback.
   
   The existing error-state test was also updated, because its payload carries a message.
   
   Manual:
   1. Create a native select filter on a dataset whose values query fails in the database. For example, point it at a virtual dataset with invalid SQL, or at a column whose query hits a database resource limit.
   2. Open the dashboard. The filter should show "Cannot load filter", and hovering or clicking it should show the database error text instead of "Network error while attempting to fetch resource".
   3. Block the `/api/v1/chart/data` request in devtools. The filter should still report a network error.
   
   ### 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
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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(native-filters): show the actual error instead of "Network error" when filter values fail to load [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44585:
URL: https://github.com/apache/superset/pull/44585#issuecomment-5803542898

   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(native-filters): show the actual error instead of "Network error" when filter values fail to load [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44585:
URL: https://github.com/apache/superset/pull/44585#issuecomment-5803712359

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44585?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 81.06%. Comparing base ([`082961b`](https://app.codecov.io/gh/apache/superset/commit/082961ba168f18422a356c2df8b724d735eec9ca?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9422f24`](https://app.codecov.io/gh/apache/superset/commit/9422f243b88c0acda24f991bb2a75b9dc2a3b67a?dropdown=coverage&el=desc&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   #44585   +/-   ##
   =======================================
     Coverage   81.06%   81.06%           
   =======================================
     Files        2955     2955           
     Lines      178319   178323    +4     
     Branches    41312    41316    +4     
   =======================================
   + Hits       144549   144553    +4     
     Misses      31060    31060           
     Partials     2710     2710           
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/44585/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/44585/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.42% <100.00%> (+<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/44585?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(native-filters): show the actual error instead of "Network error" when filter values fail to load [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #44585:
URL: https://github.com/apache/superset/pull/44585#discussion_r4087711327


##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx:
##########
@@ -405,14 +405,24 @@ const FilterValue: FC<FilterValueProps> = ({
   );
 
   if (error) {
+    // Errors without a registered `error_type` (e.g. a raw database error) are
+    // rendered by the fallback, so surface their message rather than assuming
+    // a network failure.
+    const errorMessage =
+      error.errors?.[0]?.message || error.error || error.message;
     return (
       <ErrorMessageWithStackTrace
         error={error.errors?.[0]}
         compact
         fallback={
           <ErrorAlert
-            errorType={t('Network error')}
-            message={t('Network error while attempting to fetch resource')}
+            errorType={
+              errorMessage ? t('Cannot load filter') : t('Network error')
+            }
+            message={
+              errorMessage ||
+              t('Network error while attempting to fetch resource')
+            }

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Network fallback unreachable</b></div>
   <div id="fix">
   
   On a real network failure `getClientErrorObject` resolves `{ error: t('Network error') }` (getClientErrorObject.ts:193-200), so `errorMessage` is truthy and the fallback `t('Network error while attempting to fetch resource')` is unreachable — the alert now shows the terse 'Network error'. The new test mocks `{ error: '' }`, which never occurs for network errors, so it passes while production differs. Consider treating the generic network-error value as 'no message'.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #50386e</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
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(native-filters): show a clear error instead of "Network error" when filter values fail to load [superset]

Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho merged PR #44585:
URL: https://github.com/apache/superset/pull/44585


-- 
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(native-filters): show a clear error instead of "Network error" when filter values fail to load [superset]

Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #44585:
URL: https://github.com/apache/superset/pull/44585#discussion_r4163603010


##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx:
##########
@@ -292,6 +294,15 @@ const FilterValue: FC<FilterValueProps> = ({
         })
         .catch((error: Response) => {
           getClientErrorObject(error).then(clientErrorObject => {
+            // Raw error details stay in devtools; they are not rendered.
+            logging.warn('Failed to load filter values', clientErrorObject);
+            // `fetch` rejects with a TypeError (in every browser) only when no
+            // response was received; anything else was reported by the server.
+            setIsNetworkError(
+              error instanceof TypeError &&

Review Comment:
   The catch here covers the whole `requestChartDataResolved` chain, not just `fetch`, so a plain JS TypeError from our own code (say `'result' in json` in `extractResult` getting an undefined body) also has no status and no errors, and gets labelled "Network error".
   
   That's the same misdirection this PR is fixing, just on a rarer path. Could we narrow it, e.g. only treat it as network when the TypeError message matches the known fetch wordings, or accept that and say so in the comment?



##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx:
##########
@@ -405,14 +416,22 @@ const FilterValue: FC<FilterValueProps> = ({
   );
 
   if (error) {
+    // Errors without a registered `error_type` are rendered by the fallback.
+    // Server error text can expose database internals, so it is not shown.

Review Comment:
   This only holds for the fallback. `GENERIC_DB_ENGINE_ERROR` and `GENERIC_BACKEND_ERROR` are registered to `DatabaseErrorMessage` in `setupErrorMessages.ts`, and that renders the raw engine message, so most DB failures that come back typed still show their text to viewers.
   
   Fine if that's out of scope, but mind scoping this comment (and the description) to untyped errors so nobody reads it as a guarantee?



-- 
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(native-filters): show a clear error instead of "Network error" when filter values fail to load [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #44585:
URL: https://github.com/apache/superset/pull/44585#issuecomment-6067051522

   ## CodeAnt PR Risk: Medium Risk
   
   - The PR needs attention before merging.
   - Server errors are still passed unchanged to ErrorMessageWithStackTrace, while the new tests cover only errors without an error_type.
   - Confirm that registered error handlers cannot display database details before relying on this change to prevent their exposure.
   
   **Assessed commit:** `15c54f1249f6`


-- 
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(native-filters): show a clear error instead of "Network error" when filter values fail to load [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44585:
URL: https://github.com/apache/superset/pull/44585#issuecomment-5805878544

   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 an incremental AI Review.
   - `/review full` - 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