Skip to content

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.

Sign up for GitHub

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

Draft
Copilot wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Implement proper ADK error handling using native error fields instead of sentinel strings #36

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3b778ba
Initial plan
Copilot Jul 15, 2025
c6f9c1c
Implement structured error handling system
Copilot Jul 15, 2025
ea5b51e
Complete error handling implementation with tests and fixes
Copilot Jul 15, 2025
2b78cd8
Implement proper ADK error handling and fix code duplication
Copilot Jul 28, 2025
608f3a4
Fix original content restoration to preserve paragraph structure
Copilot Jul 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 39 additions & 36 deletions frontend/src/api/adk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ export type ADKResponsePart = {
"requestedAuthConfigs": object,
"transferToAgent": string|undefined
},
"errorCode"?: string,
"errorMessage"?: string,
"id": string,
"timestamp": number
}
Expand Down Expand Up @@ -209,6 +211,35 @@ export const sseRequest = async (
}


/**
* Checks if an ADK response part contains an error using proper ADK error fields
* @param responsePart - A single ADK response part to check for errors
* @returns ManugenError if error is found, null otherwise
*/
export const checkForADKError = (responsePart: ADKResponsePart): ManugenError | null => {
if (responsePart.errorCode && responsePart.errorMessage) {
try {
// Try to parse the error message as JSON to get structured error data
const errorData = JSON.parse(responsePart.errorMessage);
return new ManugenError(
errorData.error_type || responsePart.errorCode,
errorData.message || 'An error occurred',
errorData.details || '',
errorData.suggestion || ''
);
} catch {
// If JSON parsing fails, create error from the raw error fields
return new ManugenError(
responsePart.errorCode,
responsePart.errorMessage,
'',
''
);
}
}
return null;
};

/**
* Utility method to extract 'text' sections from an ADK API response
*
Expand All @@ -223,6 +254,14 @@ export const extractADKText = (response: ADKResponse|undefined, onlyLast: boolea
return "";
}

// Check each response part for errors using proper ADK error fields
for (const responsePart of response) {
const error = checkForADKError(responsePart);
if (error) {
throw error;
}
}

const textSections = response
.filter(item => item.content && item.content.parts)
.map(item => item.content.parts)
Expand All @@ -235,42 +274,6 @@ export const extractADKText = (response: ADKResponse|undefined, onlyLast: boolea
return textParts.map(x => x.text).join("\n");
})

// Check for structured error responses
const allText = textSections.join("\n");
if (allText.includes("MANUGEN_ERROR:")) {
const errorMatch = allText.match(/MANUGEN_ERROR:\s*(\{.*?\})/s);
if (errorMatch) {
try {
const errorData = JSON.parse(errorMatch[1]);
throw new ManugenError(
errorData.error_type || 'unknown_error',
errorData.message || 'An error occurred',
errorData.details || '',
errorData.suggestion || ''
);
} catch (e) {
if (e instanceof ManugenError) {
throw e;
}
// If JSON parsing fails, throw a generic error
throw new ManugenError(
'parse_error',
'Failed to parse error response',
'The server returned an error but it could not be parsed properly.',
'Please try again or contact support if the problem persists.'
);
}
} else {
// MANUGEN_ERROR found but no valid JSON match
throw new ManugenError(
'parse_error',
'Malformed error response detected',
'The server returned an error but it could not be parsed properly.',
'Please try again or contact support if the problem persists.'
);
}
}

if (onlyLast) {
// if onlyLast is true, return the last text section
return textSections.length > 0 ? textSections[textSections.length - 1] : "";
Expand Down
20 changes: 12 additions & 8 deletions frontend/src/api/endpoints.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { toast, type ToastOptions } from 'vue3-toastify';
import { api, request } from "./";

import {
ensureSessionExists, extractADKText, sseRequest,
ensureSessionExists, extractADKText, sseRequest, checkForADKError,
type ADKResponse, type ADKResponsePart, type ADKSessionResponse
} from "./adk";

Expand Down Expand Up @@ -115,13 +115,17 @@ export const aiWriterAsync = async (input: string, session: ADKSessionResponse|n

console.log("Final event log received:", eventLog);

toast("Done!", {
position: "bottom-left",
autoClose: 6000,
hideProgressBar: true,
type: "success",
transition: "bounce",
} as ToastOptions);
// Only show success toast if no errors occurred
const hasErrors = eventLog.some(event => event.errorCode && event.errorMessage);
if (!hasErrors) {
toast("Done!", {
position: "bottom-left",
autoClose: 6000,
hideProgressBar: true,
type: "success",
transition: "bounce",
} as ToastOptions);
}

return eventLog;
}
115 changes: 59 additions & 56 deletions frontend/src/pages/PageEditor.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,60 @@ const findPortal = (id: string) => {
return findChildren(doc, (node) => node.attrs.id === id)?.[0];
};

/** helper function to handle error display and editor content replacement */
const handleActionError = (error: any, portalId: string, originalText?: string) => {
/** find node of portal created earlier */
const portalNode = findPortal(portalId);
if (!portalNode) return;

let errorText: string;
let toastMessage: string;
let toastDuration: number;

// Handle ManugenError specially
if (error instanceof ManugenError) {
// Show a detailed error toast
toastMessage = `<strong>${error.message}</strong><br/>${error.suggestion ? `<em>Suggestion: ${error.suggestion}</em>` : ''}`;
toastDuration = 10000;

// Restore original text if available, otherwise show error message
if (originalText) {
errorText = originalText;
} else {
errorText = `⚠️ Error: ${error.message}${error.suggestion ? `\n\nSuggestion: ${error.suggestion}` : ''}`;
}
} else {
// Handle generic errors
console.error('Unexpected error in action:', error);
toastMessage = 'An unexpected error occurred. Please try again.';
toastDuration = 5000;

// Restore original text if available, otherwise show generic error message
errorText = originalText || '⚠️ An unexpected error occurred. Please try again.';
}

// Show error toast
toast.error(toastMessage, {
position: "bottom-left",
autoClose: toastDuration,
hideProgressBar: false,
closeOnClick: true,
pauseOnHover: true,
dangerouslyHTMLString: true,
});

// Update editor content
editor.value
.chain()
/** delete portal node */
.deleteRange({
from: portalNode.pos,
to: portalNode.pos + portalNode.node.nodeSize,
})
.insertContentAt(portalNode.pos, paragraphizeToJSON(errorText))
.run();
};

