fix: address review follow-ups on API key sign-in error handling

src/components/dialog/content/signin/ApiKeyForm.vue

@@ -122,8 +122,7 @@ const emit = defineEmits<{
 
 const onSubmit = async (event: FormSubmitEvent) => {
   if (!event.valid) return
-  // storeApiKey resolves falsy when the key was not accepted, in which case it
-  // has already reported why and the dialog must stay open.
+  // A rejected key has already been reported; the dialog stays open for a retry.
   if (await apiKeyStore.storeApiKey(event.values.apiKey)) {
     emit('success')
   }

src/locales/en/main.json

@@ -2482,7 +2482,7 @@
       "invalid": "Invalid API Key",
       "invalidDetail": "Please enter a valid API Key",
       "verificationUnavailable": "Couldn't verify your API Key",
-      "verificationUnavailableDetail": "The Comfy API couldn't be reached. Your key has been saved — please try again in a moment.",
+      "verificationUnavailableDetail": "Your key has been saved — please try again in a moment.",
       "notPermitted": "API Key not permitted",
       "notPermittedDetail": "Your account isn't allowed to sign in with this API Key. Check your plan, or contact support.",
       "helpText": "Need an API key?",

src/stores/apiKeyAuthStore.test.ts

@@ -206,21 +206,6 @@ describe('useApiKeyAuthStore', () => {
       })
     })
 
-    it('still discards the key and tells the user when reporting throws', async () => {
-      mockTrackAuthFailed.mockImplementation(() => {
-        throw new Error('telemetry is down')
-      })
-      mockCreateCustomer.mockRejectedValue(new AuthStoreError('rejected', 401))
-      const store = useApiKeyAuthStore()
-
-      await expect(store.storeApiKey(VALID_KEY)).resolves.toBeFalsy()
-      await nextTick()
-
-      expect(store.getApiKey()).toBeNull()
-      expect(store.isAuthenticated).toBe(false)
-      expect(severities()).toEqual(['error:auth.apiKey.invalid'])
-    })
-
     it('reports nothing when the key is accepted', async () => {
       mockCreateCustomer.mockResolvedValue(customer)
       const store = useApiKeyAuthStore()
@@ -317,4 +302,53 @@ describe('useApiKeyAuthStore', () => {
       expect(severities()).toEqual([])
     })
   })
+
+  describe('a key the account is not permitted to use', () => {
+    // The key is kept, so every launch re-validates it and would re-toast.
+    const launch = async () => {
+      setActivePinia(createTestingPinia({ stubActions: false }))
+      mockCreateCustomer.mockClear()
+      const store = useApiKeyAuthStore()
+      await vi.waitFor(() => expect(mockCreateCustomer).toHaveBeenCalled())
+      await nextTick()
+      return store
+    }
+
+    beforeEach(() => {
+      localStorage.setItem(STORAGE_KEY, VALID_KEY)
+      mockCreateCustomer.mockRejectedValue(new AuthStoreError('denied', 403))
+    })
+
+    it('is reported on the launch that finds it, and not again after', async () => {
+      await launch()
+      expect(severities()).toEqual(['error:auth.apiKey.notPermitted'])
+
+      const relaunched = await launch()
+
+      expect(relaunched.getApiKey()).toBe(VALID_KEY)
+      expect(severities()).toEqual([])
+    })
+
+    it('is reported again for a different key', async () => {
+      await launch()
+
+      localStorage.setItem(STORAGE_KEY, 'comfyui-other-denied-key')
+      await launch()
+
+      expect(severities()).toEqual(['error:auth.apiKey.notPermitted'])
+    })
+
+    it('is reported again once the account regains permission and loses it', async () => {
+      await launch()
+
+      mockCreateCustomer.mockResolvedValue(customer)
+      await launch()
+      expect(severities()).toEqual([])
+
+      mockCreateCustomer.mockRejectedValue(new AuthStoreError('denied', 403))
+      await launch()
+
+      expect(severities()).toEqual(['error:auth.apiKey.notPermitted'])
+    })
+  })
 })

src/stores/apiKeyAuthStore.ts

@@ -14,6 +14,7 @@ type ComfyApiUser =
   operations['createCustomer']['responses']['201']['content']['application/json']
 
 const STORAGE_KEY = 'comfy_api_key'
