From b252821c59d399bc067b60f669c242669fa3012b Mon Sep 17 00:00:00 2001 From: wucm667 Date: Sat, 19 Sep 2026 17:22:39 +0800 Subject: [PATCH] fix(accounts): preserve refresh errors during usage queries --- .../internal/service/account_usage_service.go | 8 +- .../account_usage_service_batch_test.go | 81 +++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/backend/internal/service/account_usage_service.go b/backend/internal/service/account_usage_service.go index f49d3aaa71..3347aa328d 100644 --- a/backend/internal/service/account_usage_service.go +++ b/backend/internal/service/account_usage_service.go @@ -359,11 +359,9 @@ func (s *AccountUsageService) getUsageForAccount(ctx context.Context, account *A } if account.Platform == PlatformOpenAI && account.Type == AccountTypeOAuth { - usage, err := s.getOpenAIUsage(ctx, account, forceProbe) - if err == nil { - s.tryClearRecoverableAccountError(ctx, account) - } - return usage, err + // Usage can come from a stored snapshot even when a probe fails. Neither + // that nor a working access token proves a rejected refresh token recovered. + return s.getOpenAIUsage(ctx, account, forceProbe) } if account.Platform == PlatformGemini { diff --git a/backend/internal/service/account_usage_service_batch_test.go b/backend/internal/service/account_usage_service_batch_test.go index 067bf0ddd1..b4eb32ed27 100644 --- a/backend/internal/service/account_usage_service_batch_test.go +++ b/backend/internal/service/account_usage_service_batch_test.go @@ -189,3 +189,84 @@ func TestAccountUsageService_GetUsageBatch_BestEffortByAccount(t *testing.T) { t.Fatalf("expected API key account error to be preserved, got %q", errorsByAccount[7003]) } } + +// Model ClearError's persisted effect so this regression catches both the write +// and mutations of the account returned by the repository. +type usageErrorAccountRepo struct { + stubOpenAIAccountRepo + clearCalls int +} + +func (r *usageErrorAccountRepo) ClearError(ctx context.Context, id int64) error { + r.clearCalls++ + account, err := r.GetByID(ctx, id) + if err != nil { + return err + } + account.Status = StatusActive + account.ErrorMessage = "" + return nil +} + +func TestAccountUsageService_OpenAIQueriesPreserveRefreshError(t *testing.T) { + for _, scenario := range []string{"cached_expired_access", "cached_valid_access_invalid_refresh", "failed_probe_missing_access", "probe_throttled"} { + for _, query := range []string{"usage", "usage_batch", "forced_usage_batch", "today", "today_batch"} { + t.Run(scenario+"/"+query, func(t *testing.T) { + const message = "Token refresh failed (non-retryable): refresh_token_invalidated" + account := Account{ + ID: 7358, Platform: PlatformOpenAI, Type: AccountTypeOAuth, + Status: StatusError, ErrorMessage: message, + Credentials: map[string]any{"refresh_token": "invalid-refresh-token"}, + } + cache := NewUsageCache() + if strings.HasPrefix(scenario, "cached_") { + account.Credentials["access_token"] = "cached-access-token" + expiresAt := time.Now().Add(-time.Hour) + if scenario == "cached_valid_access_invalid_refresh" { + expiresAt = time.Now().Add(time.Hour) + } + account.Credentials["expires_at"] = expiresAt.Format(time.RFC3339) + account.Extra = map[string]any{"codex_5h_used_percent": 18.0, "codex_7d_used_percent": 34.0} + } + if scenario == "probe_throttled" { + cache.openAIProbeCache.Store(account.ID, time.Now()) + } + // Forced requests take the probe path, but fail locally without + // credentials; no external network is needed for this regression. + if query == "forced_usage_batch" { + delete(account.Credentials, "access_token") + } + repo := &usageErrorAccountRepo{stubOpenAIAccountRepo: stubOpenAIAccountRepo{accounts: []Account{account}}} + svc := &AccountUsageService{accountRepo: repo, usageLogRepo: &usageBatchLogRepoStub{}, cache: cache} + ctx := context.Background() + switch query { + case "usage": + usage, err := svc.GetUsage(ctx, account.ID) + if err != nil || usage == nil { + t.Fatalf("GetUsage: usage=%v err=%v", usage, err) + } + case "usage_batch", "forced_usage_batch": + usage, failures, err := svc.GetUsageBatch(ctx, []int64{account.ID}, query == "forced_usage_batch") + if err != nil || len(failures) != 0 || usage[account.ID] == nil { + t.Fatalf("GetUsageBatch: usage=%v failures=%v err=%v", usage, failures, err) + } + case "today": + if _, err := svc.GetTodayStats(ctx, account.ID); err != nil { + t.Fatal(err) + } + case "today_batch": + if _, err := svc.GetTodayStatsBatch(ctx, []int64{account.ID}); err != nil { + t.Fatal(err) + } + } + stored, err := repo.GetByID(ctx, account.ID) + if err != nil { + t.Fatal(err) + } + if repo.clearCalls != 0 || stored.Status != StatusError || stored.ErrorMessage != message { + t.Fatalf("query erased refresh error: clearCalls=%d status=%q error=%q", repo.clearCalls, stored.Status, stored.ErrorMessage) + } + }) + } + } +}