Merge pull request #282 from RhysSullivan/rs/typed-error-edge-model

feat(sdk): typed errors propagate to one edge mapping (StorageError, InternalError, Telemetry)

Author: RhysSullivan

apps/cloud/src/api/protected-layers.ts

@@ -6,7 +6,11 @@
 import { HttpApiBuilder, HttpRouter, HttpServer } from "@effect/platform";
 import { Layer } from "effect";
 
-import { CoreExecutorApi } from "@executor/api";
+import {
+  CoreExecutorApi,
+  InternalError,
+  observabilityMiddleware,
+} from "@executor/api";
 import { CoreHandlers } from "@executor/api/server";
 import { OpenApiGroup, OpenApiHandlers } from "@executor/plugin-openapi/api";
 import { McpGroup, McpHandlers } from "@executor/plugin-mcp/api";
@@ -18,12 +22,16 @@ import { UserStoreService } from "../auth/context";
 import { WorkOSAuth } from "../auth/workos";
 import { AutumnService } from "../services/autumn";
 import { DbService } from "../services/db";
+import { ErrorCaptureLive } from "../observability";
 
 export const ProtectedCloudApi = CoreExecutorApi.add(OpenApiGroup)
   .add(McpGroup)
   .add(GraphqlGroup)
+  .addError(InternalError)
   .middleware(OrgAuth);
 
+const ObservabilityLive = observabilityMiddleware(ProtectedCloudApi);
+
 const DbLive = DbService.Live;
 const UserStoreLive = UserStoreService.Live.pipe(Layer.provide(DbLive));
 
@@ -47,6 +55,13 @@ export const ProtectedCloudApiHandlers = Layer.mergeAll(
   GraphqlHandlers,
 );
 
+// `ErrorCaptureLive` is provided above the handler + middleware layers
+// so the `withCapture` translation path (typed-channel `StorageError →
+// InternalError(traceId)`) AND the observability middleware's defect
+// catchall both see the same Sentry-backed implementation.
 export const ProtectedCloudApiLive = HttpApiBuilder.api(ProtectedCloudApi).pipe(
-  Layer.provide(Layer.merge(ProtectedCloudApiHandlers, OrgAuthLive)),
+  Layer.provide(
+    Layer.mergeAll(ProtectedCloudApiHandlers, OrgAuthLive, ObservabilityLive),
+  ),
+  Layer.provide(ErrorCaptureLive),
 );

apps/cloud/src/api/protected.ts

@@ -34,6 +34,9 @@ const createProtectedApp = (organizationId: string, organizationName: string) =>
   Effect.gen(function* () {
     const { executor, engine } = yield* makeExecutionStack(organizationId, organizationName);
 
+    // Handlers wrap their own bodies with `capture(...)` — the edge
+    // translation lives per-handler, not at service construction. See
+    // notes/error-handling.md.
     const requestServices = Layer.mergeAll(
       Layer.succeed(ExecutorService, executor),
       Layer.succeed(ExecutionEngineService, engine),

apps/cloud/src/observability.ts

@@ -0,0 +1,31 @@
+// ---------------------------------------------------------------------------
+// Cloud-app implementation of the shared `ErrorCapture` service. This is the
+// only file in the cloud-app that imports `@sentry/cloudflare` for error
+// capture — handlers, plugin SDKs, and storage code all stay
+// Sentry-agnostic and request the `ErrorCapture` tag instead.
+//
+// `withObservability` (in @executor/api) wraps every handler effect; when
+// it sees an unmapped cause it asks `ErrorCapture.captureException` for a
+// trace id and fails with `InternalError({ traceId })`. The client gets
+// the opaque id, we get the full cause + stack in Sentry.
+// ---------------------------------------------------------------------------
+
+import * as Sentry from "@sentry/cloudflare";
+import { Cause, Effect, Layer } from "effect";
+
+import { ErrorCapture } from "@executor/api";
+
+export const ErrorCaptureLive: Layer.Layer<ErrorCapture> = Layer.succeed(
+  ErrorCapture,
+  ErrorCapture.of({
+    captureException: (cause) =>
+      Effect.sync(() => {
+        const squashed = Cause.squash(cause);
+        const eventId = Sentry.captureException(squashed, (scope) => {
+          scope.setExtra("cause", Cause.pretty(cause));
+          return scope;
+        });
+        return eventId ?? "";
+      }),
+  }),
+);

apps/cloud/src/services/executor.ts

@@ -78,5 +78,9 @@ export const createScopedExecutor = (
       createdAt: new Date(),
     });
 
+    // The executor surface returns raw `StorageFailure`; translation to
+    // the opaque `InternalError({ traceId })` happens at the HTTP edge
+    // via `withCapture` (see `api/protected-layers.ts`). That's
+    // where `ErrorCaptureLive` (Sentry) gets wired in.
     return yield* createExecutor({ scope, adapter, blobs, plugins });
   });

