Tighten HTTP error schemas, keep rich causes internally - #137
Merged
RhysSullivan merged 4 commits intoApr 9, 2026
Merged
Conversation
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
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | 07efb47 | Commit Preview URL | Apr 09 2026, 09:52 PM |
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.
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).
RhysSullivan
added a commit
that referenced
this pull request
May 31, 2026
* 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).
RhysSullivan
deleted the
04-09-errors_sweep_strip_internal_details_from_http_errors
branch
June 11, 2026 08:00
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Establish a clear rule for tagged errors in the codebase:
Schema.TaggedError= HTTP wire-facing. Every field is a safe public field. Nocause, noSchema.Unknown, noSchema.Defect.Data.TaggedError= internal-only. Can carry richcause: unknownfor logging/debugging. Cannot be accidentally wired to an HttpApi endpoint because it isn't a Schema — the compiler rejects it.Actual leaks fixed (HTTP-exposed errors with unsafe fields)
OpenApiParseError— wired toPOST /openapi/previewandPOST /openapi/specs. Droppederror: Schema.Defect.GraphqlIntrospectionError— wired toPOST /graphql/sources. Droppederror: Schema.Defect. Underlying causes are now logged viaEffect.logErrorat each construction site inintrospect.ts, so debugging info stays in server logs.Latent footguns closed (internal errors that had unsafe schema fields)
Converted
Schema.TaggedError→Data.TaggedErrorand renamederror→cause:ToolInvocationError(packages/core/sdk)OpenApiInvocationErrorGraphqlInvocationErrorGoogleDiscoveryParseErrorGoogleDiscoveryInvocationErrorKeychainErrorThese errors still flow through Effect error channels exactly as before —
.catchTag,yield* new …, etc. all work identically. The only difference is they're no longer Schemas, soHttpApiEndpoint.addError(…)won't compile with them. If one ever needs to be exposed over HTTP in the future, whoever adds it is forced to think about the wire format at that moment.Cloud auth (unchanged, already clean)
UserStoreError/WorkOSErrorkeep their empty schemas +status: 500annotation. TheEffect.tapErrorCause + Effect.logErrorpattern in the service wrappers (context.ts,workos.ts) ensures the full drizzle/workos cause chain lands in server logs while the HTTP response body stays{"_tag":"UserStoreError"}.Test plan
bun run typecheck— all touched packages clean; remaining workspace errors are pre-existing on main (theExecutionResult/PausedExecutionmismatch inpackages/core/api/src/handlers/executions.tsplus a couple unused-import warnings)bun run test— 89 tests across 13 files, all passing:@executor/sdk23/23@executor/plugin-openapi27/27@executor/plugin-graphql9/9@executor/plugin-google-discovery5/5@executor/plugin-mcp18/18@executor/plugin-keychain3/3@executor/cloud4/4