Skip to content

fix(adk): wrap tool execution errors into error output#596

Open
Deeven-Seru wants to merge 1 commit into
googleapis:mainfrom
Deeven-Seru:fix/toolbox-adk-wrap-tool-errors
Open

fix(adk): wrap tool execution errors into error output#596
Deeven-Seru wants to merge 1 commit into
googleapis:mainfrom
Deeven-Seru:fix/toolbox-adk-wrap-tool-errors

Conversation

@Deeven-Seru
Copy link
Copy Markdown
Contributor

@Deeven-Seru Deeven-Seru commented Mar 17, 2026

Summary- catch Toolbox tool execution exceptions and return an error response so ADK after-tool callbacks can handle recoverable failures- log tool execution failures for visibility- add a unit test covering exception-to-error wrapping
fixes googleapis/mcp-toolbox#2728

@Deeven-Seru Deeven-Seru requested a review from a team as a code owner March 17, 2026 13:18
@dishaprakash dishaprakash added the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Mar 24, 2026
@anubhav756
Copy link
Copy Markdown
Contributor

@Deeven-Seru Thanks for the PR!

Can you resolve the conflicts?

@anubhav756
Copy link
Copy Markdown
Contributor

/gcbrun

@Deeven-Seru
Copy link
Copy Markdown
Contributor Author

@anubhav756 could u please take a look & re initiate the checks ..thanks:)

@Deeven-Seru Deeven-Seru force-pushed the fix/toolbox-adk-wrap-tool-errors branch 2 times, most recently from eac456e to b80c202 Compare April 14, 2026 05:54
@anubhav756 anubhav756 force-pushed the fix/toolbox-adk-wrap-tool-errors branch from b80c202 to 3bf4b1c Compare April 14, 2026 21:11
@anubhav756
Copy link
Copy Markdown
Contributor

/gcbrun

@anubhav756
Copy link
Copy Markdown
Contributor

@anubhav756 could u please take a look & re initiate the checks ..thanks:)

Hi @Deeven-Seru!

Couldn't get to it earlier. Retriggered!

@Deeven-Seru
Copy link
Copy Markdown
Contributor Author

Deeven-Seru commented Apr 15, 2026

Fixes made for ADK check failures:

  • Restored packages/toolbox-adk/integration.cloudbuild.yaml and removed the invalid build step that changed directory to a non-existent path.
  • Fixed test stability in packages/toolbox-adk/tests/unit/test_tool.py (ValidationError import and assertion text).
  • Local verification: toolbox-adk unit test file passes (16 passed).

@anubhav756
Copy link
Copy Markdown
Contributor

/gcbrun

Copy link
Copy Markdown
Contributor

@anubhav756 anubhav756 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also take a look at the failures? Thanks!

Comment thread packages/toolbox-adk/src/toolbox_adk/tool.py
Comment thread packages/toolbox-adk/src/toolbox_adk/tool.py
@anubhav756
Copy link
Copy Markdown
Contributor

/gcbrun

@Deeven-Seru Deeven-Seru force-pushed the fix/toolbox-adk-wrap-tool-errors branch from d9ef159 to 6d609c8 Compare April 16, 2026 06:37
@anubhav756 anubhav756 force-pushed the fix/toolbox-adk-wrap-tool-errors branch from 6d609c8 to 02ee2c5 Compare April 17, 2026 12:20
@anubhav756 anubhav756 added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p2 Moderately-important priority. Fix may not be included in next release.

Projects

None yet

3 participants