rusackas opened a new pull request, #41839:
URL: https://github.com/apache/superset/pull/41839
### SUMMARY
Test-only PR that proves issue #39096 is already fixed on master, and closes
it.
closes #39096
The issue reported that with `GLOBAL_ASYNC_QUERIES` enabled, an
`OAuth2RedirectError` raised while loading chart data (e.g. Trino + Keycloak
OAuth2) was caught by the generic `Exception` handler in
`load_chart_data_into_cache` and flattened to a plain `{"message": "You don't
have permission to access the data."}`, losing the structured payload (`url`,
`tab_id`, `redirect_uri`) the frontend needs to render the OAuth2 banner.
That code path has since been fixed:
- #36596 added SIP-40 structured-error extraction to
`load_chart_data_into_cache` (the exact handler quoted in the issue), so any
`SupersetErrorException` — including `OAuth2RedirectError`, which subclasses it
— is serialized via `dataclasses.asdict(ex.error)` with its full
`error_type`/`level`/`extra` payload.
- #39897 extended the same treatment to the legacy
`load_explore_json_into_cache` path and added an OAuth2-specific test there.
What was still missing is a regression test for the issue's exact scenario:
`OAuth2RedirectError` raised by `ChartDataCommand.run` inside
`load_chart_data_into_cache`. The existing chart-path tests only cover a
generic `SupersetErrorException`, and the OAuth2-specific test from #39897 only
covers the explore-json path. This PR adds that complementary unit test, which
asserts the async job is updated with the structured `OAUTH2_REDIRECT` envelope
(including the `url`/`tab_id`/`redirect_uri` extras) rather than the flattened
permission message. The test passes on current master, confirming the reported
behavior is resolved and guarding against regression.
No production code is changed.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (test-only change)
### TESTING INSTRUCTIONS
```bash
python -m pytest tests/unit_tests/tasks/test_async_queries.py -x -q
```
All tests in the module pass, including the new
`test_load_chart_data_into_cache_preserves_oauth2_redirect_error`.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: closes #39096
- [ ] 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
🤖 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: [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]