apps/local/src/server/main.ts

@@ -7,7 +7,7 @@ import {
 } from "@effect/platform";
 import { Context, Effect, Layer, ManagedRuntime } from "effect";
 
-import { addGroup } from "@executor/api";
+import { addGroup, observabilityMiddleware } from "@executor/api";
 import { CoreHandlers, ExecutorService, ExecutionEngineService } from "@executor/api/server";
 import { createExecutionEngine } from "@executor/execution";
 import { makeQuickJsExecutor } from "@executor/runtime-quickjs";
@@ -34,6 +34,7 @@ import {
 } from "@executor/plugin-graphql/api";
 import { getExecutor } from "./executor";
 import { createMcpRequestHandler, type McpRequestHandler } from "./mcp";
+import { ErrorCaptureLive } from "./observability";
 
 // ---------------------------------------------------------------------------
 // Local server API — core + all plugin groups
@@ -45,6 +46,13 @@ const LocalApi = addGroup(OpenApiGroup)
   .add(OnePasswordGroup)
   .add(GraphqlGroup);
 
+// `ErrorCaptureLive` logs causes to the console and returns a short
+// correlation id. Provided above the handler + middleware layers so
+// both the `withCapture` typed-channel translation AND the
+// `observabilityMiddleware` defect catchall see the same
+// implementation.
+const LocalObservability = observabilityMiddleware(LocalApi);
+
 const LocalApiBase = HttpApiBuilder.api(LocalApi).pipe(
   Layer.provide(CoreHandlers),
   Layer.provide(
@@ -56,6 +64,8 @@ const LocalApiBase = HttpApiBuilder.api(LocalApi).pipe(
       GraphqlHandlers,
     ),
   ),
+  Layer.provide(LocalObservability),
+  Layer.provide(ErrorCaptureLive),
 );
 
 // ---------------------------------------------------------------------------
