aminghadersohi commented on PR #41921: URL: https://github.com/apache/superset/pull/41921#issuecomment-5049092971
Addressed the two remaining automated-review items in 664b5c7d8b573f7dc6a456123771c03deab0304d: **Unprotected metrics emission in the `finally` block** (CodeAnt) — valid: `_emit_call_metrics` was awaited unguarded, so a raising stats client could mask the tool's real result or exception. It's now wrapped in its own try/except (warning-logged, swallowed), matching the guard on the `event_logger.log` call. Two regression tests pin the behavior: with `incr` raising `RuntimeError`, a successful call still returns its result, and a failing call still propagates its original `ValueError` rather than the metrics error. **Missing OAuth test coverage for the `get_chart_data` re-raise fix** (Bito) — added `TestOAuthErrorRouting` driving the full tool through the FastMCP client with `ChartDataCommand.run` raising: query-time `OAuth2RedirectError` now provably returns `error_type="OAUTH2_REDIRECT"` (not a generic `DataError`), and `OAuth2Error` returns `error_type="OAUTH2_REDIRECT_ERROR"`. Also merged the latest master into the branch (includes #41923's dead-middleware removal — no conflicts with this PR's changes). Full MCP unit suite: 3125 passed locally. -- 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]
