refactor(core,express)!: split result error into errorCode and errorBody - #124
Merged
Conversation
The handler result `error` field meant two different things. On 12 sites it
held the auth API's whole failure body, forwarded to the caller unchanged.
On 8 sites it held a short code the adapter wrapped as `{ error }`. The
declared types could not describe either honestly, and FinishLoginResult
declared `error?: string` while assigning the whole body. Nothing in the
type told an adapter which rendering applied, which is the first thing a
second adapter has to get right.
Failures now go through ResultFailure, exported from core: `errorCode` for a
code this package chose, which adapters render as `{ error, details }`, and
`errorBody` for the auth API's own body, which adapters forward untouched.
Callers read fields off that body directly, so it must not be reshaped.
The HTTP responses do not change. Failure responses were captured on both
revisions across 6 upstream body shapes and 3 route kinds, and all 18 are
byte-identical. They were also run through extractMessage and
getOAuthErrorCode from seamless-auth-react with identical results.
A new failureWireFormat test locks these shapes so later work on this epic
cannot move them silently.
BREAKING: code reading `result.error` off @seamless-auth/core/handlers/*
reads `errorBody` or `errorCode` instead, and the OAuth union check
`"error" in result` becomes `"errorBody" in result`.
Refs #72
This was referenced Jul 29, 2026
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.
Step 0 of #72. Unblocks the
applyResultandproxyRequestwork, which cannot be written until a failure has one meaning.Problem
result.errormeant two different things depending on which handler produced it:res.json(result.error)res.json({ error: result.error })FinishLoginResultdeclarederror?: stringwhile assigning the whole parsed body. Harmless today only because Express happens to render that one with the raw call.Nothing in the type says which rendering applies, and that is exactly the trap a second adapter walks into: read the type, see
error?: string, wrap it as{ error }, and silently change the response shape for every auth failure on 12 routes.Change
Failures now go through
ResultFailure, exported from@seamless-auth/core:errorCode?: stringis a code this package chose. Adapters render it as{ error, details }.errorBody?: unknownis the auth API's own failure body. Adapters forward it unchanged.The Express
errorBody()helper is renamedfailureResponse()so it no longer collides with the new field name.Why the body cannot simply be normalized
Two tidier-looking options were tested against the real SDK and rejected on evidence.
Wrapping everything as
{ error: <body> }breaks every error message:extractMessageinseamless-auth-reactrequireserrorto be a string, so an object falls through to the generic fallback. All 7 real API bodies regressed.Normalizing through the existing
readUpstreamFailurelooked perfect, and was message-identical on all 7 bodies, but it moves sibling keys underdetails:The API's OAuth failure schema is
{ error, message?, code? }, and the SDK reads that top-levelcodeto drive user-actionable OAuth messaging. HenceerrorBodyis forwarded verbatim, and the reason is recorded in the type's doc comment so it does not get "cleaned up" later.The HTTP responses do not change
Verified, not assumed. The adapter's failure responses were captured on
mainand on this branch across 6 upstream body shapes (plain code, code with a sibling field, OAuth code, Zod validation body, message-only rate limit, empty body) and 3 route kinds (passthrough, the OAuth union, proxy). All 18 are byte-identical, and all 18 produce identical results throughextractMessageandgetOAuthErrorCodefromseamless-auth-react.All 131 pre-existing Express tests, which assert real HTTP responses through supertest, passed without modification.
No adopter application, SDK, or dashboard needs to change.
New regression test
packages/express/tests/failureWireFormat.test.jslocks these shapes, since PRs 2 and 3 of this epic will touch rendering again. It is not vacuous: reintroducing the old wrapping bug on a single route trips 7 of its assertions.Breaking change
For code reading
result.erroroff a handler imported from@seamless-auth/core/handlers/*. ReaderrorBodyfor the auth-flow handlers anderrorCodefor the rest; the OAuth union check"error" in resultbecomes"errorBody" in result. Major changeset included, with the same guidance.Checks
pnpm buildclean.pnpm testpasses: 44 suites, 216 tests (up 11).Note for the epic
Two things surfaced that are out of scope here and worth their own issues:
getOAuthErrorCodedepending on a top-level sibling key is what forceserrorBodyto stay verbatim. Teaching the SDK to readcodefrom either location would free future cleanup here.