eschutho opened a new pull request, #43934: URL: https://github.com/apache/superset/pull/43934
### SUMMARY `QueryEstimationCommand.run()` only caught `SupersetTimeoutException` around the `estimate_query_cost()` call. Any raw DBAPI exception (e.g. `psycopg2.errors.UndefinedTable` from `EXPLAIN` on invalid SQL against Postgres) propagated uncaught through the call stack until Flask's global exception handler turned it into an opaque, unclassified 500-class response. The sibling execution path (`SynchronousSqlJsonExecutor.execute()` in `superset/sqllab/sql_json_executer.py`) already converts raw DBAPI errors to `SupersetGenericDBErrorException` (status=400, `GENERIC_DB_ENGINE_ERROR`). This PR applies the same pattern to the cost-estimation path. **What changed:** Added an `except Exception` clause after the existing `except SupersetTimeoutException` handler in `QueryEstimationCommand.run()`. The new clause logs the exception and re-raises it as `SupersetGenericDBErrorException`, giving the frontend a typed, actionable 400 error instead of an opaque 500. This is part of the ongoing raw-exception-cleanup effort (see #42366, #42401, #43725, #43772, #43795, #43888). ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not applicable (backend-only error handling change). **Before:** Clicking "Estimate cost" on invalid SQL against a Postgres connection returns an opaque 500 response with a raw `psycopg2` traceback. **After:** The same action returns a structured 400 response with `error_type: GENERIC_DB_ENGINE_ERROR` and the database's error message. ### TESTING INSTRUCTIONS 1. Connect to a Postgres database in SQL Lab 2. Enter invalid SQL (e.g. `SELECT 1 FROM nonexistent_table`) 3. Click "Estimate cost" 4. **Before this PR:** Opaque 500 error 5. **After this PR:** Clean 400 error with the database's error message (e.g. `relation "nonexistent_table" does not exist`) Automated: `pytest tests/unit_tests/commands/sql_lab/test_estimate.py::test_run_wraps_raw_dbapi_error_from_cost_estimation` — verified fail-before/pass-after. ### ADDITIONAL INFORMATION - [ ] 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 **Merge-order note:** PR #43888 (JSONDecodeError catch in the same try block) is still open. Both PRs touch the same `try/except` block in `estimate.py`. They are independent — this PR's `except Exception` clause goes after any narrower catches. If #43888 merges first, this PR's rebase will place `except Exception` after the new `except json.JSONDecodeError` clause, which is the correct order. If this PR merges first, #43888's rebase is similarly clean. **No tradeoffs beyond the intended one:** previously-opaque 500s on bad SQL during cost estimation now correctly 400 with a typed error. -- 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]
