History

Tighten HTTP error schemas, keep rich causes internally (#137)

* Tighten HTTP error schemas, keep rich causes internally 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 * 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. * Remove errors.test.ts * Simplify graphql introspection error handling 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).

Rhys Sullivan committed 7 months ago

Commit d2ce9d71 parent 0ccccd914 files · +93 −107Browse files

Unified · Split

14 files
  1. apps/cloud/src/auth/context.ts +9 −19
  2. apps/cloud/src/auth/errors.ts +24 −5
  3. apps/cloud/src/auth/workos.ts +5 −13
  4. packages/core/sdk/src/errors.ts +7 −8
  5. packages/plugins/google-discovery/src/sdk/document.ts +2 −4
  6. packages/plugins/google-discovery/src/sdk/errors.ts +13 −14
  7. packages/plugins/google-discovery/src/sdk/invoke.ts +1 −2
  8. packages/plugins/graphql/src/sdk/errors.ts +8 −9
  9. packages/plugins/graphql/src/sdk/introspect.ts +9 −11
  10. packages/plugins/graphql/src/sdk/invoke.ts +1 −1
  11. packages/plugins/keychain/src/errors.ts +5 −8
  12. packages/plugins/openapi/src/sdk/errors.ts +8 −9
  13. packages/plugins/openapi/src/sdk/invoke.ts +1 −3
  14. packages/plugins/openapi/src/sdk/parse.ts +0 −1