+const DENIAL_REPORTED_KEY = 'comfy_api_key_denial_reported'
 
 /** Only a rejected key is discarded; the other two are kept and can be retried. */
 type ApiKeyFailure = 'rejected' | 'denied' | 'unverified'
@@ -41,31 +42,17 @@ const failureFor = (error: unknown): ApiKeyFailure => {
   return 'unverified'
 }
 
-/**
- * Pairs the failure kind with the status that produced it so the cases stay
- * separable when reported: a rise in `unverified_503` is the backend declining
- * to vouch for keys, a different incident from a rise in `rejected_401`.
- */
+/** `unverified_503` and `rejected_401` are separate incidents, not one rate. */
 const errorCodeFor = (error: unknown, failure: ApiKeyFailure) =>
   error instanceof AuthStoreError && error.status
     ? `${failure}_${error.status}`
     : failure
 
-/**
- * Reports every failure, including the unverified one the watch only logs, so
- * the case that made BE-7550 invisible is the case this can be alerted on.
- * Isolated because the dispatcher is a bare interface with no no-throw
- * guarantee, and a reporting fault must never decide what happens to the key.
- */
 const reportFailure = (error: unknown, failure: ApiKeyFailure) => {
-  try {
-    useTelemetry()?.trackAuthFailed({
-      error_code: errorCodeFor(error, failure),
-      auth_action: 'api_key_sign_in'
-    })
-  } catch (reportingError) {
-    console.error('Failed to report API key sign-in failure', reportingError)
-  }
+  useTelemetry()?.trackAuthFailed({
+    error_code: errorCodeFor(error, failure),
+    auth_action: 'api_key_sign_in'
+  })
 }
 
 const FAILURE_MESSAGES: Record<
@@ -89,6 +76,12 @@ const FAILURE_MESSAGES: Record<
 export const useApiKeyAuthStore = defineStore('apiKeyAuth', () => {
   const authStore = useAuthStore()
   const apiKey = useLocalStorage<string | null>(STORAGE_KEY, null)
+  // A denied key is kept, so without a record of having already said so the
+  // same error toast greets the user on every launch.
+  const reportedDenialFor = useLocalStorage<string | null>(
+    DENIAL_REPORTED_KEY,
+    null
+  )
   const toastStore = useToastStore()
   const { wrapWithErrorHandlingAsync, toastErrorHandler } = useErrorHandling()
 
@@ -116,6 +109,7 @@ export const useApiKeyAuthStore = defineStore('apiKeyAuth', () => {
       const user = await authStore.createCustomer()
       if (!stillWanted()) return false
       currentUser.value = user
+      reportedDenialFor.value = null
       return true
     } catch (error) {
       if (!stillWanted()) return false
@@ -146,31 +140,34 @@ export const useApiKeyAuthStore = defineStore('apiKeyAuth', () => {
     }
   }
 
-  // True while storeApiKey is driving the check itself. It keeps the write it
-  // makes to `apiKey` from having the watch below repeat the same POST and
-  // report the same failure twice, and it lets the form disable submission for
-  // the duration rather than letting a second attempt race the first.
+  // Set only while storeApiKey drives the check itself: it stops the watch from
+  // repeating the same POST, and lets the form disable submit for the duration.
   const isValidating = ref(false)
 
   watch(
     apiKey,
     () => {
       if (!apiKey.value) {
         currentUser.value = null
+        reportedDenialFor.value = null
         return
       }
       if (isValidating.value) return
       // A stored key the backend rejects or refuses is the user's problem to
       // fix; a backend that is merely unreachable is not worth a startup toast.
       void resolveUser().catch((error: unknown) => {
         if (
-          error instanceof ApiKeyAuthError &&
-          error.failure !== 'unverified'
+          !(error instanceof ApiKeyAuthError) ||
+          error.failure === 'unverified'
         ) {
-          reportError(error)
-        } else {
           console.error(error)
+          return
+        }
+        if (error.failure === 'denied') {
+          if (reportedDenialFor.value === apiKey.value) return
+          reportedDenialFor.value = apiKey.value
         }
+        reportError(error)
       })
     },
     { immediate: true }