Handle usage API business errors - #31
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughHTTP 200 GLM responses are now validated for business-level failures. Usage requests preserve authentication, server, parse, and cancellation errors, apply primary-versus-supplementary failure handling, and retain business codes and server messages in diagnostics. ChangesUsage Error Reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant UsageClient
participant readBusinessJson
participant ErrorMapping
participant logger
UsageClient->>readBusinessJson: Request usage endpoint
readBusinessJson->>ErrorMapping: Classify HTTP or business response
ErrorMapping-->>UsageClient: Return snapshot or mapped error state
UsageClient->>logger: Log supplementary request failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/response.ts`:
- Around line 56-63: Update the response parsing around errorValue, errorObject,
code, and message so nested error.code and errorObject.message take precedence
whenever error is present, while retaining top-level values as fallbacks. Add a
regression test covering a mixed envelope with top-level code 200 and nested
invalid-token error details, verifying the nested code and message drive
failure/auth classification.
In `@src/client/usage.ts`:
- Around line 145-163: Update the hasAccountData calculation in the usage
aggregation flow to treat every account-report field, including spent, gifted,
and frozen balances, as usable primary data. Ensure a rejected package request
degrades to the account snapshot rather than returning an error when any one of
these fields is present. Add coverage for each previously omitted field with a
failed package request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4dcd0695-e258-48f3-abaa-0d1117a0dd52
📒 Files selected for processing (6)
CHANGELOG.mdsrc/client/errors.tssrc/client/response.test.tssrc/client/response.tssrc/client/usage.test.tssrc/client/usage.ts
|
@codex @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
auth-errorRoot cause
The usage clients treated HTTP status as the only success signal. GLM endpoints can return HTTP 200 with a failed business envelope, causing authentication and server failures to be interpreted as empty successful data.
User impact
Usage status now reports authentication, network, and server failures accurately instead of incorrectly displaying
no-data. Authentication failures also expose the existing API-key recovery action.Validation
pnpm test— 108 tests passedpnpm typecheckpnpm lintgit diff --checkCloses #29
Summary by CodeRabbit