@@ -81,6 +91,8 @@ export const createServerHandlers = async (): Promise<ServerHandlers> => {
   const executor = await getExecutor();
   const engine = createExecutionEngine({ executor, codeExecutor: makeQuickJsExecutor() });
 
+  // Handlers wrap their own bodies with `capture(...)` — the edge
+  // translation lives per-handler, not at service construction.
   const pluginExtensions = Layer.mergeAll(
     Layer.succeed(OpenApiExtensionService, executor.openapi),
     Layer.succeed(McpExtensionService, executor.mcp),

apps/local/src/server/observability.ts

@@ -0,0 +1,32 @@
+// ---------------------------------------------------------------------------
+// Local-app `ErrorCapture` — console implementation.
+//
+// Unlike the cloud app (Sentry-backed), the CLI just prints the squashed
+// cause + pretty-printed structured cause to stderr and returns a short
+// correlation id. Operators can grep for the id in their terminal
+// scrollback when a user reports an opaque 500 traceId.
+// ---------------------------------------------------------------------------
+
+import { Cause, Effect, Layer } from "effect";
+
+import { ErrorCapture } from "@executor/api";
+
+const nextTraceId = () =>
+  `local-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}`;
+
+export const ErrorCaptureLive: Layer.Layer<ErrorCapture> = Layer.succeed(
+  ErrorCapture,
+  ErrorCapture.of({
+    captureException: (cause) =>
+      Effect.sync(() => {
+        const traceId = nextTraceId();
+        const squashed = Cause.squash(cause);
+        console.error(
+          `[executor ${traceId}]`,
+          squashed instanceof Error ? squashed.stack ?? squashed : squashed,
+        );
+        console.error(`[executor ${traceId}] cause:`, Cause.pretty(cause));
+        return traceId;
+      }),
+  }),
+);

bun.lock

@@ -0,0 +1,195 @@
+# Error handling — model + plumbing
+
+## Principle
+
+Errors are typed values that propagate through the call graph carrying
+their full causal structure. **Exactly one layer at the edge** consumes
+them and maps them to a public response. Plugin code never imports
+Sentry, never calls `captureException`, never wraps handlers with
+`sanitize*` helpers.
+
+Two error categories:
+
+- **Surfaceable**: typed Schema errors with a user-actionable message
+  (`McpOAuthError`, `OpenApiParseError`, …). Carry through to the
+  response with their own status (4xx) and message body. Not captured
+  to any external sink — they're normal business outcomes.
+- **Internal**: truly unexpected (storage failure, third-party returned
+  garbage, sync throw inside a handler). Captured via `ErrorCapture`
+  **at the HTTP edge only**, then propagated as the shared
+  `InternalError({ traceId })` schema — opaque to clients, fully
+  detailed in Sentry. One shape, one trace id, one place to look up
+  the cause.
+
+## Layering
+
+```
+ storage-core ──▶ sdk ──▶ api (HTTP edge) ──▶ host
+   StorageError   StorageError     InternalError       ErrorCaptureLive
+   (raw)          (raw)             (captured)          (Sentry)
+```
+
+The SDK stays storage-typed. The HTTP edge (`@executor/api`) is the
+**only** layer that translates `StorageError → InternalError` and
+captures the cause to telemetry. Non-HTTP consumers (CLI, Promise
+SDK, tests) see raw `StorageError` in the typed channel and can react
+however they want.
+
+## Plumbing
+
+### Storage layer (`@executor/storage-core`)
+
+Emits two `Data.TaggedError` classes and nothing else observability-related:
+
+- `StorageError({ message, cause })` — the catch-all for non-recoverable
+  backend failures. The `cause` travels as runtime data so the HTTP
+  edge can capture it, but it's never serialised to the wire.
+- `UniqueViolationError({ model? })` — typed 4xx-shaped failure plugins
+  want to react to (e.g. "source already exists").
+
+Both are `Data.TaggedError`, not `Schema.TaggedError` — you physically
+can't `addError(...)` them on an HttpApi group, which enforces "these
+are internal types, not wire shapes".
+
+`DBAdapter` / `CustomAdapter` / `TypedAdapter` declare
+`Effect<X, StorageFailure>` where `StorageFailure = StorageError |
+UniqueViolationError`. No `Error`, no telemetry service in R.
+
+Storage-core has zero observability awareness — it just emits typed
+values and lets consumers decide what to do with them.
+
+### SDK (`@executor/sdk`)
+
+The SDK is entirely observability-free.
+
+- `createExecutor` requires no observability service. `R = never`.
+- `PluginCtx.storage`, `ctx.core.*`, `ctx.secrets.*`, `ctx.transaction`
+  all surface raw `StorageFailure` in the typed error channel. Plugins
+  can `Effect.catchTag("UniqueViolationError", …)` and translate to
+  their own user-facing errors.
+- Executor public methods (`executor.tools.list()`,
+  `executor.sources.refresh()`, etc.) also surface raw `StorageFailure`.
+
+No `liftStorage`, no `wrapAdapterForPlugin`, no `ErrorCapture` tag
+inside the SDK. The value proposition: an SDK consumer can write a CLI,
+a script, a promise-based wrapper, whatever — and the typed channel
+shows them exactly what can go wrong.
+
+### HTTP edge (`@executor/api/observability`)
+
+Owns the translation, the opaque wire schema, and the capture service.
+
+- `InternalError({ traceId })` — the public opaque 500 schema, with an
+  `HttpApiSchema.annotations({ status: 500 })` annotation so the
+  framework encodes it correctly.
+- `ErrorCapture` — tagged Effect service for recording unexpected
+  causes. Shape:
+
+  ```ts
+  interface ErrorCaptureShape {
+    readonly captureException: (cause: Cause<unknown>) => Effect<string>
+  }
+  ```
+
+  Optional — resolved via `Effect.serviceOption`; missing service =
+  empty trace ids. Nothing breaks if it's not wired.
+
+- `capture(eff)` — the single translator. Catches `StorageError` on
+  the typed channel, captures the cause via `ErrorCapture`, fails with
+  `InternalError({ traceId })`. Catches `UniqueViolationError` and
+  dies (plugins that want to surface it as a typed domain error should
+  `Effect.catchTag` inside their own method first). Every other typed
+  failure passes through.
+
+  Every handler wraps its generator body with `capture(...)`:
+
+  ```ts
+  .handle("probeEndpoint", ({ payload }) =>
+    capture(Effect.gen(function* () {
+      const ext = yield* McpExtensionService;
+      return yield* ext.probeEndpoint(payload.endpoint);
+    })),
+  )
+  ```
+
+  One line at the top of each handler. No service-level proxy, no
+  `Captured<T>` type gymnastics — the translation is visible right
+  where it happens, and TypeScript rejects handlers that forget
+  (because `StorageError` isn't in the group's `.addError` list).
+
+- `observabilityMiddleware(Api)` — defect safety net. An
+  `HttpApiBuilder.middleware` layer that wraps the HttpApp once and
+  catches any cause that escaped the typed channel (defects,
+  interrupts, framework bugs) via `ErrorCapture`, returning a typed
+  `InternalError({ traceId })`. Should rarely fire when the rest of
+  the pipeline is well-typed.
+
+### Plugin SDK
+
+Plugin authors write normal Effect code. Their extension method error
+unions look like:
+
+```ts
+Effect.Effect<X, MyPluginTypedError | StorageError, never>
+```
+
+Where `MyPluginTypedError` is the union of their own
+`Schema.TaggedError` classes (with `HttpApiSchema.annotations({ status: 4xx })`).
+`StorageError` is the raw storage tag — it bubbles up, and the HTTP
+edge translates it.
+
+Plugins never provide `ErrorCapture`, never import Sentry, never see
+`InternalError` in their typed channel.
+
+### API groups
+
+Each group declares its typed errors once at the group level:
+
+```ts
+class McpGroup extends HttpApiGroup.make("mcp")
+  .add(endpoint1)
+  .add(endpoint2)
+  // …
+  .addError(InternalError)
+  .addError(McpOAuthError)
+  .addError(McpConnectionError)
+  // …
+{}
+```
+
+No per-endpoint `addError`. The framework encodes each tagged error by
+its annotated status.
+
+### Hosts
+
+- **Cloud Worker** (`apps/cloud/src/observability.ts`) — provides
+  `ErrorCaptureLive`, a Sentry-backed implementation. Wired at the API
+  layer in `protected-layers.ts` so it's available to both
+  `observabilityMiddleware` (defect catchall) AND the per-handler
+  `capture(...)` translation. Service tags (`ExecutorService`,
+  `McpExtensionService`, etc.) hold the raw SDK shapes; the cloud app
+  just does `Layer.succeed(McpExtensionService, executor.mcp)` —
+  handlers do the translation themselves. (Distinct from
+  `apps/cloud/src/services/telemetry.ts`, which is the OTEL→Axiom span
+  bridge — "telemetry" in the tracing sense.)
+- **CLI** (`apps/local/src/server/main.ts`) — same pattern. Provides a
+  console-based `ErrorCaptureLive` (`apps/local/src/server/observability.ts`)
+  that prints the squashed cause + pretty cause to stderr and returns a
+  short correlation id.
+- **Tests / Promise SDK / examples** — non-HTTP consumers see raw
+  `StorageError` / `StorageFailure` in the SDK's typed channel and
+  can match on it directly.
+
+### Anti-patterns
+
+- `Effect.orDie` at handler boundaries — silently turns recoverable
+  failures into 500s with no telemetry (defects bypass typed-channel
+  encoding).
+- Per-plugin `*InternalError` types — clients can't tell which plugin
+  emitted a 500 anyway. Use the shared `InternalError`.
+- `sanitize*` helpers in handler files that `catchAllCause` + map to a
+  generic 500 — same swallowing problem in disguise. Prefer wrapping
+  the handler's `Effect.gen` body with `capture(...)`.
+- SDK code importing `Sentry.captureException` or referencing
+  `InternalError` / `ErrorCapture` — translation lives strictly in
+  `@executor/api`. If the SDK imports it, the layering is wrong.