fix: deliver the actual error to onError(connection, error) when a handler throws

.changeset/heavy-pugs-repeat.md

@@ -0,0 +1,10 @@
+---
+"agents": patch
+"@cloudflare/ai-chat": patch
+---
+
+fix: deliver the actual error to `onError(connection, error)` when a message handler throws
+
+When `.sql` (or anything else) threw while an agent was handling a websocket message, the framework reported it with a single-argument `this.onError(error)` call. A user override written with the documented two-parameter signature — `onError(connection, error)` — therefore received the error as the _connection_ and `undefined` as the _error_, and the original failure was replaced by `throw undefined` upstream (#388).
+
+`_tryCatch` (and ai-chat's `_tryCatchChat`) now read the connection from the agent context and deliver the actual caught error through the `onError(connection, error)` overload, then rethrow the original error rather than `onError`'s return value. The base `onError` discriminates its overloads on call arity instead of error truthiness, so a connection error with no detail is no longer misrouted into the server branch (which threw the Connection object itself); errors are always passed through exactly as received, never synthesized.

docs/agent-class.md

@@ -411,21 +411,24 @@ function someUtilityFunction() {
 
 ### `this.onError`
 
-`Agent` extends `Server`'s `onError` so it can be used to handle errors that are not necessarily WebSocket errors. It is called with a `Connection` or `unknown` error.
+`Agent` extends `Server`'s `onError` so it can be used to handle errors that are not necessarily WebSocket errors. It is called in one of two shapes: `onError(connection, error)` when the error is associated with a connection (for example, an error thrown while handling a WebSocket message), or `onError(error)` for server errors (alarms, schedules, and other background work). The overloads exist only at the type level — at runtime there is a single method, so an override must handle both shapes:
 
 ```ts
 class MyAgent extends Agent {
   onError(connectionOrError: Connection | unknown, error?: unknown) {
-    if (error) {
-      // WebSocket connection error
-      console.error("Connection error:", error);
+    if (error !== undefined) {
+      // Error associated with a websocket connection.
+      // `error` is the actual thrown error (e.g. a SqlError).
+      const connection = connectionOrError as Connection;
+      console.error(`Error on connection ${connection.id}:`, error);
+
+      // Optionally rethrow to propagate the error
+      throw error;
     } else {
       // Server error
       console.error("Server error:", connectionOrError);
+      throw connectionOrError;
     }
-
-    // Optionally throw to propagate the error
-    throw connectionOrError;
   }
 }
 ```

docs/http-websockets.md

@@ -386,22 +386,23 @@ This status persists across hibernation — a connection that was marked as no-p
 
 ## Error Handling
 
-Handle errors gracefully with `onError`:
+Handle errors gracefully with `onError`. The two signatures are TypeScript overloads — at runtime there is a single method, so one implementation must handle both call shapes. When the error is associated with a connection (e.g. your `onMessage` handler threw), it arrives as `onError(connection, error)` with the actual thrown error in the second parameter; server errors (alarms, schedules, background work) arrive as `onError(error)`:
 
 ```typescript
 export class MyAgent extends Agent {
-  // WebSocket connection error
-  onError(connection: Connection, error: unknown): void {
-    console.error(`WebSocket error on ${connection.id}:`, error);
-    connection.send(
-      JSON.stringify({ type: "error", message: "Connection error" })
-    );
-  }
-
-  // Server error (overloaded signature - no connection parameter)
-  onError(error: unknown): void {
-    console.error("Server error:", error);
-    // Log to external service, etc.
+  onError(connectionOrError: Connection | unknown, error?: unknown): void {
+    if (error !== undefined) {
+      // WebSocket connection error — `error` is the actual thrown error
+      const connection = connectionOrError as Connection;
+      console.error(`WebSocket error on ${connection.id}:`, error);
+      connection.send(
+        JSON.stringify({ type: "error", message: "Connection error" })
+      );
+    } else {
+      // Server error
+      console.error("Server error:", connectionOrError);
+      // Log to external service, etc.
+    }
   }
 }
 ```

packages/agents/src/index.ts

@@ -2565,9 +2565,15 @@ export class Agent<
             }
           );
         } catch (e) {
-          // onStateChanged/onStateUpdate errors should not affect state or broadcasts
+          // onStateChanged/onStateUpdate errors should not affect state or broadcasts.
+          // When the update came from a connection, deliver the error through
+          // the onError(connection, error) overload (#388).
           try {
-            await this.onError(e);
+            if (connection) {
+              await this.onError(connection, e);
+            } else {
+              await this.onError(e);
+            }
           } catch {
             // swallow
           }
@@ -3122,7 +3128,20 @@ export class Agent<
     try {
       return await fn();
     } catch (e) {
-      throw this.onError(e);
+      // When the failure happened while handling a websocket message, the
+      // connection is in the agent context — deliver the actual error through
+      // the onError(connection, error) overload so user overrides written
+      // with the two-parameter signature receive it in the error slot (#388).
+      const { connection } = getCurrentAgent();
+      if (connection) {
+        await this.onError(connection, e);
+      } else {
+        await this.onError(e);
+      }
+      // Rethrow the original error, not onError's return value — a user
+      // override that returns normally must not turn the failure into
+      // `throw undefined`.
+      throw e;
     }
   }
 
