gaurav0107 opened a new pull request, #42489:
URL: https://github.com/apache/superset/pull/42489
### SUMMARY
Fixes #33500.
When an API error message happens to contain an HTML tag, the UI throws the
whole message away and shows a generic `Unexpected error Bad request`. The
reproduction in the issue is a custom metric containing `<a>`: the database
rejects it, `/api/v1/chart/data` returns the full syntax error in `message`,
and the user sees nothing useful.
Root cause is in `checkForHtml()`:
```ts
export function checkForHtml(str: string): boolean {
return !isJsonString(str) && isProbablyHTML(str);
}
```
`isProbablyHTML()` answers *"does this string contain a known HTML tag
anywhere?"*. That is the right question for the rendering helpers it was
written for (`safeHtmlSpan`, `sanitizeHtmlIfNeeded`, used on table cell
values), but the wrong one on the error path, which needs *"is this an HTML
error page rather than prose?"*. A driver message that quotes `<a>`
mid-sentence trips the tag scan, so `parseErrorJson()` hands it to
`retrieveErrorMessage()`, which prefers the HTTP status and returns
`Bad request`.
The fix requires the string to *begin* with markup before it can be treated
as
an error page. A page served by a proxy or gateway starts with
`<!doctype html>` or a tag; a server-authored message starts with prose. This
preserves the behaviour #29321 added — raw gateway HTML is still collapsed
into a status-code message — while letting real diagnostics through.
The guard goes in `checkForHtml()` rather than at either call site, so both
consumers (`parseStringResponse()` and `parseErrorJson()`, and therefore both
the string, JSON and text-fallback branches of `getClientErrorObject()`) are
covered by one change. The shared `isProbablyHTML()` in `utils/html.tsx` is
deliberately left alone.
No new sanitisation is needed: the resulting string is rendered as a React
text child (`BasicErrorAlert` renders `<p>{body}</p>`), and there is no
`dangerouslySetInnerHTML` anywhere on the error path.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No visual change beyond the error text itself. For the message in the issue:
**Before** — `Unexpected error` / `Bad request`
**After** — `Unexpected error` / `Error: HTTPDriver ... DB::Exception:
Syntax error: failed at position 37 ('<') (line 1, col 37): <a> AS ...`
### TESTING INSTRUCTIONS
Automated:
```bash
cd superset-frontend
npx cross-env NODE_ENV=test npx jest \
packages/superset-ui-core/test/query \
packages/superset-ui-core/src/utils/html.test.tsx \
src/middleware/asyncEvent.test.ts
```
The new test `parseErrorJson keeps a server message that only quotes an HTML
tag` fails on `master` with `error: "Bad request"` and passes with this
change. The four pre-existing HTML tests in that file are the regression
guard for #29321 and stay green — their fixtures (`<div>...`,
`<!doctype html>...`) all start with markup. `asyncEvent.test.ts` covers the
one caller of `parseErrorJson()` outside this module.
Manual:
1. Create a chart against any database whose driver echoes the failing
expression back (ClickHouse in the issue report).
2. Add a custom metric whose expression contains `<a>`.
3. Press **UPDATE CHART**.
4. The actual database error is shown instead of `Bad request`.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #33500
- [ ] Required feature flags:
- [ ] 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: [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]
