feat(plugins): add on_agent_error_callback and on_run_error_callback#5045
Draft
caohy1988 wants to merge 4 commits intogoogle:mainfrom
Draft
feat(plugins): add on_agent_error_callback and on_run_error_callback#5045caohy1988 wants to merge 4 commits intogoogle:mainfrom
caohy1988 wants to merge 4 commits intogoogle:mainfrom
Conversation
Implements RFC google#5044 to close the error callback coverage gap at the agent and runner levels. Previously, unhandled exceptions escaping agent execution or runner execution left dangling *_STARTING events with no terminal error event. Changes: - Add on_agent_error_callback and on_run_error_callback to BasePlugin (notification-only, exception always re-raised) - Wire try/except Exception in BaseAgent.run_async() and run_live() - Wire try/except Exception in Runner._exec_with_plugin() - Add AGENT_ERROR and INVOCATION_ERROR event types to BigQueryAgentAnalyticsPlugin with traceback capture and cleanup - Keep after_agent_callback and after_run_callback as success-only (no semantic change to existing callbacks) - Catch Exception (not BaseException) to exclude CancelledError/ KeyboardInterrupt from error callback dispatch Closes google#4863 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
adk-bot
added
the
core
Collaborator
|
Response from ADK Triaging Agent Hello @caohy1988, thank you for creating this PR! It looks like you haven't signed the Contributor License Agreement (CLA) yet. Please visit https://cla.developers.google.com/ to sign the agreement. Once you've signed it, the "cla/google" check will pass. Thanks! |
Error callbacks are documented as notification-only (all plugins must be notified), but were dispatched through _run_callbacks() which short-circuits on the first non-None return. A buggy plugin returning a value would prevent subsequent plugins from seeing the error. Add _run_notification_callbacks() that always iterates all plugins regardless of return values, and switch run_on_agent_error_callback() and run_on_run_error_callback() to use it. Update tests to assert both plugins are always called even when one returns non-None. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
caohy1988
added a commit
to caohy1988/adk-python
that referenced
this pull request
Mar 28, 2026
Follow-up to google#5045. Adds BQ plugin-specific test coverage and enriches the analytics views for AGENT_ERROR and INVOCATION_ERROR events. Changes: - Add error_traceback column extraction to v_agent_error and v_invocation_error views (JSON_VALUE from content field) - Add test_on_agent_error_callback_logs_correctly: verifies AGENT_ERROR event is logged with correct error_message, status, and traceback - Add test_on_run_error_callback_logs_correctly: verifies INVOCATION_ERROR event is logged with correct fields - Add TestRunErrorCallbackCleanupSafety: verifies TraceManager cleanup runs even when _log_event raises during on_run_error_callback - Add test_error_views_contain_traceback_column: verifies view SQL includes error_traceback extraction Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 tasks
Follow-up to google#5045. Adds BQ plugin-specific test coverage and enriches the analytics views for AGENT_ERROR and INVOCATION_ERROR events. Changes: - Add error_traceback column extraction to v_agent_error and v_invocation_error views (JSON_VALUE from content field) - Add test_on_agent_error_callback_logs_correctly: verifies AGENT_ERROR event is logged with correct error_message, status, and traceback - Add test_on_run_error_callback_logs_correctly: verifies INVOCATION_ERROR event is logged with correct fields - Add TestRunErrorCallbackCleanupSafety: verifies TraceManager cleanup runs even when _log_event raises during on_run_error_callback - Add test_error_views_contain_traceback_column: verifies view SQL includes error_traceback extraction Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
_run_notification_callbacks() previously raised RuntimeError on the first plugin callback failure, which masked the original application exception and prevented later plugins from being notified. Now it logs the error and continues iterating all plugins. The caller's original exception (agent crash / runner crash) remains the primary error that propagates to the caller. Add regression tests: - test_plugin_callback_failure_does_not_mask_app_error: verifies p2 is still notified when p1's error callback raises - test_original_exception_propagates_despite_plugin_failure: end-to-end test verifying the caller sees the original agent exception, not the plugin's internal error Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements RFC #5044 to close the error callback coverage gap at the agent and runner levels (fixes #4863).
on_agent_error_callback(callback_context, agent, error)andon_run_error_callback(invocation_context, error)toBasePlugintry/except ExceptioninBaseAgent.run_async(),run_live(), andRunner._exec_with_plugin()AGENT_ERRORandINVOCATION_ERRORevent types toBigQueryAgentAnalyticsPluginwith traceback capture and TraceManager cleanupv_agent_errorandv_invocation_erroranalytics views witherror_tracebackcolumnafter_agent_callbackandafter_run_callbackas success-only — no semantic change to existing callbacksException, notBaseException—CancelledError,KeyboardInterrupt,SystemExitdo not trigger error callbacks_run_notification_callbacks()ensures all plugins are notified regardless of return valuesDesign decisions
on_model_error_callback). The exception is always re-raised.on_agent_error_callback+ 1×on_run_error_callback. Neither fires more than once per failure.after_*semantic change: All 6 built-inafter_*callback usages (logging_plugin,debug_logging_plugin,bigquery_agent_analytics_plugin) treat them as success callbacks. Moving tofinallywould produce dual events (e.g., bothAGENT_ERRORandAGENT_COMPLETED)._run_notification_callbacks()always iterates all plugins, unlike_run_callbacks()which short-circuits on non-None returns.Files changed
base_plugin.pyplugin_manager.pyPluginCallbackNameentries, +2 dispatch methods, +_run_notification_callbacks()base_agent.pytry/except Exceptioninrun_async()/run_live(),_handle_agent_error_callback()runners.pytry/except Exceptionin_exec_with_plugin()bigquery_agent_analytics_plugin.pyAGENT_ERROR/INVOCATION_ERRORevents, traceback capture, cleanup, view columnstest_error_callbacks.pytest_bigquery_agent_analytics_plugin.pyTest plan
after_*NOT called on failure, exception re-raised,CancelledErrordoesn't trigger, exactly-once-per-layer, notification dispatch (no short-circuit)test_base_agent.pytests pass (0 regressions)test_plugin_manager.pytests pass (0 regressions)🤖 Generated with Claude Code