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

   ## Summary
   Two Sentry issues share a root cause:
   - 
[SUPERSET-PYTHON-YY5](https://preset-inc.sentry.io/issues/SUPERSET-PYTHON-YY5) 
— `BaseSSHTunnelForwarderError: Could not establish session to SSH gateway`, 
`ChartDataRestApi.data`, 2041 events / 34 users
   - 
[SUPERSET-PYTHON-T7B](https://preset-inc.sentry.io/issues/SUPERSET-PYTHON-T7B) 
— same exception, `DashboardRestApi.get_datasets`, 3006 events / 25 users
   
   Both are raised from `Database.get_sqla_engine()` 
(`superset/models/core.py`) when a workspace's configured SSH tunnel gateway is 
unreachable or misconfigured — an expected, customer-environment-driven failure 
(analogous to a DB connection failure), not a Superset bug.
   
   ## Root cause
   `sshtunnel.BaseSSHTunnelForwarderError` is not a `SupersetException` 
subtype, so it isn't caught by any specific handler in 
`superset/views/error_handling.py`. It falls into one of two ERROR-level 
catch-alls depending on whether the endpoint is decorated with 
`@handle_api_exception`:
   
   1. **`handle_api_exception.wraps`** — used by 
`DashboardRestApi.get_datasets` and other `@handle_api_exception`-decorated 
endpoints. Confirmed via T7B's Sentry stacktrace (`error_handling.py:100 wraps` 
frame present): hits the final broad `except Exception as ex: 
logger.exception(ex)`.
   2. **`show_unexpected_exception`** (the global 
`@app.errorhandler(Exception)`) — `ChartDataRestApi.data` has no 
`@handle_api_exception` decorator; YY5's full Sentry stacktrace has *no* 
`error_handling.py` frame at all, confirming it propagates all the way to this 
global handler, which currently double-logs at ERROR (`logger.warning(..., 
exc_info=True)` + `logger.exception(ex)`).
   
   Both paths also return an opaque generic 500 with no actionable error type.
   
   Checked for existing SSH-tunnel-specific handling: 
`superset/commands/database/ssh_tunnel/exceptions.py` only covers config-time 
validation (creating/updating a DB's SSH tunnel settings) — nothing exists for 
query-time connection failures.
   
   ## Fix
   Added a shared `handle_ssh_tunnel_error()` helper in 
`superset/views/error_handling.py` that logs at WARNING (not ERROR — 
expected/environmental, not a code bug) and returns a structured 
`SupersetError` (`error_type=CONNECTION_HOST_DOWN_ERROR`, HTTP 400) instead of 
the opaque 500. Wired into both catch-alls so the fix covers 
`@handle_api_exception`-decorated and non-decorated endpoints alike.
   
   Reused the existing `CONNECTION_HOST_DOWN_ERROR` type (already used 
elsewhere for "host might be down" DB connection failures, e.g. 
`superset/db_engine_specs/postgres.py`) rather than adding a new type for a 
single call site.
   
   ## Tradeoffs
   - **No functional/connectivity change** — this only changes logging level 
and the shape of the error response for an already-failing request. No 
raise→suppress or silently-swallowed-error semantics introduced.
   - **HTTP status code changes 500 → 400** for these two failure modes. This 
is a real, user-visible behavior change for any API client that specifically 
branches on status code, even though 400 is the more correct code for a 
client/environment-config-driven failure. Flagging explicitly per our 
disclosure convention.
   - Users now get an actionable "Failed to establish an SSH tunnel to the 
database: ..." message instead of a generic 500 — incidental UX improvement, 
not the primary goal.
   
   ## Testing
   Added `tests/unit_tests/views/test_error_handling.py` (3 new tests, all 
passing):
   - `@handle_api_exception`-wrapped view raising the SSH tunnel error → 400 + 
`CONNECTION_HOST_DOWN_ERROR`, asserts no ERROR-level log record emitted.
   - `show_unexpected_exception` given the SSH tunnel error → same structured 
400, no ERROR-level log.
   - Regression check: `show_unexpected_exception` given a generic `ValueError` 
still returns the original 500 + ERROR-level log (SIP-40 catch-all untouched).
   
   `ruff check` / `ruff format --check` (CI-pinned `ruff==0.9.7`) clean. `mypy` 
clean on both changed files (pre-existing unrelated errors in 
transitively-imported modules unchanged before/after, verified via `git stash` 
comparison).
   
   ## Links
   - Shortcut: https://app.shortcut.com/preset/story/115347
   
   Fixes SUPERSET-PYTHON-YY5
   Fixes SUPERSET-PYTHON-T7B
   
   🤖 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]

Reply via email to