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

   ### SUMMARY
   
   Related to #36840.
   
   `getDatasourceSamples` returns `response.json.result`, so a 200 response 
that carries no `result` resolves to `undefined`. `SamplesPane` then read 
fields straight off it:
   
   ```ts
   setData(ensureIsArray(response.data));
   ```
   
   which throws `TypeError: Cannot read properties of undefined (reading 
'data')`. The `.catch` in the same effect turns that into user-facing text, so 
the pane renders **"Failed to load samples — TypeError: Cannot read properties 
of undefined (reading 'data')"** — an internal error leaked into the UI, on a 
code path where the component already has a perfectly good empty state (`No 
samples were returned for this dataset`) that it never reaches.
   
   This reads through the response so that path falls back to the existing 
empty state.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Rendered output of the Samples tab for a 200 response whose body has no 
`result` key (taken from the regression test added here):
   
   | | rendered |
   | --- | --- |
   | before | `Failed to load samples` / `TypeError: Cannot read properties of 
undefined (reading 'data')` |
   | after | `No samples were returned for this dataset` |
   
   ### TESTING INSTRUCTIONS
   
   Automated — a regression test is included. It fails on `master` with the 
exact `TypeError` above and passes with this change:
   
   ```bash
   cd superset-frontend
   npx jest src/explore/components/DataTablesPane
   ```
   
   5 suites / 32 tests pass, including the new case.
   
   Manual:
   
   1. Open any chart in Explore and click the **Samples** tab.
   2. With `/datasource/samples` returning `200` and a body without a `result` 
key (the shape reported in #36840), the pane shows "No samples were returned 
for this dataset" rather than a `TypeError`.
   
   ### ADDITIONAL INFORMATION
   
   A note on scope, since #36840 is labelled `validation:required`: I could not 
reproduce the reported **UI freeze** on current `master` — the request path now 
settles and renders an error alert rather than hanging, so whatever caused the 
freeze appears to have been fixed since the report. What does still reproduce 
is the misleading error above, which is what this PR addresses; it matches the 
issue's stated expectation that the tab "display rows or show a 'No data 
available' message".
   
   One neighbouring case is deliberately left alone: a 200 with a genuinely 
**empty body** fails JSON parsing and surfaces `Failed to load samples — Error: 
Unexpected ''`. That is a malformed response, so an error state seems right 
there; only the "well-formed 200, no payload" case is treated as empty. Happy 
to fold that in if maintainers would rather both paths render the empty state.
   
   - [x] Has associated issue: #36840
   - [ ] 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
   


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

Reply via email to