fix: surface OAuth error_description on token-exchange 4xx#469
Draft
posthog[bot] wants to merge 1 commit into
Draft
fix: surface OAuth error_description on token-exchange 4xx#469posthog[bot] wants to merge 1 commit into
posthog[bot] wants to merge 1 commit into
Conversation
The token endpoint at `${POSTHOG_OAUTH_URL}/oauth/token` returns standard
OAuth `error` / `error_description` fields on a 4xx, but
`exchangeCodeForToken` only ever consumed `response.data` on success. A 400
(e.g. replayed or expired code, PKCE mismatch, redirect_uri mismatch after
the multi-port refactor in #400) surfaced as the opaque "Request failed with
status code 400" on the very first step of the wizard, leaving users without
a path to recovery.
This catches axios errors in `exchangeCodeForToken`, parses the OAuth error
payload, and rethrows with a clear message that gets rendered by the
existing `performOAuthFlow` catch (e.g. "invalid_grant: authorization code
expired - please re-run the wizard"). Also logs the resolved `redirect_uri`
+ port + server response via `logToFile` on the chosen port and on failure,
so future 400s can be traced to a specific port iteration.
Mirrors the surfacing pattern from #432.
Generated-By: PostHog Code
Task-Id: 311205bc-0f27-42f4-a2e6-0112a4d64855
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
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.
Problem
A new error tracking issue (fingerprint
b8d1191658…) shows users hittingAxiosError: Request failed with status code 400from the PostHog/oauth/tokenendpoint duringnpx @posthog/wizard. The opaque message gives them no path to recovery on the very first step of the wizard.exchangeCodeForToken(src/utils/oauth.ts) posts the PKCE code exchange and only consumesresponse.dataon success — it never reads the OAuth-standarderror/error_descriptionfromresponse.dataon a 4xx. The catch inperformOAuthFlowthen logserror.message, which for axios 400s is just the generic status string.Likely server-side triggers: replayed/expired authorization code, PKCE
code_verifiermismatch, or aredirect_urimismatch (more plausible now after the backup-ports refactor in #400 pickscallbackUrldynamically per port). The recent CI-mode 401 fix (#432) added similar handling but only for 401s from the LLM gateway, not 400s from/oauth/token.Changes
exchangeCodeForTokennow wraps the axios call in try/catch. On any 4xx, it parsesresponse.datawith a smallOAuthErrorResponseSchema(zod), extractserror+error_description, and rethrows anErrorwhose message reads e.g.invalid_grant: authorization code expired — please re-run the wizard to start a fresh login. The existing catch inperformOAuthFlowthen renders that viagetUI().log.error(...)without further changes.redirect_uri, status, and parsed OAuth fields are written vialogToFileso future 400s can be traced to a specific port iteration.[oauth] attempting callback server on port Nlog line now also includes the chosenredirect_uri, making port-vs-callback-URL drift visible in the log.Mirrors the surfacing pattern from #432 but narrowly scoped to the OAuth token endpoint —
AuthErrorScreenis left alone since its docstring explicitly scopes it to LLM Gateway 401s.Test plan
pnpm build— succeeds, smoke test passespnpm test—provision-cli.test.tsflakes in the full parallel run (pre-existing, passes in isolation onmain); all other 39 suites pass (646/647)pnpm lint— 0 errors (pre-existing warnings unchanged)/oauth/tokenis hard without server cooperation; covered by code review + the unchanged success path which existing tests exerciseCreated with PostHog Code