feat: Improve error handling from 3P requests on interceptor (#238)

* refactor: replace undefined return with ExecutionResult in execute()

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>

* test: update execute() tests for ExecutionResult and add AppKitError subclass coverage

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>

* chore: docs update

* chore: preserve upstream HTTP status codes in execute() for non-AppKitError errors

* docs: regenerate ExecutionResult API docs with error handling details

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>

* refactor: use HTTP status code descriptions in files plugin (#258)

* refactor: use HTTP status code descriptions instead of hardcoded error strings

Replace hardcoded error messages like "List failed" with standard HTTP
status descriptions from node:http STATUS_CODES for consistent error responses.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>

* chore: dry status error responses in files plugin

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>

* fix: standardize stream error to use _sendStatusError helper

Addresses Copilot review comment about mixed error formats in the
read/download stream handler.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>

---------

Signed-off-by: Atila Fassina <atila@fassina.eu>

* fix: correct test assertions for 404/409 status text in files plugin

The _sendStatusError() helper returns STATUS_CODES[status] (e.g., "Not Found"
for 404, "Conflict" for 409), but tests incorrectly expected "Internal Server
Error" for all error status codes.

Co-authored-by: Isaac
Signed-off-by: Atila Fassina <atila@fassina.eu>

---------

Signed-off-by: Atila Fassina <atila@fassina.eu>

Author: atilafassina

docs/docs/api/appkit/Class.Plugin.md

@@ -313,14 +313,16 @@ BasePlugin.clientConfig
 protected execute<T>(
    fn: (signal?: AbortSignal) => Promise<T>, 
    options: PluginExecutionSettings, 
-userKey?: string): Promise<T | undefined>;
+userKey?: string): Promise<ExecutionResult<T>>;
 ```
 
 Execute a function with the plugin's interceptor chain.
 
-All errors are caught and `undefined` is returned (production-safe).
-Route handlers should check for `undefined` and respond with an
-appropriate error status.
+Returns an [ExecutionResult](TypeAlias.ExecutionResult.md) discriminated union:
+- `{ ok: true, data: T }` on success
+- `{ ok: false, status: number, message: string }` on failure
+
+Errors are never thrown — the method is production-safe.
 
 #### Type Parameters
 
@@ -338,7 +340,7 @@ appropriate error status.
 
 #### Returns
 
-`Promise`\<`T` \| `undefined`\>
+`Promise`\<[`ExecutionResult`](TypeAlias.ExecutionResult.md)\<`T`\>\>
 
 ***
 

docs/docs/api/appkit/TypeAlias.ExecutionResult.md

@@ -0,0 +1,33 @@
+# Type Alias: ExecutionResult\<T\>
+
+```ts
+type ExecutionResult<T> = 
+  | {
+  data: T;
+  ok: true;
+}
+  | {
+  message: string;
+  ok: false;
+  status: number;
+};
+```
+
+Discriminated union for plugin execution results.
+
+Replaces the previous `T | undefined` return type on `execute()`.
+
+On failure, the HTTP status code is preserved from:
+- `AppKitError` subclasses (via `statusCode`)
+- Any `Error` with a numeric `statusCode` property (e.g. `ApiError`)
+- All other errors default to status 500
+
+In production, error messages from non-AppKitError sources are handled as:
+- 4xx errors: original message is preserved (client-facing by design)
+- 5xx errors: replaced with "Server error" to prevent information leakage
+
+## Type Parameters
+
+| Type Parameter |
+| ------ |
+| `T` |

docs/docs/api/appkit/index.md

@@ -54,6 +54,7 @@ plugin architecture, and React integration.
 | Type Alias | Description |
 | ------ | ------ |
 | [ConfigSchema](TypeAlias.ConfigSchema.md) | Configuration schema definition for plugin config. Re-exported from the standard JSON Schema Draft 7 types. |
+| [ExecutionResult](TypeAlias.ExecutionResult.md) | Discriminated union for plugin execution results. |
 | [IAppRouter](TypeAlias.IAppRouter.md) | Express router type for plugin route registration |
 | [PluginData](TypeAlias.PluginData.md) | Tuple of plugin class, config, and name. Created by `toPlugin()` and passed to `createApp()`. |
 | [ResourcePermission](TypeAlias.ResourcePermission.md) | Union of all possible permission levels across all resource types. |

docs/docs/api/appkit/typedoc-sidebar.ts

@@ -183,6 +183,11 @@ const typedocSidebar: SidebarsConfig = {
           id: "api/appkit/TypeAlias.ConfigSchema",
           label: "ConfigSchema"
         },
+        {
+          type: "doc",
+          id: "api/appkit/TypeAlias.ExecutionResult",
+          label: "ExecutionResult"
+        },
         {
           type: "doc",
           id: "api/appkit/TypeAlias.IAppRouter",

packages/appkit/src/index.ts

@@ -47,7 +47,12 @@ export {
   ValidationError,
 } from "./errors";
 // Plugin authoring
-export { Plugin, type ToPlugin, toPlugin } from "./plugin";
+export {
+  type ExecutionResult,
+  Plugin,
+  type ToPlugin,
+  toPlugin,
+} from "./plugin";
 export { analytics, files, genie, lakebase, server, serving } from "./plugins";
 export type {
   EndpointConfig,

packages/appkit/src/plugin/execution-result.ts

@@ -0,0 +1,17 @@
+/**
+ * Discriminated union for plugin execution results.
+ *
+ * Replaces the previous `T | undefined` return type on `execute()`.
+ *
+ * On failure, the HTTP status code is preserved from:
+ * - `AppKitError` subclasses (via `statusCode`)
+ * - Any `Error` with a numeric `statusCode` property (e.g. `ApiError`)
+ * - All other errors default to status 500
+ *
+ * In production, error messages from non-AppKitError sources are handled as:
+ * - 4xx errors: original message is preserved (client-facing by design)
+ * - 5xx errors: replaced with "Server error" to prevent information leakage
+ */
+export type ExecutionResult<T> =
+  | { ok: true; data: T }
+  | { ok: false; status: number; message: string };

packages/appkit/src/plugin/index.ts

@@ -1,3 +1,4 @@
 export type { ToPlugin } from "shared";
+export type { ExecutionResult } from "./execution-result";
 export { Plugin } from "./plugin";
 export { toPlugin } from "./to-plugin";

packages/appkit/src/plugin/plugin.ts

@@ -19,7 +19,7 @@ import {
   ServiceContext,
   type UserContext,
 } from "../context";
-import { AuthenticationError } from "../errors";
+import { AppKitError, AuthenticationError } from "../errors";
 import { createLogger } from "../logging/logger";
 import { StreamManager } from "../stream";
 import {
@@ -29,6 +29,7 @@ import {
 } from "../telemetry";
 import { deepMerge } from "../utils";
 import { DevFileReader } from "./dev-reader";
+import type { ExecutionResult } from "./execution-result";
 import { CacheInterceptor } from "./interceptors/cache";
 import { RetryInterceptor } from "./interceptors/retry";
 import { TelemetryInterceptor } from "./interceptors/telemetry";
@@ -40,6 +41,20 @@ import type {
 
 const logger = createLogger("plugin");
 
+/**
+ * Narrow an unknown thrown value to an Error that carries a numeric
+ * `statusCode` property (e.g. `ApiError` from `@databricks/sdk-experimental`).
+ */
+function hasHttpStatusCode(
+  error: unknown,
+): error is Error & { statusCode: number } {
+  return (
+    error instanceof Error &&
+    "statusCode" in error &&
+    typeof (error as Record<string, unknown>).statusCode === "number"
+  );
+}
+
 /**
  * Methods that should not be proxied by asUser().
  * These are lifecycle/internal methods that don't make sense
@@ -435,15 +450,17 @@ export abstract class Plugin<
   /**
    * Execute a function with the plugin's interceptor chain.
    *
-   * All errors are caught and `undefined` is returned (production-safe).
-   * Route handlers should check for `undefined` and respond with an
-   * appropriate error status.
+   * Returns an {@link ExecutionResult} discriminated union:
+   * - `{ ok: true, data: T }` on success
+   * - `{ ok: false, status: number, message: string }` on failure
+   *
+   * Errors are never thrown — the method is production-safe.
    */
   protected async execute<T>(
     fn: (signal?: AbortSignal) => Promise<T>,
     options: PluginExecutionSettings,
     userKey?: string,
-  ): Promise<T | undefined> {
+  ): Promise<ExecutionResult<T>> {
     const executeConfig = this._buildExecutionConfig(options);
 
     const interceptors = this._buildInterceptors(executeConfig);
@@ -457,11 +474,40 @@ export abstract class Plugin<
     };
 
     try {
-      return await this._executeWithInterceptors(fn, interceptors, context);
+      const data = await this._executeWithInterceptors(
+        fn,
+        interceptors,
+        context,
+      );
+      return { ok: true, data };
     } catch (error) {
-      // production-safe: swallow all errors, don't crash the app
       logger.error("Plugin execution failed", { error, plugin: this.name });
-      return undefined;
+
+      if (error instanceof AppKitError) {
+        return {
+          ok: false,
+          status: error.statusCode,
+          message: error.message,
+        };
+      }
+
+      if (hasHttpStatusCode(error)) {
+        const isDev = process.env.NODE_ENV !== "production";
+        const isClientError = error.statusCode >= 400 && error.statusCode < 500;
+        return {
+          ok: false,
+          status: error.statusCode,
+          message: isDev || isClientError ? error.message : "Server error",
+        };
+      }
+
+      const isDev = process.env.NODE_ENV !== "production";
+      return {
+        ok: false,
+        status: 500,
+        message:
+          isDev && error instanceof Error ? error.message : "Server error",
+      };
     }
   }