fix(appkit): forward async handler rejections to a terminal error middleware instead of crashing

packages/appkit/src/core/plugin-context.ts

@@ -2,6 +2,7 @@ import type express from "express";
 import type { BasePlugin, IAppRequest, ToolProvider } from "shared";
 import { createLogger } from "../logging/logger";
 import { SpanStatusCode, TelemetryManager } from "../telemetry";
+import { forwardAsyncErrors } from "../utils/safe-handler";
 
 const logger = createLogger("plugin-context");
 
@@ -287,7 +288,9 @@ export class PluginContext {
       if (typeof app[method] === "function") {
         (app[method] as (...a: unknown[]) => void)(
           route.path,
-          ...route.handlers,
+          // Forward sync throws and async rejections to the Express error
+          // middleware — Express 4 does not do this on its own.
+          ...route.handlers.map(forwardAsyncErrors),
         );
       }
     });
@@ -299,7 +302,9 @@ export class PluginContext {
   ): void {
     if (!this.routeTarget) return;
     this.routeTarget.addExtension((app) => {
-      app.use(path, ...handlers);
+      // forwardAsyncErrors leaves 4-arg error middleware untouched, so error
+      // handlers passed through addMiddleware keep their Express semantics.
+      app.use(path, ...handlers.map(forwardAsyncErrors));
     });
   }
 }

packages/appkit/src/core/tests/plugin-context.test.ts

@@ -75,7 +75,7 @@ describe("PluginContext", () => {
       expect(ctx.getPluginNames()).toEqual([]);
     });
 
