From d138a9d98b423f2e1602dc20df437ee674f431e6 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Thu, 9 Apr 2026 14:10:33 -0700 Subject: [PATCH 1/4] Tighten HTTP error schemas, keep rich causes internally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Establish a clear rule: Schema.TaggedError is HTTP wire-facing — every field must be a safe public field. Data.TaggedError is internal-only; it can carry rich cause: unknown for logging/debugging and can't be accidentally wired to an HttpApi endpoint because it isn't a Schema. - OpenApiParseError (HTTP-exposed): drop error: Schema.Defect field - GraphqlIntrospectionError (HTTP-exposed): drop error: Schema.Defect field; log underlying causes via Effect.logError at construction sites in introspect.ts - OpenApiInvocationError, GraphqlInvocationError, GoogleDiscoveryParseError, GoogleDiscoveryInvocationError, KeychainError, ToolInvocationError: convert Schema.TaggedError -> Data.TaggedError, rename error -> cause - Cloud auth (UserStoreError, WorkOSError): keep empty schema + 500 annotation, log full cause via Effect.tapErrorCause at service wrappers --- apps/cloud/src/auth/context.ts | 9 +++---- apps/cloud/src/auth/errors.ts | 4 --- apps/cloud/src/auth/workos.ts | 9 +++---- packages/core/sdk/src/errors.ts | 15 +++++------ .../google-discovery/src/sdk/document.ts | 6 ++--- .../google-discovery/src/sdk/errors.ts | 27 +++++++++---------- .../google-discovery/src/sdk/invoke.ts | 3 +-- packages/plugins/graphql/src/sdk/errors.ts | 17 ++++++------ .../plugins/graphql/src/sdk/introspect.ts | 27 ++++++++++++------- packages/plugins/graphql/src/sdk/invoke.ts | 2 +- packages/plugins/keychain/src/errors.ts | 13 ++++----- packages/plugins/openapi/src/sdk/errors.ts | 17 ++++++------ packages/plugins/openapi/src/sdk/invoke.ts | 4 +-- packages/plugins/openapi/src/sdk/parse.ts | 1 - 14 files changed, 69 insertions(+), 85 deletions(-) diff --git a/apps/cloud/src/auth/context.ts b/apps/cloud/src/auth/context.ts index 6c937c5eda..31ba5472ba 100644 --- a/apps/cloud/src/auth/context.ts +++ b/apps/cloud/src/auth/context.ts @@ -16,13 +16,10 @@ const makeService = (store: RawStore) => { const use = (fn: (s: RawStore) => Promise) => Effect.tryPromise({ try: () => fn(store), - catch: (cause) => cause, + catch: (e) => e, }).pipe( - Effect.tapError((cause) => - Effect.sync(() => { - // eslint-disable-next-line no-console - console.error("[user_store] query failed:", cause); - }), + Effect.tapErrorCause((cause) => + Effect.logError("user_store query failed", cause), ), Effect.mapError(() => new UserStoreError()), Effect.withSpan("user_store"), diff --git a/apps/cloud/src/auth/errors.ts b/apps/cloud/src/auth/errors.ts index 43a17139b0..3c8cc8068e 100644 --- a/apps/cloud/src/auth/errors.ts +++ b/apps/cloud/src/auth/errors.ts @@ -1,10 +1,6 @@ import { HttpApiSchema } from "@effect/platform"; import { Schema } from "effect"; -// Tagged errors returned to HTTP clients. The response payload must NEVER -// include internal details (SQL, stack traces, WorkOS error bodies) — those -// can leak schema + secrets. Keep the schema empty and log the `cause` -// server-side via Effect's logger when the error is constructed. export class UserStoreError extends Schema.TaggedError()( "UserStoreError", {}, diff --git a/apps/cloud/src/auth/workos.ts b/apps/cloud/src/auth/workos.ts index 58c6fa8c69..3f7ad26593 100644 --- a/apps/cloud/src/auth/workos.ts +++ b/apps/cloud/src/auth/workos.ts @@ -28,13 +28,10 @@ const make = Effect.gen(function* () { const use = (fn: (wos: WorkOS) => Promise) => Effect.tryPromise({ try: () => fn(workos), - catch: (cause) => cause, + catch: (e) => e, }).pipe( - Effect.tapError((cause) => - Effect.sync(() => { - // eslint-disable-next-line no-console - console.error("[workos] call failed:", cause); - }), + Effect.tapErrorCause((cause) => + Effect.logError("workos call failed", cause), ), Effect.mapError(() => new WorkOSError()), Effect.withSpan("workos"), diff --git a/packages/core/sdk/src/errors.ts b/packages/core/sdk/src/errors.ts index c924055466..5596424e22 100644 --- a/packages/core/sdk/src/errors.ts +++ b/packages/core/sdk/src/errors.ts @@ -1,4 +1,4 @@ -import { Schema } from "effect"; +import { Data, Schema } from "effect"; import { ToolId, SecretId, PolicyId } from "./ids"; @@ -7,14 +7,13 @@ export class ToolNotFoundError extends Schema.TaggedError()( { toolId: ToolId }, ) {} -export class ToolInvocationError extends Schema.TaggedError()( +export class ToolInvocationError extends Data.TaggedError( "ToolInvocationError", - { - toolId: ToolId, - message: Schema.String, - cause: Schema.optional(Schema.Unknown), - }, -) {} +)<{ + readonly toolId: ToolId; + readonly message: string; + readonly cause?: unknown; +}> {} export class SecretNotFoundError extends Schema.TaggedError()( "SecretNotFoundError", diff --git a/packages/plugins/google-discovery/src/sdk/document.ts b/packages/plugins/google-discovery/src/sdk/document.ts index 654e2e8914..65fd1b0965 100644 --- a/packages/plugins/google-discovery/src/sdk/document.ts +++ b/packages/plugins/google-discovery/src/sdk/document.ts @@ -140,8 +140,8 @@ const DiscoveryDocumentModel = Schema.Struct({ }); type DiscoveryDocument = typeof DiscoveryDocumentModel.Type; -const toParseError = (message: string, error: unknown) => - new GoogleDiscoveryParseError({ message, error }); +const toParseError = (message: string, cause: unknown) => + new GoogleDiscoveryParseError({ message, cause }); const decodeUnknownWith = ( message: string, @@ -436,7 +436,6 @@ const manifestMethodFromMethod = (input: { if (!method.httpMethod) { return yield* new GoogleDiscoveryParseError({ message: `Google Discovery method '${methodId}' is missing httpMethod`, - error: new Error(`Missing httpMethod for ${methodId}`), }); } @@ -509,7 +508,6 @@ export const extractGoogleDiscoveryManifest = Effect.fn( return yield* new GoogleDiscoveryParseError({ message: "Google Discovery document is missing one of: name, version, rootUrl", - error: new Error("Invalid document"), }); } diff --git a/packages/plugins/google-discovery/src/sdk/errors.ts b/packages/plugins/google-discovery/src/sdk/errors.ts index 14d604b596..17c6fa70be 100644 --- a/packages/plugins/google-discovery/src/sdk/errors.ts +++ b/packages/plugins/google-discovery/src/sdk/errors.ts @@ -1,21 +1,20 @@ -import { Schema } from "effect"; +import { Data, Schema } from "effect"; +import type { Option } from "effect"; -export class GoogleDiscoveryParseError extends Schema.TaggedError()( +export class GoogleDiscoveryParseError extends Data.TaggedError( "GoogleDiscoveryParseError", - { - message: Schema.String, - error: Schema.Defect, - }, -) {} +)<{ + readonly message: string; + readonly cause?: unknown; +}> {} -export class GoogleDiscoveryInvocationError extends Schema.TaggedError()( +export class GoogleDiscoveryInvocationError extends Data.TaggedError( "GoogleDiscoveryInvocationError", - { - message: Schema.String, - statusCode: Schema.optionalWith(Schema.Number, { as: "Option" }), - error: Schema.Defect, - }, -) {} +)<{ + readonly message: string; + readonly statusCode: Option.Option; + readonly cause?: unknown; +}> {} export class GoogleDiscoveryOAuthError extends Schema.TaggedError()( "GoogleDiscoveryOAuthError", diff --git a/packages/plugins/google-discovery/src/sdk/invoke.ts b/packages/plugins/google-discovery/src/sdk/invoke.ts index 61134cdfff..f4066195bc 100644 --- a/packages/plugins/google-discovery/src/sdk/invoke.ts +++ b/packages/plugins/google-discovery/src/sdk/invoke.ts @@ -277,7 +277,6 @@ const invoke = Effect.fn("GoogleDiscovery.invoke")(function* (input: { return yield* new GoogleDiscoveryInvocationError({ message: `Missing required ${parameter.location} parameter: ${parameter.name}`, statusCode: Option.none(), - error: undefined, }); } continue; @@ -327,7 +326,7 @@ const invoke = Effect.fn("GoogleDiscovery.invoke")(function* (input: { new GoogleDiscoveryInvocationError({ message: `HTTP request failed: ${err.message}`, statusCode: Option.none(), - error: err, + cause: err, }), ), ); diff --git a/packages/plugins/graphql/src/sdk/errors.ts b/packages/plugins/graphql/src/sdk/errors.ts index 8b3e888797..ec2965c3ec 100644 --- a/packages/plugins/graphql/src/sdk/errors.ts +++ b/packages/plugins/graphql/src/sdk/errors.ts @@ -1,10 +1,10 @@ -import { Schema } from "effect"; +import { Data, Schema } from "effect"; +import type { Option } from "effect"; export class GraphqlIntrospectionError extends Schema.TaggedError()( "GraphqlIntrospectionError", { message: Schema.String, - error: Schema.Defect, }, ) {} @@ -15,11 +15,10 @@ export class GraphqlExtractionError extends Schema.TaggedError()( +export class GraphqlInvocationError extends Data.TaggedError( "GraphqlInvocationError", - { - message: Schema.String, - statusCode: Schema.optionalWith(Schema.Number, { as: "Option" }), - error: Schema.Defect, - }, -) {} +)<{ + readonly message: string; + readonly statusCode: Option.Option; + readonly cause?: unknown; +}> {} diff --git a/packages/plugins/graphql/src/sdk/introspect.ts b/packages/plugins/graphql/src/sdk/introspect.ts index 15fa665b52..414df8b456 100644 --- a/packages/plugins/graphql/src/sdk/introspect.ts +++ b/packages/plugins/graphql/src/sdk/introspect.ts @@ -149,29 +149,36 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( } const response = yield* client.execute(request).pipe( + Effect.tapErrorCause((cause) => + Effect.logError("graphql introspection: request failed", cause), + ), Effect.mapError( (err) => new GraphqlIntrospectionError({ message: `Failed to reach GraphQL endpoint: ${err.message}`, - error: err, }), ), ); if (response.status !== 200) { const body = yield* response.text.pipe(Effect.catchAll(() => Effect.succeed(""))); + yield* Effect.logError( + `graphql introspection: status ${response.status}`, + body, + ); return yield* new GraphqlIntrospectionError({ - message: `Introspection failed with status ${response.status}: ${body}`, - error: undefined, + message: `Introspection failed with status ${response.status}`, }); } const raw = yield* response.json.pipe( + Effect.tapErrorCause((cause) => + Effect.logError("graphql introspection: JSON parse failed", cause), + ), Effect.mapError( - (err) => + () => new GraphqlIntrospectionError({ message: `Failed to parse introspection response as JSON`, - error: err, }), ), ); @@ -179,17 +186,18 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( const json = raw as { data?: IntrospectionResult; errors?: unknown[] }; if (json.errors && Array.isArray(json.errors) && json.errors.length > 0) { + yield* Effect.logError( + `graphql introspection: endpoint returned errors`, + json.errors, + ); return yield* new GraphqlIntrospectionError({ - // @effect-diagnostics-next-line preferSchemaOverJson:off - message: `Introspection returned errors: ${JSON.stringify(json.errors)}`, - error: undefined, + message: `Introspection returned ${json.errors.length} error(s)`, }); } if (!json.data?.__schema) { return yield* new GraphqlIntrospectionError({ message: "Introspection response missing __schema", - error: undefined, }); } @@ -216,6 +224,5 @@ export const parseIntrospectionJson = ( catch: (err) => new GraphqlIntrospectionError({ message: `Failed to parse introspection JSON: ${err instanceof Error ? err.message : String(err)}`, - error: err, }), }); diff --git a/packages/plugins/graphql/src/sdk/invoke.ts b/packages/plugins/graphql/src/sdk/invoke.ts index 00bd4cf078..7ae4a2a3d2 100644 --- a/packages/plugins/graphql/src/sdk/invoke.ts +++ b/packages/plugins/graphql/src/sdk/invoke.ts @@ -109,7 +109,7 @@ export const invoke = Effect.fn("GraphQL.invoke")(function* ( new GraphqlInvocationError({ message: `GraphQL request failed: ${err.message}`, statusCode: Option.none(), - error: err, + cause: err, }), ), ); diff --git a/packages/plugins/keychain/src/errors.ts b/packages/plugins/keychain/src/errors.ts index 3e0eaaf4b3..53730ac0b3 100644 --- a/packages/plugins/keychain/src/errors.ts +++ b/packages/plugins/keychain/src/errors.ts @@ -1,9 +1,6 @@ -import { Schema } from "effect"; +import { Data } from "effect"; -export class KeychainError extends Schema.TaggedError()( - "KeychainError", - { - message: Schema.String, - cause: Schema.optional(Schema.Unknown), - }, -) {} +export class KeychainError extends Data.TaggedError("KeychainError")<{ + readonly message: string; + readonly cause?: unknown; +}> {} diff --git a/packages/plugins/openapi/src/sdk/errors.ts b/packages/plugins/openapi/src/sdk/errors.ts index 2c46d558dd..46478c7cbc 100644 --- a/packages/plugins/openapi/src/sdk/errors.ts +++ b/packages/plugins/openapi/src/sdk/errors.ts @@ -1,10 +1,10 @@ -import { Schema } from "effect"; +import { Data, Schema } from "effect"; +import type { Option } from "effect"; export class OpenApiParseError extends Schema.TaggedError()( "OpenApiParseError", { message: Schema.String, - error: Schema.Defect, }, ) {} @@ -15,11 +15,10 @@ export class OpenApiExtractionError extends Schema.TaggedError()( +export class OpenApiInvocationError extends Data.TaggedError( "OpenApiInvocationError", - { - message: Schema.String, - statusCode: Schema.optionalWith(Schema.Number, { as: "Option" }), - error: Schema.Defect, - }, -) {} +)<{ + readonly message: string; + readonly statusCode: Option.Option; + readonly cause?: unknown; +}> {} diff --git a/packages/plugins/openapi/src/sdk/invoke.ts b/packages/plugins/openapi/src/sdk/invoke.ts index 82969fb451..0c45bcfb43 100644 --- a/packages/plugins/openapi/src/sdk/invoke.ts +++ b/packages/plugins/openapi/src/sdk/invoke.ts @@ -76,7 +76,6 @@ const resolvePath = Effect.fn("OpenApi.resolvePath")(function* ( return yield* new OpenApiInvocationError({ message: `Missing required path parameter: ${param.name}`, statusCode: Option.none(), - error: undefined, }); } continue; @@ -111,7 +110,6 @@ const resolvePath = Effect.fn("OpenApi.resolvePath")(function* ( return yield* new OpenApiInvocationError({ message: `Unresolved path parameters: ${[...new Set(unresolved)].join(", ")}`, statusCode: Option.none(), - error: undefined, }); } @@ -245,7 +243,7 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( new OpenApiInvocationError({ message: `HTTP request failed: ${err.message}`, statusCode: Option.none(), - error: err, + cause: err, }), ), ); diff --git a/packages/plugins/openapi/src/sdk/parse.ts b/packages/plugins/openapi/src/sdk/parse.ts index f97719cdfc..115c07978c 100644 --- a/packages/plugins/openapi/src/sdk/parse.ts +++ b/packages/plugins/openapi/src/sdk/parse.ts @@ -20,7 +20,6 @@ export const parse = Effect.fn("OpenApi.parse")(function* (input: string) { catch: (error) => new OpenApiParseError({ message: `Failed to parse OpenAPI document: ${error instanceof Error ? error.message : String(error)}`, - error, }), }); From ca47f16d36cac4c3bb3993c03b9716d5e692133e Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Thu, 9 Apr 2026 14:32:59 -0700 Subject: [PATCH 2/4] Extract withServiceLogging helper + errors.test.ts contract Collapse the repetitive tapErrorCause + mapError + withSpan ceremony at each service wrapper into one helper in auth/errors.ts. Add a real HttpApi integration test that pins down the contract: - Wire response contains only declared tagged-error fields - Server-side logs capture the full Cause chain (drizzle query, params, nested pg Error.cause) The test uses a real HttpApi + HttpApiBuilder.toWebHandler + fetch round-trip with a capturing Logger to verify both sides without mocking. Validates Option C from the earlier discussion: service-level logging is the only pattern that preserves the full cause because mapError discards it before any edge middleware could see it. --- apps/cloud/src/auth/context.ts | 25 ++-- apps/cloud/src/auth/errors.test.ts | 195 +++++++++++++++++++++++++++++ apps/cloud/src/auth/errors.ts | 25 +++- apps/cloud/src/auth/workos.ts | 15 +-- 4 files changed, 233 insertions(+), 27 deletions(-) create mode 100644 apps/cloud/src/auth/errors.test.ts diff --git a/apps/cloud/src/auth/context.ts b/apps/cloud/src/auth/context.ts index 31ba5472ba..d12ec24019 100644 --- a/apps/cloud/src/auth/context.ts +++ b/apps/cloud/src/auth/context.ts @@ -1,7 +1,7 @@ import { Context, Effect, Layer } from "effect"; import { makeUserStore } from "../services/user-store"; import { DbService } from "../services/db"; -import { UserStoreError } from "./errors"; +import { UserStoreError, withServiceLogging } from "./errors"; // AuthContext is defined in ./middleware.ts to keep middleware-related types together. export { AuthContext } from "./middleware"; @@ -12,21 +12,14 @@ export { AuthContext } from "./middleware"; type RawStore = ReturnType; -const makeService = (store: RawStore) => { - const use = (fn: (s: RawStore) => Promise) => - Effect.tryPromise({ - try: () => fn(store), - catch: (e) => e, - }).pipe( - Effect.tapErrorCause((cause) => - Effect.logError("user_store query failed", cause), - ), - Effect.mapError(() => new UserStoreError()), - Effect.withSpan("user_store"), - ); - - return { use }; -}; +const makeService = (store: RawStore) => ({ + use: (fn: (s: RawStore) => Promise) => + withServiceLogging( + "user_store", + () => new UserStoreError(), + Effect.tryPromise({ try: () => fn(store), catch: (e) => e }), + ), +}); type UserStoreServiceType = ReturnType; diff --git a/apps/cloud/src/auth/errors.test.ts b/apps/cloud/src/auth/errors.test.ts new file mode 100644 index 0000000000..92971e49eb --- /dev/null +++ b/apps/cloud/src/auth/errors.test.ts @@ -0,0 +1,195 @@ +// --------------------------------------------------------------------------- +// HTTP error-handling integration test +// --------------------------------------------------------------------------- +// +// Pins down two things we care about: +// +// 1. The wire response body contains ONLY the declared error schema. +// No SQL, no stack traces, no `cause` / `error` fields. +// 2. A server-side logger sees the full Cause chain (drizzle error, +// original message, etc.) — so bugs are still debuggable in +// production logs even though clients get nothing. +// +// The test builds a real HttpApi with an endpoint whose handler fails +// via the exact `tryPromise + tapErrorCause + mapError` pattern used in +// user-store/workos wrappers, then calls it via fetch and inspects both +// the response body and captured log lines. +// +// This lets us validate error-handling *patterns* without touching the +// prod wiring. + +import { describe, expect, it } from "vitest"; +import { + HttpApi, + HttpApiBuilder, + HttpApiEndpoint, + HttpApiGroup, + HttpApiSchema, + HttpServer, +} from "@effect/platform"; +import { + Cause, + Effect, + Layer, + Logger, + LogLevel, + Ref, + Schema, +} from "effect"; + +import { withServiceLogging } from "./errors"; + +// --------------------------------------------------------------------------- +// Fixture API — one endpoint that fails with a tagged error whose schema +// has only a `message` field. The handler runs a failing service call +// wrapped with `withServiceLogging` — the same pattern real service +// wrappers in context.ts and workos.ts use. +// --------------------------------------------------------------------------- + +class FixtureError extends Schema.TaggedError()( + "FixtureError", + { + message: Schema.String, + }, + HttpApiSchema.annotations({ status: 500 }), +) {} + +const FixtureGroup = HttpApiGroup.make("fixture").add( + HttpApiEndpoint.get("boom")`/boom` + .addSuccess(Schema.Struct({ ok: Schema.Boolean })) + .addError(FixtureError), +); + +const FixtureApi = HttpApi.make("fixture").add(FixtureGroup); + +// Drizzle-shaped error: carries a .cause with SQL + params, like +// postgres.js + drizzle-orm would. +const makeDrizzleError = () => { + const pgError = new Error( + 'duplicate key value violates unique constraint "accounts_pkey"', + ); + (pgError as { code?: string }).code = "23505"; + const drizzleError = new Error( + `Failed query: insert into "accounts" ("id") values ($1) returning "id"`, + ); + (drizzleError as { query?: string }).query = + 'insert into "accounts" ("id") values ($1) returning "id"'; + (drizzleError as { params?: unknown[] }).params = ["user_abc123"]; + (drizzleError as { cause?: unknown }).cause = pgError; + return drizzleError; +}; + +const failingUse = withServiceLogging( + "user_store", + () => new FixtureError({ message: "internal database error" }), + Effect.tryPromise({ + try: () => Promise.reject(makeDrizzleError()), + catch: (e) => e, + }), +); + +const FixtureGroupLive = HttpApiBuilder.group( + FixtureApi, + "fixture", + (handlers) => + handlers.handle("boom", () => failingUse), +); + +const FixtureApiLive = HttpApiBuilder.api(FixtureApi).pipe( + Layer.provide(FixtureGroupLive), +); + +// --------------------------------------------------------------------------- +// Test helper: run a request through the full HttpApi pipeline with a +// capturing logger, return the response + captured log messages. +// --------------------------------------------------------------------------- + +interface CapturedLog { + readonly level: string; + readonly message: string; + readonly causeText: string; +} + +const runWithCapturedLogs = async ( + layer: Layer.Layer, + request: Request, +): Promise<{ response: Response; logs: CapturedLog[] }> => { + const logsRef = await Effect.runPromise(Ref.make([])); + + const capturingLogger = Logger.make(({ logLevel, message, cause }) => { + const msg = Array.isArray(message) + ? message.map((p) => String(p)).join(" ") + : String(message); + const causeText = Cause.isEmpty(cause) + ? "" + : Cause.pretty(cause, { renderErrorCause: true }); + Effect.runSync( + Ref.update(logsRef, (xs) => [ + ...xs, + { level: logLevel.label, message: msg, causeText }, + ]), + ); + }); + + const LoggerLive = Logger.replace(Logger.defaultLogger, capturingLogger); + + const handler = HttpApiBuilder.toWebHandler( + layer.pipe( + Layer.provideMerge(HttpServer.layerContext), + Layer.provideMerge(LoggerLive), + Layer.provideMerge(Logger.minimumLogLevel(LogLevel.All)), + ), + ); + + const response = await handler.handler(request); + const logs = await Effect.runPromise(Ref.get(logsRef)); + return { response, logs }; +}; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("HTTP error boundary", () => { + it("returns only declared fields on the wire", async () => { + const { response } = await runWithCapturedLogs( + FixtureApiLive, + new Request("http://localhost/boom"), + ); + + expect(response.status).toBe(500); + const body = (await response.json()) as Record; + expect(body).toEqual({ + _tag: "FixtureError", + message: "internal database error", + }); + + // Explicit: none of the internal details leaked. + const bodyText = JSON.stringify(body); + expect(bodyText).not.toContain("duplicate key"); + expect(bodyText).not.toContain("accounts_pkey"); + expect(bodyText).not.toContain("insert into"); + expect(bodyText).not.toContain("user_abc123"); + expect(bodyText).not.toContain("23505"); + }); + + it("logs the full Cause chain server-side via tapErrorCause", async () => { + const { logs } = await runWithCapturedLogs( + FixtureApiLive, + new Request("http://localhost/boom"), + ); + + const errorLogs = logs.filter((l) => l.level === "ERROR"); + expect(errorLogs.length).toBeGreaterThan(0); + + const rendered = errorLogs + .map((l) => `${l.message} ${l.causeText}`) + .join("\n"); + + // The original drizzle query + params + underlying pg error should + // all be recoverable from the log output. + expect(rendered).toContain("user_store failed"); + expect(rendered).toContain("insert into"); + expect(rendered).toContain("duplicate key"); + }); +}); diff --git a/apps/cloud/src/auth/errors.ts b/apps/cloud/src/auth/errors.ts index 3c8cc8068e..51816503c1 100644 --- a/apps/cloud/src/auth/errors.ts +++ b/apps/cloud/src/auth/errors.ts @@ -1,5 +1,5 @@ import { HttpApiSchema } from "@effect/platform"; -import { Schema } from "effect"; +import { Effect, Schema } from "effect"; export class UserStoreError extends Schema.TaggedError()( "UserStoreError", @@ -12,3 +12,26 @@ export class WorkOSError extends Schema.TaggedError()( {}, HttpApiSchema.annotations({ status: 500 }), ) {} + +/** + * Service-boundary error wrapper. Logs the full Cause chain (drizzle + * query/params, pg error codes, nested Error.cause, etc.) via Effect's + * structured logger, then maps to a tagged error so the HTTP wire + * response contains only safe fields. + * + * Use this whenever a Promise-based API gets lifted into an Effect and + * its failure needs both debuggable server-side logging and a safe + * public shape. + */ +export const withServiceLogging = ( + name: string, + publicError: () => E, + effect: Effect.Effect, +): Effect.Effect => + effect.pipe( + Effect.tapErrorCause((cause) => + Effect.logError(`${name} failed`, cause), + ), + Effect.mapError(publicError), + Effect.withSpan(name), + ); diff --git a/apps/cloud/src/auth/workos.ts b/apps/cloud/src/auth/workos.ts index 3f7ad26593..fc4d217d0b 100644 --- a/apps/cloud/src/auth/workos.ts +++ b/apps/cloud/src/auth/workos.ts @@ -4,7 +4,7 @@ import { Context, Effect, Layer } from "effect"; import { WorkOS } from "@workos-inc/node/worker"; -import { WorkOSError } from "./errors"; +import { WorkOSError, withServiceLogging } from "./errors"; import { server } from "../env"; const COOKIE_NAME = "wos-session"; @@ -26,15 +26,10 @@ const make = Effect.gen(function* () { const workos = new WorkOS({ apiKey, clientId }); const use = (fn: (wos: WorkOS) => Promise) => - Effect.tryPromise({ - try: () => fn(workos), - catch: (e) => e, - }).pipe( - Effect.tapErrorCause((cause) => - Effect.logError("workos call failed", cause), - ), - Effect.mapError(() => new WorkOSError()), - Effect.withSpan("workos"), + withServiceLogging( + "workos", + () => new WorkOSError(), + Effect.tryPromise({ try: () => fn(workos), catch: (e) => e }), ); const authenticateSealedSession = (sessionData: string) => From 52b123731400e37ce31fce62260b639539a37f98 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Thu, 9 Apr 2026 14:36:27 -0700 Subject: [PATCH 3/4] Remove errors.test.ts --- apps/cloud/src/auth/errors.test.ts | 195 ----------------------------- 1 file changed, 195 deletions(-) delete mode 100644 apps/cloud/src/auth/errors.test.ts diff --git a/apps/cloud/src/auth/errors.test.ts b/apps/cloud/src/auth/errors.test.ts deleted file mode 100644 index 92971e49eb..0000000000 --- a/apps/cloud/src/auth/errors.test.ts +++ /dev/null @@ -1,195 +0,0 @@ -// --------------------------------------------------------------------------- -// HTTP error-handling integration test -// --------------------------------------------------------------------------- -// -// Pins down two things we care about: -// -// 1. The wire response body contains ONLY the declared error schema. -// No SQL, no stack traces, no `cause` / `error` fields. -// 2. A server-side logger sees the full Cause chain (drizzle error, -// original message, etc.) — so bugs are still debuggable in -// production logs even though clients get nothing. -// -// The test builds a real HttpApi with an endpoint whose handler fails -// via the exact `tryPromise + tapErrorCause + mapError` pattern used in -// user-store/workos wrappers, then calls it via fetch and inspects both -// the response body and captured log lines. -// -// This lets us validate error-handling *patterns* without touching the -// prod wiring. - -import { describe, expect, it } from "vitest"; -import { - HttpApi, - HttpApiBuilder, - HttpApiEndpoint, - HttpApiGroup, - HttpApiSchema, - HttpServer, -} from "@effect/platform"; -import { - Cause, - Effect, - Layer, - Logger, - LogLevel, - Ref, - Schema, -} from "effect"; - -import { withServiceLogging } from "./errors"; - -// --------------------------------------------------------------------------- -// Fixture API — one endpoint that fails with a tagged error whose schema -// has only a `message` field. The handler runs a failing service call -// wrapped with `withServiceLogging` — the same pattern real service -// wrappers in context.ts and workos.ts use. -// --------------------------------------------------------------------------- - -class FixtureError extends Schema.TaggedError()( - "FixtureError", - { - message: Schema.String, - }, - HttpApiSchema.annotations({ status: 500 }), -) {} - -const FixtureGroup = HttpApiGroup.make("fixture").add( - HttpApiEndpoint.get("boom")`/boom` - .addSuccess(Schema.Struct({ ok: Schema.Boolean })) - .addError(FixtureError), -); - -const FixtureApi = HttpApi.make("fixture").add(FixtureGroup); - -// Drizzle-shaped error: carries a .cause with SQL + params, like -// postgres.js + drizzle-orm would. -const makeDrizzleError = () => { - const pgError = new Error( - 'duplicate key value violates unique constraint "accounts_pkey"', - ); - (pgError as { code?: string }).code = "23505"; - const drizzleError = new Error( - `Failed query: insert into "accounts" ("id") values ($1) returning "id"`, - ); - (drizzleError as { query?: string }).query = - 'insert into "accounts" ("id") values ($1) returning "id"'; - (drizzleError as { params?: unknown[] }).params = ["user_abc123"]; - (drizzleError as { cause?: unknown }).cause = pgError; - return drizzleError; -}; - -const failingUse = withServiceLogging( - "user_store", - () => new FixtureError({ message: "internal database error" }), - Effect.tryPromise({ - try: () => Promise.reject(makeDrizzleError()), - catch: (e) => e, - }), -); - -const FixtureGroupLive = HttpApiBuilder.group( - FixtureApi, - "fixture", - (handlers) => - handlers.handle("boom", () => failingUse), -); - -const FixtureApiLive = HttpApiBuilder.api(FixtureApi).pipe( - Layer.provide(FixtureGroupLive), -); - -// --------------------------------------------------------------------------- -// Test helper: run a request through the full HttpApi pipeline with a -// capturing logger, return the response + captured log messages. -// --------------------------------------------------------------------------- - -interface CapturedLog { - readonly level: string; - readonly message: string; - readonly causeText: string; -} - -const runWithCapturedLogs = async ( - layer: Layer.Layer, - request: Request, -): Promise<{ response: Response; logs: CapturedLog[] }> => { - const logsRef = await Effect.runPromise(Ref.make([])); - - const capturingLogger = Logger.make(({ logLevel, message, cause }) => { - const msg = Array.isArray(message) - ? message.map((p) => String(p)).join(" ") - : String(message); - const causeText = Cause.isEmpty(cause) - ? "" - : Cause.pretty(cause, { renderErrorCause: true }); - Effect.runSync( - Ref.update(logsRef, (xs) => [ - ...xs, - { level: logLevel.label, message: msg, causeText }, - ]), - ); - }); - - const LoggerLive = Logger.replace(Logger.defaultLogger, capturingLogger); - - const handler = HttpApiBuilder.toWebHandler( - layer.pipe( - Layer.provideMerge(HttpServer.layerContext), - Layer.provideMerge(LoggerLive), - Layer.provideMerge(Logger.minimumLogLevel(LogLevel.All)), - ), - ); - - const response = await handler.handler(request); - const logs = await Effect.runPromise(Ref.get(logsRef)); - return { response, logs }; -}; - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describe("HTTP error boundary", () => { - it("returns only declared fields on the wire", async () => { - const { response } = await runWithCapturedLogs( - FixtureApiLive, - new Request("http://localhost/boom"), - ); - - expect(response.status).toBe(500); - const body = (await response.json()) as Record; - expect(body).toEqual({ - _tag: "FixtureError", - message: "internal database error", - }); - - // Explicit: none of the internal details leaked. - const bodyText = JSON.stringify(body); - expect(bodyText).not.toContain("duplicate key"); - expect(bodyText).not.toContain("accounts_pkey"); - expect(bodyText).not.toContain("insert into"); - expect(bodyText).not.toContain("user_abc123"); - expect(bodyText).not.toContain("23505"); - }); - - it("logs the full Cause chain server-side via tapErrorCause", async () => { - const { logs } = await runWithCapturedLogs( - FixtureApiLive, - new Request("http://localhost/boom"), - ); - - const errorLogs = logs.filter((l) => l.level === "ERROR"); - expect(errorLogs.length).toBeGreaterThan(0); - - const rendered = errorLogs - .map((l) => `${l.message} ${l.causeText}`) - .join("\n"); - - // The original drizzle query + params + underlying pg error should - // all be recoverable from the log output. - expect(rendered).toContain("user_store failed"); - expect(rendered).toContain("insert into"); - expect(rendered).toContain("duplicate key"); - }); -}); From 07efb478a026f770a0741a68b8b5587c0ed9fcb2 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Thu, 9 Apr 2026 14:49:59 -0700 Subject: [PATCH 4/4] Simplify graphql introspection error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop ad-hoc Effect.logError sprinkles on branch-level failures (bad status, returned errors, missing __schema) — the tagged error's message already carries the useful info and there's no Cause chain to tap. Keep tapErrorCause + mapError inline for the two cases where an upstream error exists (HTTP request failure, JSON parse failure). --- packages/plugins/graphql/src/sdk/introspect.ts | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/plugins/graphql/src/sdk/introspect.ts b/packages/plugins/graphql/src/sdk/introspect.ts index 414df8b456..62c3667f24 100644 --- a/packages/plugins/graphql/src/sdk/introspect.ts +++ b/packages/plugins/graphql/src/sdk/introspect.ts @@ -150,7 +150,7 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( const response = yield* client.execute(request).pipe( Effect.tapErrorCause((cause) => - Effect.logError("graphql introspection: request failed", cause), + Effect.logError("graphql introspection request failed", cause), ), Effect.mapError( (err) => @@ -161,11 +161,6 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( ); if (response.status !== 200) { - const body = yield* response.text.pipe(Effect.catchAll(() => Effect.succeed(""))); - yield* Effect.logError( - `graphql introspection: status ${response.status}`, - body, - ); return yield* new GraphqlIntrospectionError({ message: `Introspection failed with status ${response.status}`, }); @@ -173,7 +168,7 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( const raw = yield* response.json.pipe( Effect.tapErrorCause((cause) => - Effect.logError("graphql introspection: JSON parse failed", cause), + Effect.logError("graphql introspection JSON parse failed", cause), ), Effect.mapError( () => @@ -186,10 +181,6 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( const json = raw as { data?: IntrospectionResult; errors?: unknown[] }; if (json.errors && Array.isArray(json.errors) && json.errors.length > 0) { - yield* Effect.logError( - `graphql introspection: endpoint returned errors`, - json.errors, - ); return yield* new GraphqlIntrospectionError({ message: `Introspection returned ${json.errors.length} error(s)`, });