bito-code-review[bot] commented on code in PR #38254:
URL: https://github.com/apache/superset/pull/38254#discussion_r2854680223
##########
superset/mcp_service/chart/tool/get_chart_preview.py:
##########
@@ -2110,14 +2110,7 @@ async def get_chart_preview(
)
return result
- except (
- CommandException,
- SupersetException,
- ValueError,
- KeyError,
- AttributeError,
- TypeError,
- ) as e:
+ except Exception as e:
Review Comment:
<!-- Bito Reply -->
The suggestion to replace the broad Exception catch with specific types
isn’t appropriate here. As outermost tool-level handlers, they act as the last
line of defense to catch any unexpected exceptions, preventing unhandled
crashes and ensuring the MCP client receives structured error responses. Inner
handlers in the file already use specific exceptions for proper error
categorization.
##########
superset/mcp_service/chart/tool/get_chart_data.py:
##########
@@ -691,7 +691,7 @@ async def get_chart_data( # noqa: C901
error_type="DataError",
)
- except (CommandException, SupersetException, ValueError, KeyError) as e:
+ except Exception as e:
await ctx.error(
"Chart data retrieval failed: identifier=%s, error=%s,
error_type=%s"
% (
Review Comment:
<!-- Bito Reply -->
The suggestion to catch specific exceptions isn't appropriate here. As the
outermost tool-level handler, broad Exception ensures the MCP client always
receives a structured error response instead of an unhandled crash. Inner
handlers in the file already use specific exception types for proper
categorization.
--
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]
