mikebridge opened a new pull request, #45537: URL: https://github.com/apache/superset/pull/45537
### SUMMARY When a semantic layer provider fails while listing its dimensions or metrics, Superset should report a server error. It should not tell the user their request was wrong, and it should not show the provider's internal error text. This PR fixes both. It also makes the semantic query mapper say exactly which of its errors are user mistakes. Before this change: - If the provider failed in `get_dimensions()` while a chart-data request was being parsed, the API's generic `ValueError` handler returned **400 "Request is incorrect"** with the provider's exception text in the body. - If it failed in `get_metrics()`, the request returned 500. The generic error handler still put the provider's exception text in the JSON body. - The mapper used built-in `ValueError` for real input refusals (unknown columns, bad filter values, bad time ranges). It caught every `ValueError` in `get_results` / `validate_query_object` and turned it into a 400, so a bug that happened to raise `ValueError` was reported as a user error. Changes: - `discover_semantic_metadata()` (next to `execute_semantic_query()` in `superset/semantic_layers/exceptions.py`) wraps provider metadata reads on the chart-data path. Host signals (validation, security, OAuth, cancellation, existing semantic errors) pass through unchanged. Any other provider exception is logged and becomes `SemanticLayerExecutionError`: a 500 with the generic "An error occurred while fetching the data." message. The two helpers now share one list of passthrough exceptions. - Every `SemanticView` read of provider metadata goes through it (`metrics`, `columns`, `data`, time grains, the drill-by access check, compatibility lookups). So do the mapper's per-query metadata snapshot and the semantic filter validation in `QueryObject.validate`. The wrapper logs only the exception type, which matches the existing dashboard datasets policy, because provider messages can contain credentials or URLs. - The mapper raises `SemanticQueryInputError` for input refusals. It subclasses `ValueError`, so existing direct callers that catch `ValueError` (MCP chart helpers, `QueryObject.validate`) behave as before. A small adapter turns `ValueError`s from the date/time-range parsers (for example, "From date cannot be larger than to date") into this type. `get_results` and `validate_query_object` now convert only `SemanticQueryInputError` to `QueryObjectValidationError` (400). Any other exception propagates as a 500. User-facing 400 messages for real input mistakes are unchanged. Not changed here: the semantic layer REST endpoints and the MCP semantic view tool read provider metadata under their own error handling. Provider construction and the compatibility endpoints' `get_compatible_*` calls also stay outside the classifier. Over HTTP, Superset's standard `SupersetErrorException` handler still logs the chained exception, the same as for query execution faults. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (API behavior) | Provider/mapper failure | Before | After | |---|---|---| | `get_dimensions()` raises during chart data | 400, provider text in body | 500, generic message | | `get_metrics()` raises during chart data | 500, provider text in body | 500, generic message | | Unexpected `ValueError` inside the mapper | 400 | 500 | | Unknown column / bad filter value / reversed time range | 400 | 400 (same message) | ### TESTING INSTRUCTIONS ```bash pytest tests/unit_tests/semantic_layers/query_errors_test.py pytest tests/unit_tests/semantic_layers ``` The new tests in `query_errors_test.py` cover: - both discovery methods, with and without filters, over HTTP: status, generic message, provider text not exposed, one error log entry, and no query executed; - each `SemanticView` metadata read, plus the dashboard datasets path, with a real provider fault: no provider text in the log; - each date/time parser adapter; - an injected `ValueError` during member lookup and during mapping: HTTP 500, and the original exception from `get_results`; - invalid selections and time ranges: still 400, typed; - the helper contract: `SemanticQueryInputError` is a `ValueError`. ### ADDITIONAL INFORMATION Follow-up to #45292. - [ ] Has associated issue: - [ ] 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]