@@ -3193,11 +3212,17 @@ export class Agent<
     error: unknown
   ): void | Promise<void>;
   override onError(error: unknown): void | Promise<void>;
-  override onError(connectionOrError: Connection | unknown, error?: unknown) {
+  override onError(
+    connectionOrError: Connection | unknown,
+    ...rest: [unknown?]
+  ) {
     let theError: unknown;
-    if (connectionOrError && error) {
-      theError = error;
-      // this is a websocket connection error
+    if (rest.length > 0) {
+      // Two-argument call: a websocket connection error. Discriminated on
+      // arity, not truthiness — an undefined error must not be misrouted
+      // into the server branch (which would throw the Connection object
+      // itself). The error is rethrown exactly as received, never replaced.
+      theError = rest[0];
       console.error(
         "Error on websocket connection:",
         (connectionOrError as Connection).id,

packages/agents/src/tests/agents/on-error.ts

@@ -0,0 +1,53 @@
+import { Agent, type Connection, type WSMessage } from "../../index.ts";
+
+export type OnErrorCapture = {
+  firstArgIsConnection: boolean;
+  firstArgErrorMessage: string | null;
+  connectionId: string | null;
+  errorDefined: boolean;
+  errorName: string | null;
+  errorMessage: string | null;
+};
+
+export type OnErrorTestState = { count: number };
+
+// Overrides onError exactly the way users do per the documented websocket
+// signature (#388): two parameters, reading the error from the second one.
+// The captures record what actually arrived in each slot.
+export class TestOnErrorAgent extends Agent<Cloudflare.Env, OnErrorTestState> {
+  initialState: OnErrorTestState = { count: 0 };
+  captures: OnErrorCapture[] = [];
+
+  async onMessage(_connection: Connection, message: WSMessage) {
+    if (message === "throw-sql") {
+      this.sql`SELECT * FROM table_that_does_not_exist`;
+    }
+  }
+
+  onStateChanged(state: OnErrorTestState, _source: Connection | "server") {
+    if (state.count === 999) {
+      throw new Error("state hook boom");
+    }
+  }
+
+  override onError(connection: Connection, error?: unknown): void {
+    const isConnection =
+      typeof connection === "object" &&
+      connection !== null &&
+      typeof (connection as Connection).send === "function";
+    this.captures.push({
+      firstArgIsConnection: isConnection,
+      firstArgErrorMessage:
+        connection instanceof Error ? connection.message : null,
+      connectionId: isConnection ? (connection as Connection).id : null,
+      errorDefined: error !== undefined,
+      errorName: error instanceof Error ? error.name : null,
+      errorMessage: error instanceof Error ? error.message : null
+    });
+    // Like the issue reporter's handler: observe the error, don't rethrow.
+  }
+
+  async getCaptures(): Promise<OnErrorCapture[]> {
+    return this.captures;
+  }
+}

packages/agents/src/tests/agents/state.ts

@@ -30,6 +30,31 @@ export class TestStateAgent extends Agent<Cloudflare.Env, TestState> {
     });
   }
 
+  // Probe for base-onError behavior tests (#388): calls the base onError the
+  // way partyserver does and reports what it threw.
+  async probeOnError(
+    kind: "ws-undefined-error" | "ws-error" | "server-error"
+  ): Promise<{ thrown: string; message: string }> {
+    const fakeConnection = { id: "probe-connection" } as Connection;
+    try {
+      if (kind === "ws-undefined-error") {
+        // partyserver forwards ErrorEvent.error, which the runtime can leave
+        // undefined: this.onError(connection, e.error)
+        await this.onError(fakeConnection, undefined);
+      } else if (kind === "ws-error") {
+        await this.onError(fakeConnection, new Error("ws boom"));
+      } else {
+        await this.onError(new Error("server boom"));
+      }
+    } catch (e) {
+      return {
+        thrown: e instanceof Error ? "Error" : typeof e,
+        message: e instanceof Error ? e.message : String(e)
+      };
+    }
+    return { thrown: "nothing", message: "" };
+  }
+
   // HTTP handler for testing agentFetch and path routing
   // Only handles specific test paths - returns 404 for others to preserve routing test behavior
   async onRequest(request: Request): Promise<Response> {

packages/agents/src/tests/on-error.test.ts

@@ -0,0 +1,107 @@
+import { env, exports } from "cloudflare:workers";
+import { describe, expect, it } from "vitest";
+import { getAgentByName } from "..";
+import { MessageType } from "../types";
+import type { OnErrorCapture } from "./agents/on-error.ts";
+
+// Regression tests for #388: when .sql (or anything else) throws while the
+// agent is handling a websocket message, the actual error must be delivered
+// to onError(connection, error) — the error in the error slot, the connection
+// in the connection slot. Before the fix, _tryCatch called this.onError(e)
+// with a single argument, so a user override written with the documented
+// two-parameter signature received the error AS the connection and an
+// undefined error — "my agent is silently failing and i have no clue why".
+
+async function connectWS(path: string) {
+  const res = await exports.default.fetch(`http://example.com${path}`, {
+    headers: { Upgrade: "websocket" }
+  });
+  expect(res.status).toBe(101);
+  const ws = res.webSocket as WebSocket;
+  expect(ws).toBeDefined();
+  ws.accept();
+  return { ws };
+}
+
+describe("onError receives the actual error thrown by .sql (#388)", () => {
+  it("delivers (connection, error) to a two-parameter onError override", async () => {
+    const room = crypto.randomUUID();
+    const { ws } = await connectWS(`/agents/test-on-error-agent/${room}`);
+    ws.send("throw-sql");
+
+    const agent = await getAgentByName(env.TestOnErrorAgent, room);
+    let captures: OnErrorCapture[] = [];
+    const start = Date.now();
+    while (captures.length === 0 && Date.now() - start < 2000) {
+      captures = await agent.getCaptures();
+      if (captures.length === 0) {
+        await new Promise((r) => setTimeout(r, 10));
+      }
+    }
+    ws.close();
+
+    expect(captures.length).toBe(1);
+    const capture = captures[0];
+    // The error slot must carry the actual SqlError…
+    expect(capture.errorDefined).toBe(true);
+    expect(capture.errorName).toBe("SqlError");
+    expect(capture.errorMessage).toContain("no such table");
+    // …and the connection slot must carry the Connection, not the error.
+    expect(capture.firstArgIsConnection).toBe(true);
+    expect(capture.firstArgErrorMessage).toBeNull();
+  });
+
+  it("delivers (connection, error) when a state persistence hook throws on a connection-driven update", async () => {
+    const room = crypto.randomUUID();
+    const { ws } = await connectWS(`/agents/test-on-error-agent/${room}`);
+    ws.send(
+      JSON.stringify({
+        type: MessageType.CF_AGENT_STATE,
+        state: { count: 999 }
+      })
+    );
+
+    const agent = await getAgentByName(env.TestOnErrorAgent, room);
+    let captures: OnErrorCapture[] = [];
+    const start = Date.now();
+    while (captures.length === 0 && Date.now() - start < 2000) {
+      captures = await agent.getCaptures();
+      if (captures.length === 0) {
+        await new Promise((r) => setTimeout(r, 10));
+      }
+    }
+    ws.close();
+
+    expect(captures.length).toBe(1);
+    const capture = captures[0];
+    expect(capture.errorDefined).toBe(true);
+    expect(capture.errorMessage).toBe("state hook boom");
+    expect(capture.firstArgIsConnection).toBe(true);
+    expect(capture.firstArgErrorMessage).toBeNull();
+  });
+});
+
+describe("base onError overload discrimination", () => {
+  it("rethrows the original error for a connection error", async () => {
+    const agent = await getAgentByName(env.TestStateAgent, "on-error-ws");
+    const result = await agent.probeOnError("ws-error");
+    expect(result.thrown).toBe("Error");
+    expect(result.message).toBe("ws boom");
+  });
+
+  it("rethrows the original error for a server error", async () => {
+    const agent = await getAgentByName(env.TestStateAgent, "on-error-server");
+    const result = await agent.probeOnError("server-error");
+    expect(result.thrown).toBe("Error");
+    expect(result.message).toBe("server boom");
+  });
+
+  it("passes an undefined connection error through untouched", async () => {
+    // No injection: a two-argument call with no error detail must not throw
+    // the Connection object (the old truthiness misroute) and must not
+    // synthesize a stand-in Error — it rethrows exactly what was passed.
+    const agent = await getAgentByName(env.TestStateAgent, "on-error-ws-undef");
+    const result = await agent.probeOnError("ws-undefined-error");
+    expect(result.thrown).toBe("undefined");
+  });
+});

packages/agents/src/tests/worker.ts

@@ -71,6 +71,8 @@ export {
 export { ChatSdkStateAgent } from "./agents";
 export { TestRunFiberAgent } from "./agents/run-fiber";
 import type { TestRunFiberAgent } from "./agents/run-fiber";
+export { TestOnErrorAgent } from "./agents/on-error.ts";
+import type { TestOnErrorAgent } from "./agents/on-error.ts";
 
 export type { TestState } from "./agents";
 
@@ -160,6 +162,7 @@ export type Env = {
   TestRpcMcpClientAgent: DurableObjectNamespace<TestRpcMcpClientAgent>;
   TestHttpMcpDedupAgent: DurableObjectNamespace<TestHttpMcpDedupAgent>;
   TestStateAgent: DurableObjectNamespace<TestStateAgent>;
+  TestOnErrorAgent: DurableObjectNamespace<TestOnErrorAgent>;
   TestStateAgentNoInitial: DurableObjectNamespace<TestStateAgentNoInitial>;
   TestThrowingStateAgent: DurableObjectNamespace<TestThrowingStateAgent>;
   TestPersistedStateAgent: DurableObjectNamespace<TestPersistedStateAgent>;

packages/agents/src/tests/wrangler.jsonc

@@ -97,6 +97,10 @@
         "class_name": "TestStateAgent",
         "name": "TestStateAgent"
       },
+      {
+        "class_name": "TestOnErrorAgent",
+        "name": "TestOnErrorAgent"
+      },
       {
         "class_name": "TestStateAgentNoInitial",
         "name": "TestStateAgentNoInitial"
@@ -317,6 +321,7 @@
         "TestRpcMcpClientAgent",
         "TestHttpMcpDedupAgent",
         "TestStateAgent",
+        "TestOnErrorAgent",
         "TestStateAgentNoInitial",
         "TestThrowingStateAgent",
         "TestPersistedStateAgent",

packages/ai-chat/src/index.ts

@@ -14,6 +14,7 @@ import {
   type Connection,
   type ConnectionContext,
   type FiberRecoveryContext,
+  getCurrentAgent,
   type WSMessage
 } from "agents";
 
@@ -1749,7 +1750,17 @@ export class AIChatAgent<
     try {
       return await fn();
     } catch (e) {
-      throw this.onError(e);
+      // Mirror Agent._tryCatch: when a websocket connection is in the agent
+      // context, deliver the actual error through onError(connection, error)
+      // so two-parameter user overrides receive it in the error slot (#388),
+      // then rethrow the original error rather than onError's return value.
+      const { connection } = getCurrentAgent();
+      if (connection) {
+        await this.onError(connection, e);
+      } else {
+        await this.onError(e);
+      }
+      throw e;
     }
   }