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: [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]