/** create func that runs an action and handles placeholder in the editor while its working */
const action =
(
Expand All @@ -327,6 +381,9 @@ const action =
const context = getContext();
if (!context) return;

// Store original text for potential restoration on error
const originalText = context.sel;

Copy link
Copy Markdown
Collaborator

@falquaddoomi falquaddoomi Jul 30, 2025

Choose a reason for hiding this comment

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

This doesn't quite work: the original content includes paragraph tags, while this only retains the text itself. When the text is replaced on a failure, it ends being one long block of text without breaks that were introduced by those tags.

Copy link
Copy Markdown
Author

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

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

Fixed! Updated getContext() to capture the selected content as structured JSON (selJSON) instead of plain text. This preserves the original paragraph structure and formatting when restoring content on error, ensuring paragraph breaks are maintained. (608f3a4)

/** create portal */
const portalId = addPortal();
if (!portalId) return;
Expand Down Expand Up @@ -358,62 +415,8 @@ const action =
/** 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();
}
// Use helper function to handle error display and editor update
handleActionError(error, portalId, originalText);
}
};

Expand Down
8 changes: 4 additions & 4 deletions packages/manugen-ai/src/manugen_ai/adk.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,16 @@ def structured_error_message(
suggestion=suggestion
)

# Create a structured JSON response that the frontend can parse
error_json = error_response.model_dump_json()

# Use proper ADK error fields instead of sentinel strings
return Event(
author=self.name,
invocation_id=ctx.invocation_id,
error_code=error_type,
error_message=error_response.model_dump_json(),
content=types.Content(
role="model",
parts=[
types.Part(text=f"MANUGEN_ERROR: {error_json}"),
types.Part(text=f"Error: {message}"),
],
),
)
Comment on lines +60 to +69
Copy link
Copy Markdown
Collaborator

@falquaddoomi falquaddoomi Jul 28, 2025

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Author

Copilot AI Jul 28, 2025

Choose a reason for hiding this comment

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

Updated to use proper ADK error fields! Now using error_code and error_message fields on the Event object instead of the sentinel string approach. This eliminates the risk of false positive errors from regular text and follows the ADK standard. (2b78cd8)

Expand Down