From a15b84e2a1f1c6ae750127a3bdaf5d08b3acaa43 Mon Sep 17 00:00:00 2001 From: Mingholy Date: Thu, 20 Nov 2025 14:37:39 +0800 Subject: [PATCH] refactor(auth): enhance useAuthCommand to include history management and improve error handling in QwenOAuth2Client (#1077) --- packages/cli/src/ui/AppContainer.tsx | 2 +- packages/cli/src/ui/auth/useAuth.ts | 28 +++++++--- packages/core/src/qwen/qwenOAuth2.test.ts | 30 ++++++----- packages/core/src/qwen/qwenOAuth2.ts | 65 ++++++++++++----------- 4 files changed, 74 insertions(+), 51 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index ecacbda4..345bebd2 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -353,7 +353,7 @@ export const AppContainer = (props: AppContainerProps) => { handleAuthSelect, openAuthDialog, cancelAuthentication, - } = useAuthCommand(settings, config); + } = useAuthCommand(settings, config, historyManager.addItem); const { proQuotaRequest, handleProQuotaChoice } = useQuotaAndFallback({ config, diff --git a/packages/cli/src/ui/auth/useAuth.ts b/packages/cli/src/ui/auth/useAuth.ts index 9b1198bf..04da911c 100644 --- a/packages/cli/src/ui/auth/useAuth.ts +++ b/packages/cli/src/ui/auth/useAuth.ts @@ -4,23 +4,28 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useCallback, useEffect } from 'react'; -import type { LoadedSettings, SettingScope } from '../../config/settings.js'; import type { Config } from '@qwen-code/qwen-code-core'; import { + AuthEvent, AuthType, clearCachedCredentialFile, getErrorMessage, logAuth, - AuthEvent, } from '@qwen-code/qwen-code-core'; -import { AuthState } from '../types.js'; -import { useQwenAuth } from '../hooks/useQwenAuth.js'; +import { useCallback, useEffect, useState } from 'react'; +import type { LoadedSettings, SettingScope } from '../../config/settings.js'; import type { OpenAICredentials } from '../components/OpenAIKeyPrompt.js'; +import { useQwenAuth } from '../hooks/useQwenAuth.js'; +import { AuthState, MessageType } from '../types.js'; +import type { HistoryItem } from '../types.js'; export type { QwenAuthState } from '../hooks/useQwenAuth.js'; -export const useAuthCommand = (settings: LoadedSettings, config: Config) => { +export const useAuthCommand = ( + settings: LoadedSettings, + config: Config, + addItem: (item: Omit, timestamp: number) => void, +) => { const unAuthenticated = settings.merged.security?.auth?.selectedType === undefined; @@ -117,8 +122,17 @@ export const useAuthCommand = (settings: LoadedSettings, config: Config) => { // Log authentication success const authEvent = new AuthEvent(authType, 'manual', 'success'); logAuth(config, authEvent); + + // Show success message + addItem( + { + type: MessageType.INFO, + text: `Authenticated successfully with ${authType} credentials.`, + }, + Date.now(), + ); }, - [settings, handleAuthFailure, config], + [settings, handleAuthFailure, config, addItem], ); const performAuth = useCallback( diff --git a/packages/core/src/qwen/qwenOAuth2.test.ts b/packages/core/src/qwen/qwenOAuth2.test.ts index 2e8bf83e..23c26296 100644 --- a/packages/core/src/qwen/qwenOAuth2.test.ts +++ b/packages/core/src/qwen/qwenOAuth2.test.ts @@ -623,14 +623,16 @@ describe('QwenOAuth2Client', () => { }); it('should handle authorization_pending with HTTP 400 according to RFC 8628', async () => { + const errorData = { + error: 'authorization_pending', + error_description: 'The authorization request is still pending', + }; const mockResponse = { ok: false, status: 400, statusText: 'Bad Request', - json: async () => ({ - error: 'authorization_pending', - error_description: 'The authorization request is still pending', - }), + text: async () => JSON.stringify(errorData), + json: async () => errorData, }; vi.mocked(global.fetch).mockResolvedValue(mockResponse as Response); @@ -646,14 +648,16 @@ describe('QwenOAuth2Client', () => { }); it('should handle slow_down with HTTP 429 according to RFC 8628', async () => { + const errorData = { + error: 'slow_down', + error_description: 'The client is polling too frequently', + }; const mockResponse = { ok: false, status: 429, statusText: 'Too Many Requests', - json: async () => ({ - error: 'slow_down', - error_description: 'The client is polling too frequently', - }), + text: async () => JSON.stringify(errorData), + json: async () => errorData, }; vi.mocked(global.fetch).mockResolvedValue(mockResponse as Response); @@ -1993,14 +1997,16 @@ describe('Enhanced Error Handling and Edge Cases', () => { }); it('should handle authorization_pending with correct status', async () => { + const errorData = { + error: 'authorization_pending', + error_description: 'Authorization request is pending', + }; const mockResponse = { ok: false, status: 400, statusText: 'Bad Request', - json: vi.fn().mockResolvedValue({ - error: 'authorization_pending', - error_description: 'Authorization request is pending', - }), + text: vi.fn().mockResolvedValue(JSON.stringify(errorData)), + json: vi.fn().mockResolvedValue(errorData), }; vi.mocked(global.fetch).mockResolvedValue( diff --git a/packages/core/src/qwen/qwenOAuth2.ts b/packages/core/src/qwen/qwenOAuth2.ts index b9a35bff..c4cfa933 100644 --- a/packages/core/src/qwen/qwenOAuth2.ts +++ b/packages/core/src/qwen/qwenOAuth2.ts @@ -345,44 +345,47 @@ export class QwenOAuth2Client implements IQwenOAuth2Client { }); if (!response.ok) { - // Parse the response as JSON to check for OAuth RFC 8628 standard errors + // Read response body as text first (can only be read once) + const responseText = await response.text(); + + // Try to parse as JSON to check for OAuth RFC 8628 standard errors + let errorData: ErrorData | null = null; try { - const errorData = (await response.json()) as ErrorData; - - // According to OAuth RFC 8628, handle standard polling responses - if ( - response.status === 400 && - errorData.error === 'authorization_pending' - ) { - // User has not yet approved the authorization request. Continue polling. - return { status: 'pending' } as DeviceTokenPendingData; - } - - if (response.status === 429 && errorData.error === 'slow_down') { - // Client is polling too frequently. Return pending with slowDown flag. - return { - status: 'pending', - slowDown: true, - } as DeviceTokenPendingData; - } - - // Handle other 400 errors (access_denied, expired_token, etc.) as real errors - - // For other errors, throw with proper error information - const error = new Error( - `Device token poll failed: ${errorData.error || 'Unknown error'} - ${errorData.error_description || 'No details provided'}`, - ); - (error as Error & { status?: number }).status = response.status; - throw error; + errorData = JSON.parse(responseText) as ErrorData; } catch (_parseError) { - // If JSON parsing fails, fall back to text response - const errorData = await response.text(); + // If JSON parsing fails, use text response const error = new Error( - `Device token poll failed: ${response.status} ${response.statusText}. Response: ${errorData}`, + `Device token poll failed: ${response.status} ${response.statusText}. Response: ${responseText}`, ); (error as Error & { status?: number }).status = response.status; throw error; } + + // According to OAuth RFC 8628, handle standard polling responses + if ( + response.status === 400 && + errorData.error === 'authorization_pending' + ) { + // User has not yet approved the authorization request. Continue polling. + return { status: 'pending' } as DeviceTokenPendingData; + } + + if (response.status === 429 && errorData.error === 'slow_down') { + // Client is polling too frequently. Return pending with slowDown flag. + return { + status: 'pending', + slowDown: true, + } as DeviceTokenPendingData; + } + + // Handle other 400 errors (access_denied, expired_token, etc.) as real errors + + // For other errors, throw with proper error information + const error = new Error( + `Device token poll failed: ${errorData.error || 'Unknown error'} - ${errorData.error_description}`, + ); + (error as Error & { status?: number }).status = response.status; + throw error; } return (await response.json()) as DeviceTokenResponse;