Skip to content

Add error handling for default get-messages request and message parsing #180

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add error handling for default get-messages request and message parsing #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
472a5b4
'add error handling for default get-messages request and other messag…
nickfujita Apr 16, 2025
80eee3e
'changeset'
nickfujita Apr 16, 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
5 changes: 5 additions & 0 deletions .changeset/large-plums-smile.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"agents": patch
---

Add error handling for default /get-messages request & hook up option.onError and error return value.
45 changes: 29 additions & 16 deletions packages/agents/src/ai-react.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useChat } from "@ai-sdk/react";
import type { Message } from "ai";
import type { useAgent } from "./react";
import { useEffect, use } from "react";
import { useEffect, use, useState } from "react";
import type { OutgoingMessage } from "./ai-types";

type GetInitialMessagesOptions = {
Expand Down Expand Up @@ -37,7 +37,8 @@ const requestCache = new Map<string, Promise<Message[]>>();
export function useAgentChat<State = unknown>(
options: UseAgentChatOptions<State>
) {
const { agent, getInitialMessages, ...rest } = options;
const { agent, getInitialMessages, onError, ...rest } = options;
const [error, setError] = useState<Error|undefined>()

const agentUrl = new URL(
`${// @ts-expect-error we're using a protected _url property that includes query params
Expand All @@ -55,11 +56,18 @@ export function useAgentChat<State = unknown>(
}: GetInitialMessagesOptions) {
const getMessagesUrl = new URL(url);
getMessagesUrl.pathname += "/get-messages";
const response = await fetch(getMessagesUrl.toString(), {
headers: options.headers,
credentials: options.credentials,
});
return response.json<Message[]>();
try {
const response = await fetch(getMessagesUrl.toString(), {
headers: options.headers,
credentials: options.credentials,
});
return response.json<Message[]>();
} catch (e) {
Copy link
Collaborator

@threepointone threepointone Apr 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think it's better to actually throw the error here, and expect the user to add an ErrorBoundary

Copy link
Contributor Author

@nickfujita nickfujita Apr 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think that might throw off many users, as it did me, because it doesn't seem to be the expected behavior for a hook to throw an error upon receiving a 40x response. For example, when using tanstack-query the standard behavior isn't to throw an error if the request fails, but rather to communicate the issue in the form of an error property on the return object, and leaving data as undefined (which maybe we should do here instead of returning an empty array 🤔).
I don't think the underlying userPartySocket requires an ErrorBoundary either, but rather communitates errors via the onerror property in it's return object.

If there was a way to provide a typescript error when using this hook without a ErrorBoundary, that would probably make it more feasible, but I'm not currently aware of a way to add that.

Maybe adding a very clear note in the documentation that there needs to be an ErrorBoundary and updating the agent template repo might prevent users from running into this, but I just can't help but feel that there would be more people filing issues for this same thing in the future 😅

const errorInstance = new Error(`Error getting messages: ${e}`);
onError?.(errorInstance);
setError(errorInstance);
return [];
}
}

const getInitialMessagesFetch =
Expand Down Expand Up @@ -129,9 +137,10 @@ export function useAgentChat<State = unknown>(
let data: OutgoingMessage;
try {
data = JSON.parse(event.data) as OutgoingMessage;
} catch (error) {
// silently ignore invalid messages for now
// TODO: log errors with log levels
} catch (e) {
const errorInstance = new Error(`Error parsing onClearHistory messages: ${e}`);
onError?.(errorInstance)
setError(errorInstance);
return;
}
if (data.type === "cf_agent_use_chat_response") {
Expand Down Expand Up @@ -195,9 +204,10 @@ export function useAgentChat<State = unknown>(
let data: OutgoingMessage;
try {
data = JSON.parse(event.data) as OutgoingMessage;
} catch (error) {
// silently ignore invalid messages for now
// TODO: log errors with log levels
} catch (e) {
const errorInstance = new Error(`Error parsing onClearHistory messages: ${e}`);
onError?.(errorInstance)
setError(errorInstance);
return;
}
if (data.type === "cf_agent_chat_clear") {
Expand All @@ -212,9 +222,10 @@ export function useAgentChat<State = unknown>(
let data: OutgoingMessage;
try {
data = JSON.parse(event.data) as OutgoingMessage;
} catch (error) {
// silently ignore invalid messages for now
// TODO: log errors with log levels
} catch (e) {
const errorInstance = new Error(`Error parsing onMessages messages: ${e}`);
onError?.(errorInstance)
setError(errorInstance);
return;
}
if (data.type === "cf_agent_chat_messages") {
Expand All @@ -233,10 +244,12 @@ export function useAgentChat<State = unknown>(
agent.addEventListener,
agent.removeEventListener,
useChatHelpers.setMessages,
onError,
]);

return {
...useChatHelpers,
error: useChatHelpers.error ?? error,
/**
* Set the chat messages and synchronize with the Agent
* @param messages New messages to set
Expand Down