fix(oauth): surface server error detail from finishOAuthLogin#83
Merged
Conversation
Contributor
Author
|
Filed the API-side follow-up: fells-code/seamless-auth-api#91. That ticket covers returning a stable machine-readable Once that lands, consumers can read |
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
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.
What
finishOAuthLogin()now throws aSeamlessAuthErrorcarrying the auth server's message, HTTPstatus, and parsedbody, instead of discarding the response and throwing a genericError('Failed to finish OAuth login').SeamlessAuthErroris exported fromsrc/index.tsso callers can narrow withinstanceofand readstatusandbody.Closes #51.
Important: this does not fully deliver the issue's motivating case
The issue's motivation is the "provider did not return an email address" failure. That specific reason is not currently exposed by the API, so this change alone will not make it actionable.
In
seamless-auth-api, the OAuth callback'scatchblock logs the reason server-side (metadata: { reason: 'callback_failed' }) and responds with a generic{ error: 'OAuth login failed' }. There is no distinct payload or code for the missing-email case, so that is the most detail the SDK can surface today.What this PR does deliver: every reason the API does distinguish now reaches the caller, for example
OAuth signup is disabled(403),Invalid OAuth state(400), andOAuth provider not found(404).Making the missing-email case actionable needs a follow-up in
seamless-auth-apito return a distinct error (ideally with a stable machine-readable code). Worth filing there.Correction to the issue's proposed fix
The description proposes reading
res.error?.messageandres.error?.codefrom the server-side@seamless-auth/coreresult shape. Two mismatches with this repo:Response, not{ ok, error, status }, so the body has to be read explicitly and defensively (it may be empty or non-JSON).{ error: string }. There is nomessagefield and nocodefield, so acodecannot be populated yet.messageis still accepted as a fallback shape in case other endpoints differ.Relationship to #24
This defines an error convention, which is #24's job. I kept the surface deliberately small (one error class, no result-object refactor) so #24 can supersede or extend it without a second migration. Noted on both issues.
Tests
tests/errors.test.ts: message extraction fromerror, fallback tomessage, fallback when the body is not JSON, fallback on an empty message, andinstanceof Errorwith a stablename.tests/authProvider.test.tsx: provider-level cases asserting the server detail reaches the caller and that a non-JSON body falls back cleanly.Confirmed the provider tests actually catch the regression: with the fix reverted they fail with the old
[Error: Failed to finish OAuth login].errors.tsis at 100% coverage.Checks
npm run typecheck,npm run lint,npm run format:checkclean