-    test("flushRoutes applies buffered routes via addExtension", () => {
+    test("flushRoutes applies buffered routes via addExtension", async () => {
       const handler = vi.fn();
       ctx.addRoute("post", "/invocations", handler);
 
@@ -87,10 +87,19 @@ describe("PluginContext", () => {
 
       const mockApp = { post: vi.fn() };
       extensionFn(mockApp);
-      expect(mockApp.post).toHaveBeenCalledWith("/invocations", handler);
+      // Handlers are wrapped (forwardAsyncErrors) so async rejections reach
+      // the Express error middleware — assert the wrapper delegates.
+      expect(mockApp.post).toHaveBeenCalledWith(
+        "/invocations",
+        expect.any(Function),
+      );
+      const wrapped = mockApp.post.mock.calls[0][1];
+      const next = vi.fn();
+      await wrapped("req", "res", next);
+      expect(handler).toHaveBeenCalledWith("req", "res", next);
     });
 
-    test("addRoute called after registerAsRouteTarget applies immediately", () => {
+    test("addRoute called after registerAsRouteTarget applies immediately", async () => {
       const addExtension = vi.fn();
       ctx.registerAsRouteTarget({ addExtension });
 
@@ -102,10 +111,13 @@ describe("PluginContext", () => {
 
       const mockApp = { get: vi.fn() };
       extensionFn(mockApp);
-      expect(mockApp.get).toHaveBeenCalledWith("/health", handler);
+      expect(mockApp.get).toHaveBeenCalledWith("/health", expect.any(Function));
+      const wrapped = mockApp.get.mock.calls[0][1];
+      await wrapped("req", "res", vi.fn());
+      expect(handler).toHaveBeenCalled();
     });
 
-    test("addRoute supports middleware chains", () => {
+    test("addRoute supports middleware chains", async () => {
       const auth = vi.fn();
       const handler = vi.fn();
 
@@ -117,10 +129,19 @@ describe("PluginContext", () => {
       const extensionFn = addExtension.mock.calls[0][0];
       const mockApp = { post: vi.fn() };
       extensionFn(mockApp);
-      expect(mockApp.post).toHaveBeenCalledWith("/api", auth, handler);
+      expect(mockApp.post).toHaveBeenCalledWith(
+        "/api",
+        expect.any(Function),
+        expect.any(Function),
+      );
+      const [, wrappedAuth, wrappedHandler] = mockApp.post.mock.calls[0];
+      await wrappedAuth("req", "res", vi.fn());
+      await wrappedHandler("req", "res", vi.fn());
+      expect(auth).toHaveBeenCalled();
+      expect(handler).toHaveBeenCalled();
     });
 
-    test("addMiddleware buffers and applies via use()", () => {
+    test("addMiddleware buffers and applies via use()", async () => {
       const handler = vi.fn();
       ctx.addMiddleware("/api", handler);
 
@@ -132,7 +153,46 @@ describe("PluginContext", () => {
 
       const mockApp = { use: vi.fn() };
       extensionFn(mockApp);
-      expect(mockApp.use).toHaveBeenCalledWith("/api", handler);
+      expect(mockApp.use).toHaveBeenCalledWith("/api", expect.any(Function));
+      const wrapped = mockApp.use.mock.calls[0][1];
+      await wrapped("req", "res", vi.fn());
+      expect(handler).toHaveBeenCalled();
+    });
+
+    test("wrapped route handlers forward async rejections to next", async () => {
+      const failure = new Error("handler rejected");
+      const handler = vi.fn().mockRejectedValue(failure);
+      const addExtension = vi.fn();
+      ctx.registerAsRouteTarget({ addExtension });
+
+      ctx.addRoute("get", "/boom", handler);
+
+      const extensionFn = addExtension.mock.calls[0][0];
+      const mockApp = { get: vi.fn() };
+      extensionFn(mockApp);
+      const wrapped = mockApp.get.mock.calls[0][1];
+      const next = vi.fn();
+      await wrapped("req", "res", next);
+      expect(next).toHaveBeenCalledWith(failure);
+    });
+
+    test("addMiddleware leaves 4-arg error middleware unwrapped", () => {
+      const errorMiddleware = (
+        _err: unknown,
+        _req: unknown,
+        _res: unknown,
+        _next: unknown,
+      ) => {};
+      const addExtension = vi.fn();
+      ctx.registerAsRouteTarget({ addExtension });
+
+      ctx.addMiddleware("/api", errorMiddleware as never);
+
+      const extensionFn = addExtension.mock.calls[0][0];
+      const mockApp = { use: vi.fn() };
+      extensionFn(mockApp);
+      // Express detects error middleware by arity — wrapping would break it.
+      expect(mockApp.use).toHaveBeenCalledWith("/api", errorMiddleware);
     });
 
     test("multiple buffered routes are all applied on registration", () => {

packages/appkit/src/plugin/plugin.ts

@@ -25,6 +25,7 @@ import {
   TelemetryManager,
 } from "../telemetry";
 import { deepMerge } from "../utils";
+import { forwardAsyncErrors } from "../utils/safe-handler";
 import { DevFileReader } from "./dev-reader";
 import type { ExecutionResult } from "./execution-result";
 import { CacheInterceptor } from "./interceptors/cache";
@@ -666,7 +667,9 @@ export abstract class Plugin<
   ): void {
     const { name, method, path, handler } = config;
 
-    router[method](path, handler);
+    // Sync throws and async rejections are forwarded to the Express error
+    // middleware via next(err) — Express 4 does not do this on its own.
+    router[method](path, forwardAsyncErrors(handler));
 
     const fullPath = `/api/${this.name}${path}`;
     this.registerEndpoint(name, fullPath);

packages/appkit/src/plugins/agents/agents.ts

@@ -771,9 +771,10 @@ export class AgentsPlugin extends Plugin implements ToolProvider {
    */
   private mountInvokeRoutes() {
     if (!this.context) return;
-    const handler = (req: express.Request, res: express.Response) => {
+    // Return the promise so the forwardAsyncErrors wrapper applied by
+    // PluginContext.addRoute can forward rejections to the error middleware.
+    const handler = (req: express.Request, res: express.Response) =>
       this._handleInvoke(req, res);
-    };
     this.context.addRoute("post", "/invocations", handler);
     this.context.addRoute("post", "/responses", handler);
   }

packages/appkit/src/plugins/server/index.ts

@@ -5,7 +5,7 @@ import dotenv from "dotenv";
 import express from "express";
 import getPort, { portNumbers } from "get-port";
 import type { PluginClientConfigs, PluginPhase } from "shared";
-import { ServerError } from "../../errors";
+import { AppKitError, ServerError } from "../../errors";
 import { TelemetryReporter } from "../../internal-telemetry";
 import { createLogger } from "../../logging/logger";
 import { Plugin, toPlugin } from "../../plugin";
@@ -147,6 +147,12 @@ export class ServerPlugin extends Plugin {
 
     await this.setupFrontend(endpoints, pluginConfigs);
 
+    // Terminal error handler — registered after all routes, extensions, and
+    // frontend middleware so that errors forwarded via next(err) from any of
+    // them (e.g. async handler rejections forwarded by Plugin.route()) are
+    // turned into JSON error responses instead of hanging the request.
+    this.serverApplication.use(errorHandlerMiddleware);
+
     const listenPort = await this.resolveListenPort();
 
     const server = this.serverApplication.listen(
@@ -192,6 +198,11 @@ export class ServerPlugin extends Plugin {
    * Call this inside the `onPluginsReady` callback of `createApp` to register
    * custom Express routes or middleware before the server starts listening.
    *
+   * Note: async handlers registered directly on the app must handle their own
+   * rejections — Express 4 does not forward rejected promises to the error
+   * middleware. Errors passed explicitly to `next(err)` are formatted by the
+   * terminal error middleware.
+   *
    * @param fn - A function that receives the express application.
    * @returns The server plugin instance for chaining.
    */
@@ -505,6 +516,87 @@ export function requestMetricsMiddleware(
   next();
 }
 
+/**
+ * Narrow an unknown thrown value to an Error that carries a numeric
+ * `statusCode` property in the HTTP error range (e.g. `ApiError` from
+ * `@databricks/sdk-experimental`).
+ */
+function hasHttpStatusCode(
+  error: unknown,
+): error is Error & { statusCode: number } {
+  if (!(error instanceof Error) || !("statusCode" in error)) return false;
+  const statusCode = (error as Record<string, unknown>).statusCode;
+  return (
+    typeof statusCode === "number" &&
+    Number.isInteger(statusCode) &&
+    statusCode >= 400 &&
+    statusCode <= 599
+  );
+}
+
+/**
+ * Terminal Express error-handling middleware.
+ *
+ * Converts errors forwarded via `next(err)` (including async handler
+ * rejections forwarded by `Plugin.route()`) into JSON error responses,
+ * following the same status/message conventions as `Plugin.execute()`:
+ * - errors carrying an HTTP `statusCode` (including `AppKitError`) → that
+ *   status; 5xx messages are masked in production to avoid leaking internals
+ * - anything else → 500 with a generic message in production
+ *
+ * 4xx errors are expected client errors (e.g. missing auth headers) and are
+ * logged at `warn` with just the message; 5xx/unknown errors are logged at
+ * `error` with the full error.
+ *
+ * @internal Exported for unit tests.
+ */
+export function errorHandlerMiddleware(
+  err: unknown,
+  req: express.Request,
+  res: express.Response,
+  next: express.NextFunction,
+) {
+  const httpError =
+    err instanceof AppKitError || hasHttpStatusCode(err) ? err : null;
+  const statusCode = httpError ? httpError.statusCode : 500;
+  const isClientError = statusCode < 500;
+
+  if (isClientError) {
+    logger.warn(
+      "Request failed for %s %s with status %d: %s",
+      req.method,
+      req.originalUrl,
+      statusCode,
+      err instanceof Error ? err.message : String(err),
+    );
+  } else {
+    logger.error(
+      "Unhandled error for %s %s: %O",
+      req.method,
+      req.originalUrl,
+      err,
+    );
+  }
+
+  if (res.headersSent) {
+    next(err);
+    return;
+  }
+
+  const isDev = process.env.NODE_ENV !== "production";
+
+  if (httpError) {
+    res.status(statusCode).json({
+      error: isDev || isClientError ? httpError.message : "Server error",
+    });
+    return;
+  }
+
+  res.status(statusCode).json({
+    error: isDev && err instanceof Error ? err.message : "Server error",
+  });
+}
+
 /**
  * @internal
  */