aminghadersohi commented on code in PR #38254:
URL: https://github.com/apache/superset/pull/38254#discussion_r2854675768
##########
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:
This is intentionally broad. These are outermost tool-level handlers — the
last line of defense before the MCP client. If an unexpected exception escapes
here, the client gets an unhandled crash instead of a structured error
response. Inner handlers throughout the file still catch specific exception
types 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:
Same as above — this is the outermost tool-level handler. Broad Exception is
correct here to ensure the MCP client always gets a structured error response.
Inner handlers still use specific exception types.
--
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]
