-
Notifications
You must be signed in to change notification settings - Fork 1
Implement proper ADK error handling using native error fields instead of sentinel strings #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
3b778ba
c6f9c1c
ea5b51e
2b78cd8
608f3a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,8 +142,9 @@ import Portal from "./portal"; | |
| import { agentsWorking } from "@/components/AppAgents.vue"; | ||
| import type { AgentId } from "@/api/agents"; | ||
| import { aiWriter, aiWriterAsync } from "@/api/endpoints"; | ||
| import { type ADKSessionResponse, ensureSessionExists, extractADKText } from "@/api/adk"; | ||
| import { type ADKSessionResponse, ensureSessionExists, extractADKText, ManugenError } from "@/api/adk"; | ||
| import example from "./example.txt?raw"; | ||
| import { toast } from 'vue3-toastify'; | ||
|
|
||
| /** app info */ | ||
| const { VITE_TITLE: title } = import.meta.env; | ||
|
|
@@ -170,7 +171,7 @@ onMounted(() => { | |
| adkUsername.value, | ||
| adkSessionId.value | ||
| ).then((data) => { | ||
| sessionData.value = data; | ||
| sessionData.value = data as ADKSessionResponse; | ||
| console.log("ADK session created:", data); | ||
| }).catch((error) => { | ||
| console.error("Error creating ADK session:", error); | ||
|
|
@@ -333,25 +334,87 @@ const action = | |
| /** tell agents component that these agents are working in this portal */ | ||
| agentsWorking.value[portalId] = agents; | ||
|
|
||
| /** run actual work func, providing context */ | ||
| const result = await func(context); | ||
|
|
||
| /** tell agents component that work is done */ | ||
| delete agentsWorking.value[portalId]; | ||
|
|
||
| /** find node of portal created earlier */ | ||
| const portalNode = findPortal(portalId); | ||
| if (!portalNode) return; | ||
|
|
||
| editor.value | ||
| .chain() | ||
| /** delete portal node */ | ||
| .deleteRange({ | ||
| from: portalNode.pos, | ||
| to: portalNode.pos + portalNode.node.nodeSize, | ||
| }) | ||
| .insertContentAt(portalNode.pos, paragraphizeToJSON(result)) | ||
| .run(); | ||
| try { | ||
| /** run actual work func, providing context */ | ||
| const result = await func(context); | ||
|
|
||
| /** tell agents component that work is done */ | ||
| delete agentsWorking.value[portalId]; | ||
|
|
||
| /** find node of portal created earlier */ | ||
| const portalNode = findPortal(portalId); | ||
| if (!portalNode) return; | ||
|
|
||
| editor.value | ||
| .chain() | ||
| /** delete portal node */ | ||
| .deleteRange({ | ||
| from: portalNode.pos, | ||
| to: portalNode.pos + portalNode.node.nodeSize, | ||
| }) | ||
| .insertContentAt(portalNode.pos, paragraphizeToJSON(result)) | ||
| .run(); | ||
| } catch (error) { | ||
| /** tell agents component that work is done */ | ||
| delete agentsWorking.value[portalId]; | ||
|
|
||
| /** find node of portal created earlier */ | ||
| const portalNode = findPortal(portalId); | ||
| if (!portalNode) return; | ||
|
|
||
| // Handle ManugenError specially | ||
| if (error instanceof ManugenError) { | ||
| // Show a detailed error toast | ||
| toast.error( | ||
| `<strong>${error.message}</strong><br/>${error.suggestion ? `<em>Suggestion: ${error.suggestion}</em>` : ''}`, | ||
| { | ||
| position: "bottom-left", | ||
| autoClose: 10000, | ||
| hideProgressBar: false, | ||
| closeOnClick: true, | ||
| pauseOnHover: true, | ||
| dangerouslyHTMLString: true, | ||
| } | ||
| ); | ||
|
|
||
| // Insert an error message in the editor instead of empty content | ||
| const errorText = `⚠️ Error: ${error.message}${error.suggestion ? `\n\nSuggestion: ${error.suggestion}` : ''}`; | ||
|
|
||
| editor.value | ||
| .chain() | ||
| /** delete portal node */ | ||
| .deleteRange({ | ||
| from: portalNode.pos, | ||
| to: portalNode.pos + portalNode.node.nodeSize, | ||
| }) | ||
| .insertContentAt(portalNode.pos, paragraphizeToJSON(errorText)) | ||
| .run(); | ||
| } else { | ||
| // Handle generic errors | ||
| console.error('Unexpected error in action:', error); | ||
|
|
||
| toast.error( | ||
| 'An unexpected error occurred. Please try again.', | ||
| { | ||
| position: "bottom-left", | ||
| autoClose: 5000, | ||
| } | ||
| ); | ||
|
|
||
| // Insert a generic error message | ||
| const errorText = '⚠️ An unexpected error occurred. Please try again.'; | ||
|
|
||
| editor.value | ||
| .chain() | ||
| /** delete portal node */ | ||
| .deleteRange({ | ||
| from: portalNode.pos, | ||
| to: portalNode.pos + portalNode.node.nodeSize, | ||
| }) | ||
| .insertContentAt(portalNode.pos, paragraphizeToJSON(errorText)) | ||
| .run(); | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love that it essentially duplicated the same block of code for handling a specific and nonspecific error. I also don't love that it's emitting the error text into the document rather than doing something more useful, like reinserting the original text. I'm not going to fix this since it'll likely get replaced in the upcoming UI refactor, but I thought I'd note it.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the code duplication by creating a reusable |
||
| } | ||
| }; | ||
|
|
||
| const aiWriterSelectAction = (label: string, icon: any, prefix: string = "", agent: AgentId = "aiWriter") => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,17 @@ | ||
| import json | ||
| from abc import ABCMeta | ||
|
|
||
| from google.adk.agents import BaseAgent | ||
| from google.adk.agents.invocation_context import InvocationContext | ||
| from google.adk.events import Event, EventActions | ||
| from google.genai import types | ||
|
|
||
| from .schema import ErrorResponse | ||
|
|
||
|
|
||
| class ManugenAIBaseAgent(BaseAgent, metaclass=ABCMeta): | ||
| """ | ||
| TODO: add docs | ||
| Base agent class for Manugen AI with enhanced error handling. | ||
| """ | ||
|
|
||
| def error_message(self, ctx: InvocationContext, error_msg: str) -> Event: | ||
|
|
@@ -26,6 +29,45 @@ def error_message(self, ctx: InvocationContext, error_msg: str) -> Event: | |
| ), | ||
| ) | ||
|
|
||
| def structured_error_message( | ||
| self, | ||
| ctx: InvocationContext, | ||
| error_type: str, | ||
| message: str, | ||
| details: str = "", | ||
| suggestion: str = "" | ||
| ) -> Event: | ||
| """ | ||
| Create a structured error response that the UI can parse and display properly. | ||
|
|
||
| Args: | ||
| ctx: The invocation context | ||
| error_type: Type of error (e.g., 'model_error', 'agent_error', 'validation_error') | ||
| message: Human-readable error message | ||
| details: Additional error details for debugging | ||
| suggestion: Suggested action for the user | ||
| """ | ||
| error_response = ErrorResponse( | ||
| error_type=error_type, | ||
| message=message, | ||
| details=details, | ||
| suggestion=suggestion | ||
| ) | ||
|
|
||
| # Create a structured JSON response that the frontend can parse | ||
| error_json = error_response.model_dump_json() | ||
|
|
||
| return Event( | ||
| author=self.name, | ||
| invocation_id=ctx.invocation_id, | ||
| content=types.Content( | ||
| role="model", | ||
| parts=[ | ||
| types.Part(text=f"MANUGEN_ERROR: {error_json}"), | ||
| ], | ||
| ), | ||
| ) | ||
|
Comment on lines
+60
to
+69
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get why it's sending the error back as part of the event stream; I suppose it's the least invasive way to add reporting from the agents through to the frontend. While you can use ADK events to represent errors, the way it's done here only flags it as an error by the presence of a string, "MANUGEN_ERROR: ", preceding the error text. The ADK has a way to flag events as errors, documented here: https://google.github.io/adk-docs/events/#additional-context-and-event-details.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to use proper ADK error fields! Now using |
||
|
|
||
| def get_transfer_to_agent_event( | ||
| self, ctx: InvocationContext, agent: BaseAgent | ||
| ) -> Event: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, I dislike it looking for a sentinel string to detect an error. This code will also erroneously report an error if "MANUGEN_ERROR:" occurs anywhere in the text. IMO if you were going to use sentinel strings for errors, you should perform this check over each
textPartand see if it begins withMANUGEN_ERROR:.Even better, in the server-sent events case, this error-detecting code should be moved to the server-side events handler, so that errors are displayed immediately rather than once the entire response has been received.
Regarding how errors are flagged, you could add the
error_codeanderror_messagefields to the event to make it a proper ADK error; this code would then look for those fields and report them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed all the issues you mentioned:
error_codeanderror_messagefieldscheckForADKError()function that checks each response part individuallyextractADKText()function now checks for errors early in the response processing rather than after concatenating all textThe error detection now happens immediately when processing each
ADKResponsePart, eliminating the risk of false positives and providing much faster feedback to users. (2b78cd8)