apps/cloud/src/auth/context.ts+9−19
@@ -1,7 +1,7 @@
1import { Context, Effect, Layer } from "effect";1import { Context, Effect, Layer } from "effect";
2import { makeUserStore } from "../services/user-store";2import { makeUserStore } from "../services/user-store";
3import { DbService } from "../services/db";3import { DbService } from "../services/db";
4import { UserStoreError } from "./errors";4import { UserStoreError, withServiceLogging } from "./errors";
5 5
6// AuthContext is defined in ./middleware.ts to keep middleware-related types together.6// AuthContext is defined in ./middleware.ts to keep middleware-related types together.
7export { AuthContext } from "./middleware";7export { AuthContext } from "./middleware";
@@ -12,24 +12,14 @@ export { AuthContext } from "./middleware"; · 4 unmodified lines
12 12
13type RawStore = ReturnType<typeof makeUserStore>;13type RawStore = ReturnType<typeof makeUserStore>;
14 14
15const makeService = (store: RawStore) => {15const makeService = (store: RawStore) => ({
16 const use = <A>(fn: (s: RawStore) => Promise<A>) =>16 use: <A>(fn: (s: RawStore) => Promise<A>) =>
17 Effect.tryPromise({17 withServiceLogging(
18 try: () => fn(store),18 "user_store",
19 catch: (cause) => cause,19 () => new UserStoreError(),
20 }).pipe(20 Effect.tryPromise({ try: () => fn(store), catch: (e) => e }),
21 Effect.tapError((cause) =>21 ),
22 Effect.sync(() => {22});
23 // eslint-disable-next-line no-console
24 console.error("[user_store] query failed:", cause);
25 }),
26 ),
27 Effect.mapError(() => new UserStoreError()),
28 Effect.withSpan("user_store"),
29 );
30
31 return { use };
32};
33 23
34type UserStoreServiceType = ReturnType<typeof makeService>;24type UserStoreServiceType = ReturnType<typeof makeService>;
35 25
apps/cloud/src/auth/errors.ts+24−5
@@ -1,10 +1,6 @@
1import { HttpApiSchema } from "@effect/platform";1import { HttpApiSchema } from "@effect/platform";
2import { Schema } from "effect";2import { Effect, Schema } from "effect";
3 3
4// Tagged errors returned to HTTP clients. The response payload must NEVER
5// include internal details (SQL, stack traces, WorkOS error bodies) — those
6// can leak schema + secrets. Keep the schema empty and log the `cause`
7// server-side via Effect's logger when the error is constructed.
8export class UserStoreError extends Schema.TaggedError<UserStoreError>()(4export class UserStoreError extends Schema.TaggedError<UserStoreError>()(
9 "UserStoreError",5 "UserStoreError",
10 {},6 {},
@@ -16,3 +12,26 @@ export class WorkOSError extends Schema.TaggedError<WorkOSError>()( · 5 unmodified lines
16 {},12 {},
17 HttpApiSchema.annotations({ status: 500 }),13 HttpApiSchema.annotations({ status: 500 }),
18) {}14) {}
15
16/**
17 * Service-boundary error wrapper. Logs the full Cause chain (drizzle
18 * query/params, pg error codes, nested Error.cause, etc.) via Effect's
19 * structured logger, then maps to a tagged error so the HTTP wire
20 * response contains only safe fields.
21 *
22 * Use this whenever a Promise-based API gets lifted into an Effect and
23 * its failure needs both debuggable server-side logging and a safe
24 * public shape.
25 */
26export const withServiceLogging = <A, E, R>(
27 name: string,
28 publicError: () => E,
29 effect: Effect.Effect<A, unknown, R>,
30): Effect.Effect<A, E, R> =>
31 effect.pipe(
32 Effect.tapErrorCause((cause) =>
33 Effect.logError(`${name} failed`, cause),
34 ),
35 Effect.mapError(publicError),
36 Effect.withSpan(name),
37 );
apps/cloud/src/auth/workos.ts+5−13
@@ -4,7 +4,7 @@ · 3 unmodified lines
4 4
5import { Context, Effect, Layer } from "effect";5import { Context, Effect, Layer } from "effect";
6import { WorkOS } from "@workos-inc/node/worker";6import { WorkOS } from "@workos-inc/node/worker";
7import { WorkOSError } from "./errors";7import { WorkOSError, withServiceLogging } from "./errors";
8import { server } from "../env";8import { server } from "../env";
9 9
10const COOKIE_NAME = "wos-session";10const COOKIE_NAME = "wos-session";
@@ -26,18 +26,10 @@ const make = Effect.gen(function* () { · 15 unmodified lines
26 const workos = new WorkOS({ apiKey, clientId });26 const workos = new WorkOS({ apiKey, clientId });
27 27
28 const use = <A>(fn: (wos: WorkOS) => Promise<A>) =>28 const use = <A>(fn: (wos: WorkOS) => Promise<A>) =>
29 Effect.tryPromise({29 withServiceLogging(
30 try: () => fn(workos),30 "workos",
31 catch: (cause) => cause,31 () => new WorkOSError(),
32 }).pipe(32 Effect.tryPromise({ try: () => fn(workos), catch: (e) => e }),
33 Effect.tapError((cause) =>
34 Effect.sync(() => {
35 // eslint-disable-next-line no-console
36 console.error("[workos] call failed:", cause);
37 }),
38 ),
39 Effect.mapError(() => new WorkOSError()),
40 Effect.withSpan("workos"),
41 );33 );
42 34
43 const authenticateSealedSession = (sessionData: string) =>35 const authenticateSealedSession = (sessionData: string) =>
packages/core/sdk/src/errors.ts+7−8
@@ -1,4 +1,4 @@
1import { Schema } from "effect";1import { Data, Schema } from "effect";
2 2
3import { ToolId, SecretId, PolicyId } from "./ids";3import { ToolId, SecretId, PolicyId } from "./ids";
4 4
@@ -7,14 +7,13 @@ export class ToolNotFoundError extends Schema.TaggedError<ToolNotFoundError>()( · 2 unmodified lines
7 { toolId: ToolId },7 { toolId: ToolId },
8) {}8) {}
9 9
10export class ToolInvocationError extends Schema.TaggedError<ToolInvocationError>()(10export class ToolInvocationError extends Data.TaggedError(
11 "ToolInvocationError",11 "ToolInvocationError",
12 {12)<{
13 toolId: ToolId,13 readonly toolId: ToolId;
14 message: Schema.String,14 readonly message: string;
15 cause: Schema.optional(Schema.Unknown),15 readonly cause?: unknown;
16 },16}> {}
17) {}
18 17
19export class SecretNotFoundError extends Schema.TaggedError<SecretNotFoundError>()(18export class SecretNotFoundError extends Schema.TaggedError<SecretNotFoundError>()(
20 "SecretNotFoundError",19 "SecretNotFoundError",
packages/plugins/google-discovery/src/sdk/document.ts+2−4
@@ -140,8 +140,8 @@ const DiscoveryDocumentModel = Schema.Struct({ · 139 unmodified lines
140});140});
141type DiscoveryDocument = typeof DiscoveryDocumentModel.Type;141type DiscoveryDocument = typeof DiscoveryDocumentModel.Type;
142 142
143const toParseError = (message: string, error: unknown) =>143const toParseError = (message: string, cause: unknown) =>
144 new GoogleDiscoveryParseError({ message, error });144 new GoogleDiscoveryParseError({ message, cause });
145 145
146const decodeUnknownWith = <A>(146const decodeUnknownWith = <A>(
147 message: string,147 message: string,
@@ -436,7 +436,6 @@ const manifestMethodFromMethod = (input: { · 288 unmodified lines
436 if (!method.httpMethod) {436 if (!method.httpMethod) {
437 return yield* new GoogleDiscoveryParseError({437 return yield* new GoogleDiscoveryParseError({
438 message: `Google Discovery method '${methodId}' is missing httpMethod`,438 message: `Google Discovery method '${methodId}' is missing httpMethod`,
439 error: new Error(`Missing httpMethod for ${methodId}`),
440 });439 });
441 }440 }
442 441
@@ -509,7 +508,6 @@ export const extractGoogleDiscoveryManifest = Effect.fn( · 66 unmodified lines
509 return yield* new GoogleDiscoveryParseError({508 return yield* new GoogleDiscoveryParseError({
510 message:509 message:
511 "Google Discovery document is missing one of: name, version, rootUrl",510 "Google Discovery document is missing one of: name, version, rootUrl",
512 error: new Error("Invalid document"),
513 });511 });
514 }512 }
515 513
packages/plugins/google-discovery/src/sdk/errors.ts+13−14
@@ -1,21 +1,20 @@
1import { Schema } from "effect";1import { Data, Schema } from "effect";
2import type { Option } from "effect";
2 3
3export class GoogleDiscoveryParseError extends Schema.TaggedError<GoogleDiscoveryParseError>()(4export class GoogleDiscoveryParseError extends Data.TaggedError(
4 "GoogleDiscoveryParseError",5 "GoogleDiscoveryParseError",
5 {6)<{
6 message: Schema.String,7 readonly message: string;
7 error: Schema.Defect,8 readonly cause?: unknown;
8 },9}> {}
9) {}
10 10
11export class GoogleDiscoveryInvocationError extends Schema.TaggedError<GoogleDiscoveryInvocationError>()(11export class GoogleDiscoveryInvocationError extends Data.TaggedError(
12 "GoogleDiscoveryInvocationError",12 "GoogleDiscoveryInvocationError",
13 {13)<{
14 message: Schema.String,14 readonly message: string;
15 statusCode: Schema.optionalWith(Schema.Number, { as: "Option" }),15 readonly statusCode: Option.Option<number>;
16 error: Schema.Defect,16 readonly cause?: unknown;
17 },17}> {}
18) {}
19 18
20export class GoogleDiscoveryOAuthError extends Schema.TaggedError<GoogleDiscoveryOAuthError>()(19export class GoogleDiscoveryOAuthError extends Schema.TaggedError<GoogleDiscoveryOAuthError>()(
21 "GoogleDiscoveryOAuthError",20 "GoogleDiscoveryOAuthError",
packages/plugins/google-discovery/src/sdk/invoke.ts+1−2
@@ -277,7 +277,6 @@ const invoke = Effect.fn("GoogleDiscovery.invoke")(function* (input: { · 276 unmodified lines
277 return yield* new GoogleDiscoveryInvocationError({277 return yield* new GoogleDiscoveryInvocationError({
278 message: `Missing required ${parameter.location} parameter: ${parameter.name}`,278 message: `Missing required ${parameter.location} parameter: ${parameter.name}`,
279 statusCode: Option.none(),279 statusCode: Option.none(),
280 error: undefined,
281 });280 });
282 }281 }
283 continue;282 continue;
@@ -327,7 +326,7 @@ const invoke = Effect.fn("GoogleDiscovery.invoke")(function* (input: { · 43 unmodified lines
327 new GoogleDiscoveryInvocationError({326 new GoogleDiscoveryInvocationError({
328 message: `HTTP request failed: ${err.message}`,327 message: `HTTP request failed: ${err.message}`,
329 statusCode: Option.none(),328 statusCode: Option.none(),
330 error: err,329 cause: err,
331 }),330 }),
332 ),331 ),
333 );332 );
packages/plugins/graphql/src/sdk/errors.ts+8−9
@@ -1,10 +1,10 @@
1import { Schema } from "effect";1import { Data, Schema } from "effect";
2import type { Option } from "effect";
2 3
3export class GraphqlIntrospectionError extends Schema.TaggedError<GraphqlIntrospectionError>()(4export class GraphqlIntrospectionError extends Schema.TaggedError<GraphqlIntrospectionError>()(
4 "GraphqlIntrospectionError",5 "GraphqlIntrospectionError",
5 {6 {
6 message: Schema.String,7 message: Schema.String,
7 error: Schema.Defect,
8 },8 },
9) {}9) {}
10 10
@@ -15,11 +15,10 @@ export class GraphqlExtractionError extends Schema.TaggedError<GraphqlExtraction · 4 unmodified lines
15 },15 },
16) {}16) {}
17 17
18export class GraphqlInvocationError extends Schema.TaggedError<GraphqlInvocationError>()(18export class GraphqlInvocationError extends Data.TaggedError(
19 "GraphqlInvocationError",19 "GraphqlInvocationError",
20 {20)<{
21 message: Schema.String,21 readonly message: string;
22 statusCode: Schema.optionalWith(Schema.Number, { as: "Option" }),22 readonly statusCode: Option.Option<number>;
23 error: Schema.Defect,23 readonly cause?: unknown;
24 },24}> {}
25) {}
packages/plugins/graphql/src/sdk/introspect.ts+9−11
@@ -149,29 +149,31 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( · 148 unmodified lines
149 }149 }
150 150
151 const response = yield* client.execute(request).pipe(151 const response = yield* client.execute(request).pipe(
152 Effect.tapErrorCause((cause) =>
153 Effect.logError("graphql introspection request failed", cause),
154 ),
152 Effect.mapError(155 Effect.mapError(
153 (err) =>156 (err) =>
154 new GraphqlIntrospectionError({157 new GraphqlIntrospectionError({
155 message: `Failed to reach GraphQL endpoint: ${err.message}`,158 message: `Failed to reach GraphQL endpoint: ${err.message}`,
156 error: err,
157 }),159 }),
158 ),160 ),
159 );161 );
160 162
161 if (response.status !== 200) {163 if (response.status !== 200) {
162 const body = yield* response.text.pipe(Effect.catchAll(() => Effect.succeed("")));
163 return yield* new GraphqlIntrospectionError({164 return yield* new GraphqlIntrospectionError({
164 message: `Introspection failed with status ${response.status}: ${body}`,165 message: `Introspection failed with status ${response.status}`,
165 error: undefined,
166 });166 });
167 }167 }
168 168
169 const raw = yield* response.json.pipe(169 const raw = yield* response.json.pipe(
170 Effect.tapErrorCause((cause) =>
171 Effect.logError("graphql introspection JSON parse failed", cause),
172 ),
170 Effect.mapError(173 Effect.mapError(
171 (err) =>174 () =>
172 new GraphqlIntrospectionError({175 new GraphqlIntrospectionError({
173 message: `Failed to parse introspection response as JSON`,176 message: `Failed to parse introspection response as JSON`,
174 error: err,
175 }),177 }),
176 ),178 ),
177 );179 );
@@ -180,16 +182,13 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( · 2 unmodified lines
180 182
181 if (json.errors && Array.isArray(json.errors) && json.errors.length > 0) {183 if (json.errors && Array.isArray(json.errors) && json.errors.length > 0) {
182 return yield* new GraphqlIntrospectionError({184 return yield* new GraphqlIntrospectionError({
183 // @effect-diagnostics-next-line preferSchemaOverJson:off185 message: `Introspection returned ${json.errors.length} error(s)`,
184 message: `Introspection returned errors: ${JSON.stringify(json.errors)}`,
185 error: undefined,
186 });186 });
187 }187 }
188 188
189 if (!json.data?.__schema) {189 if (!json.data?.__schema) {
190 return yield* new GraphqlIntrospectionError({190 return yield* new GraphqlIntrospectionError({
191 message: "Introspection response missing __schema",191 message: "Introspection response missing __schema",
192 error: undefined,
193 });192 });
194 }193 }
195 194
@@ -216,6 +215,5 @@ export const parseIntrospectionJson = ( · 20 unmodified lines
216 catch: (err) =>215 catch: (err) =>
217 new GraphqlIntrospectionError({216 new GraphqlIntrospectionError({
218 message: `Failed to parse introspection JSON: ${err instanceof Error ? err.message : String(err)}`,217 message: `Failed to parse introspection JSON: ${err instanceof Error ? err.message : String(err)}`,
219 error: err,
220 }),218 }),
221 });219 });
packages/plugins/graphql/src/sdk/invoke.ts+1−1
@@ -109,7 +109,7 @@ export const invoke = Effect.fn("GraphQL.invoke")(function* ( · 108 unmodified lines
109 new GraphqlInvocationError({109 new GraphqlInvocationError({
110 message: `GraphQL request failed: ${err.message}`,110 message: `GraphQL request failed: ${err.message}`,
111 statusCode: Option.none(),111 statusCode: Option.none(),
112 error: err,112 cause: err,
113 }),113 }),
114 ),114 ),
115 );115 );
packages/plugins/keychain/src/errors.ts+5−8
@@ -1,9 +1,6 @@
1import { Schema } from "effect";1import { Data } from "effect";
2 2
3export class KeychainError extends Schema.TaggedError<KeychainError>()(3export class KeychainError extends Data.TaggedError("KeychainError")<{
4 "KeychainError",4 readonly message: string;
5 {5 readonly cause?: unknown;
6 message: Schema.String,6}> {}
7 cause: Schema.optional(Schema.Unknown),
8 },
9) {}
packages/plugins/openapi/src/sdk/errors.ts+8−9
@@ -1,10 +1,10 @@
1import { Schema } from "effect";1import { Data, Schema } from "effect";
2import type { Option } from "effect";
2 3
3export class OpenApiParseError extends Schema.TaggedError<OpenApiParseError>()(4export class OpenApiParseError extends Schema.TaggedError<OpenApiParseError>()(
4 "OpenApiParseError",5 "OpenApiParseError",
5 {6 {
6 message: Schema.String,7 message: Schema.String,
7 error: Schema.Defect,
8 },8 },
9) {}9) {}
10 10
@@ -15,11 +15,10 @@ export class OpenApiExtractionError extends Schema.TaggedError<OpenApiExtraction · 4 unmodified lines
15 },15 },
16) {}16) {}
17 17
18export class OpenApiInvocationError extends Schema.TaggedError<OpenApiInvocationError>()(18export class OpenApiInvocationError extends Data.TaggedError(
19 "OpenApiInvocationError",19 "OpenApiInvocationError",
20 {20)<{
21 message: Schema.String,21 readonly message: string;
22 statusCode: Schema.optionalWith(Schema.Number, { as: "Option" }),22 readonly statusCode: Option.Option<number>;
23 error: Schema.Defect,23 readonly cause?: unknown;
24 },24}> {}
25) {}
packages/plugins/openapi/src/sdk/invoke.ts+1−3
@@ -76,7 +76,6 @@ const resolvePath = Effect.fn("OpenApi.resolvePath")(function* ( · 75 unmodified lines
76 return yield* new OpenApiInvocationError({76 return yield* new OpenApiInvocationError({
77 message: `Missing required path parameter: ${param.name}`,77 message: `Missing required path parameter: ${param.name}`,
78 statusCode: Option.none(),78 statusCode: Option.none(),
79 error: undefined,
80 });79 });
81 }80 }
82 continue;81 continue;
@@ -111,7 +110,6 @@ const resolvePath = Effect.fn("OpenApi.resolvePath")(function* ( · 28 unmodified lines
111 return yield* new OpenApiInvocationError({110 return yield* new OpenApiInvocationError({
112 message: `Unresolved path parameters: ${[...new Set(unresolved)].join(", ")}`,111 message: `Unresolved path parameters: ${[...new Set(unresolved)].join(", ")}`,
113 statusCode: Option.none(),112 statusCode: Option.none(),
114 error: undefined,
115 });113 });
116 }114 }
117 115
@@ -245,7 +243,7 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( · 127 unmodified lines
245 new OpenApiInvocationError({243 new OpenApiInvocationError({
246 message: `HTTP request failed: ${err.message}`,244 message: `HTTP request failed: ${err.message}`,
247 statusCode: Option.none(),245 statusCode: Option.none(),
248 error: err,246 cause: err,
249 }),247 }),
250 ),248 ),
251 );249 );
packages/plugins/openapi/src/sdk/parse.ts+0−1
@@ -20,7 +20,6 @@ export const parse = Effect.fn("OpenApi.parse")(function* (input: string) { · 19 unmodified lines
20 catch: (error) =>20 catch: (error) =>
21 new OpenApiParseError({21 new OpenApiParseError({
22 message: `Failed to parse OpenAPI document: ${error instanceof Error ? error.message : String(error)}`,22 message: `Failed to parse OpenAPI document: ${error instanceof Error ? error.message : String(error)}`,
23 error,
24 }),23 }),
25 });24 });
26 25