fix(auth): only treat 401/403 as an invalid token on login (CLI-19)#1153
Merged
Conversation
`sentry auth login --token` validated the token via getUserRegions() and
wrapped ANY failure in a blind `catch {}` that cleared the token and threw
"Invalid API token". So a transient network blip or a 5xx server error was
reported to the user as a bad token (misleading), discarded the real cause
(no diagnostics — a silent-catch anti-pattern), and wiped a possibly-valid
token. This made CLI-19 a catch-all bucket (~90 users).
- Discriminate the failure: only a 401/403 ApiError means the supplied
token is invalid/insufficient — clear it and raise AuthError("invalid").
Any other error keeps the token and is re-thrown unchanged, so the user
sees the real failure and it is classified accurately.
- Now that "invalid" is precise (a genuine bad-token user error, like
not_authenticated/expired), silence all AuthError reasons in
classifySilenced instead of just two — these are expected auth states,
not CLI bugs. Volume stays visible via the cli.error.silenced metric.
Contributor
|
Contributor
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 5132 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.47% 81.47% —%
==========================================
Files 397 397 —
Lines 27699 27702 +3
Branches 17989 17991 +2
==========================================
+ Hits 22568 22570 +2
- Misses 5131 5132 +1
- Partials 1861 1862 +1Generated by Codecov Action |
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
sentry auth login --token <t>saves the token and validates it viagetUserRegions(). The validation was wrapped in a blindcatch {}that, onany failure, cleared the token and threw
AuthError("invalid", "Invalid API token…").So a transient network blip, a 5xx server error, or a parse failure was:
AGENTS.md), andThis made CLI-19 a catch-all
bucket (~90 users).
Fix
handleTokenValidationError): only a 401/403ApiErrormeans the supplied token is invalid/insufficient — clear it andraise
AuthError("invalid"). Any other error keeps the token and is re-thrownunchanged, so the user sees the real failure and it's classified accurately
(network → silenced per CLI-16W; 5xx → captured as a real server issue).
AuthErrorreasons inclassifySilencedinstead of justnot_authenticated/expired. Now thatinvalidis precise (a genuinebad-token user error), all three are expected auth states the user must act
on — not CLI bugs. Volume stays visible via the
cli.error.silencedmetric.Extracted the discrimination into a helper to keep
funcunder the cognitivecomplexity limit.
Test plan
login: 401 → clears auth + throwsAuthError; non-401 (503) → keeps tokenand re-throws the original cause (new test).
classifySilenced/reportCliError:AuthError("invalid")→ silenced(
auth_expected,auth_reason: invalid) instead of captured.vitest run test/lib/{errors,error-reporting,telemetry}.test.ts test/commands/auth/→ 326 passed;
biome checkclean.Fixes CLI-19