From af7708650801a19a0a3aaa47f2c6c6eb8ecee119 Mon Sep 17 00:00:00 2001 From: KevinYoung-Kw Date: Thu, 20 Aug 2026 00:34:23 +0000 Subject: [PATCH] fix(openai/codex): surface nested OAuth errors and lastError Expand token-exchange errors so `{error:{message}}` is not `[object Object]`, persist lastError on status, and show region/expired-state hints via i18n. Fixes #464. --- src/__tests__/unit/openai-oauth-error.test.ts | 60 +++++++++++++++++++ src/components/settings/ProviderManager.tsx | 34 +++++++++-- src/i18n/en.ts | 6 ++ src/i18n/zh.ts | 6 ++ src/lib/openai-oauth-manager.ts | 17 +++++- src/lib/openai-oauth.ts | 57 +++++++++++++----- 6 files changed, 159 insertions(+), 21 deletions(-) create mode 100644 src/__tests__/unit/openai-oauth-error.test.ts diff --git a/src/__tests__/unit/openai-oauth-error.test.ts b/src/__tests__/unit/openai-oauth-error.test.ts new file mode 100644 index 000000000..e2af765b9 --- /dev/null +++ b/src/__tests__/unit/openai-oauth-error.test.ts @@ -0,0 +1,60 @@ +/** + * Tests for nested OAuth error extraction (issue #464). + * + * OpenAI sometimes returns `{ "error": { "message": "..." } }`. The old + * shallow parse used `j.error` as a truthy object, so users saw + * `403 - [object Object]`. These cases pin the nested extract order. + */ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +import { extractOAuthErrorMessage } from '../../lib/openai-oauth'; + +describe('extractOAuthErrorMessage', () => { + it('extracts nested error.message and never returns [object Object]', () => { + const raw = JSON.stringify({ + error: { message: 'Country, region, or territory not supported' }, + }); + const msg = extractOAuthErrorMessage(raw); + assert.equal(msg, 'Country, region, or territory not supported'); + assert.equal(msg.includes('[object Object]'), false); + }); + + it('prefers error_description over nested error.message', () => { + const raw = JSON.stringify({ + error_description: 'invalid_grant: code expired', + error: { message: 'Country, region, or territory not supported' }, + }); + assert.equal(extractOAuthErrorMessage(raw), 'invalid_grant: code expired'); + }); + + it('returns a non-JSON body as-is', () => { + const raw = 'upstream 502 bad gateway'; + assert.equal(extractOAuthErrorMessage(raw), raw); + }); + + it('falls back to Error.message when the body is empty', () => { + assert.equal( + extractOAuthErrorMessage('', new Error('socket hang up')), + 'socket hang up', + ); + }); + + it('stringifies a string error field', () => { + const raw = JSON.stringify({ error: 'invalid_grant' }); + assert.equal(extractOAuthErrorMessage(raw), 'invalid_grant'); + }); +}); + +describe('openai-oauth-manager lastError pin', () => { + it('persists openai_oauth_last_error and exposes oauthError', () => { + const src = readFileSync( + join(__dirname, '../../lib/openai-oauth-manager.ts'), + 'utf8', + ); + assert.match(src, /openai_oauth_last_error/); + assert.match(src, /oauthError/); + }); +}); diff --git a/src/components/settings/ProviderManager.tsx b/src/components/settings/ProviderManager.tsx index cf8301de3..6e84f7371 100644 --- a/src/components/settings/ProviderManager.tsx +++ b/src/components/settings/ProviderManager.tsx @@ -125,6 +125,16 @@ export function ProviderManager() { const [envDetected, setEnvDetected] = useState>({}); const { t } = useTranslation(); const isZh = t('nav.chats') === '对话'; + const mapOpenAIOAuthError = useCallback((raw: string): string => { + const lower = raw.toLowerCase(); + if (lower.includes('country, region, or territory not supported')) { + return t('provider.openaiOAuth.error.regionUnsupported'); + } + if (lower.includes('invalid or expired state') || lower.includes('invalid state')) { + return t('provider.openaiOAuth.error.expiredState'); + } + return raw; + }, [t]); // Edit dialog state — fallback ProviderForm for providers that don't match any preset const [formOpen, setFormOpen] = useState(false); @@ -146,7 +156,12 @@ export function ProviderManager() { const [deleting, setDeleting] = useState(false); // OpenAI OAuth state - const [openaiAuth, setOpenaiAuth] = useState<{ authenticated: boolean; email?: string; plan?: string } | null>(null); + const [openaiAuth, setOpenaiAuth] = useState<{ + authenticated: boolean; + email?: string; + plan?: string; + oauthError?: string; + } | null>(null); const [openaiLoggingIn, setOpenaiLoggingIn] = useState(false); const [openaiError, setOpenaiError] = useState(null); @@ -369,9 +384,15 @@ export function ProviderManager() { useEffect(() => { fetch('/api/openai-oauth/status') .then(r => r.ok ? r.json() : null) - .then(data => { if (data) setOpenaiAuth(data); }) + .then(data => { + if (!data) return; + setOpenaiAuth(data); + if (!data.authenticated && data.oauthError) { + setOpenaiError(mapOpenAIOAuthError(data.oauthError)); + } + }) .catch(() => {}); - }, []); + }, [mapOpenAIOAuthError]); const fetchXaiOAuthStatus = useCallback(async () => { try { @@ -613,7 +634,7 @@ export function ProviderManager() { if (pollCount >= maxPolls) { clearInterval(poll); setOpenaiLoggingIn(false); - setOpenaiError(isZh ? '登录超时，请重试' : 'Login timed out, please try again'); + setOpenaiError(t('provider.openaiOAuth.error.timeout')); return; } try { @@ -629,6 +650,11 @@ export function ProviderManager() { // counts; broadcast so listeners (SetupCenter's ProviderCard, // anywhere reading provider presence) re-evaluate. window.dispatchEvent(new Event('provider-changed')); + } else if (status.oauthError) { + clearInterval(poll); + setOpenaiAuth(status); + setOpenaiLoggingIn(false); + setOpenaiError(mapOpenAIOAuthError(status.oauthError)); } } } catch { /* keep polling */ } diff --git a/src/i18n/en.ts b/src/i18n/en.ts index 5f14c9661..c5367ccc9 100644 --- a/src/i18n/en.ts +++ b/src/i18n/en.ts @@ -338,6 +338,12 @@ const en = { 'provider.ccSwitchHint': 'Claude Code settings have been moved to the Claude Code settings page.', 'provider.goToClaudeCodeSettings': 'Go to Settings', 'provider.openaiOAuthHint': 'Sign in with ChatGPT Plus/Pro to access OpenAI models without an API key.', + 'provider.openaiOAuth.error.regionUnsupported': + 'OpenAI OAuth is unavailable in your current country/region. You can use an OpenAI-compatible third-party endpoint via Provider settings (Base URL + API key).', + 'provider.openaiOAuth.error.expiredState': + 'The login session has expired. Please click OpenAI Login again and complete authorization in the newly opened page.', + 'provider.openaiOAuth.error.timeout': + 'Login timed out, please try again', 'provider.addProviderSection': 'Add Provider', 'provider.addProviderDesc': 'Select a provider to connect. Most presets only require an API key.', 'provider.connectedServices': 'Connected services', diff --git a/src/i18n/zh.ts b/src/i18n/zh.ts index 3917164ff..aba33b82d 100644 --- a/src/i18n/zh.ts +++ b/src/i18n/zh.ts @@ -325,6 +325,12 @@ const zh: Record = { 'provider.ccSwitchHint': 'Claude Code 相关设置已迁移到 Claude Code 设置页面。', 'provider.goToClaudeCodeSettings': '前往设置', 'provider.openaiOAuthHint': '使用 ChatGPT Plus/Pro 订阅登录，无需 API Key 即可使用 OpenAI 模型。', + 'provider.openaiOAuth.error.regionUnsupported': + 'OpenAI OAuth 当前在你所在地区不可用。可改用兼容 OpenAI 协议的第三方服务（在 Provider 中配置 Base URL + API Key）。', + 'provider.openaiOAuth.error.expiredState': + '登录会话已过期，请重新点击 OpenAI 登录并在新打开的页面中完成授权。', + 'provider.openaiOAuth.error.timeout': + '登录超时，请重试', 'provider.addProviderSection': '添加提供商', 'provider.addProviderDesc': '选择要连接的提供商。大多数预设只需填写 API 密钥。', 'provider.connectedServices': '已连接服务', diff --git a/src/lib/openai-oauth-manager.ts b/src/lib/openai-oauth-manager.ts index 07b13c68a..ed7bcd0b0 100644 --- a/src/lib/openai-oauth-manager.ts +++ b/src/lib/openai-oauth-manager.ts @@ -27,6 +27,7 @@ const KEYS = { email: 'openai_oauth_email', plan: 'openai_oauth_plan', accountId: 'openai_oauth_account_id', + lastError: 'openai_oauth_last_error', } as const; const REFRESH_BUFFER_MS = 5 * 60 * 1000; @@ -40,11 +41,17 @@ export interface OpenAIOAuthStatus { accountId?: string; /** True when token is near/past expiry but a refresh token exists */ needsRefresh?: boolean; + oauthError?: string; } export function getOAuthStatus(): OpenAIOAuthStatus { const accessToken = getSetting(KEYS.accessToken); - if (!accessToken) return { authenticated: false }; + if (!accessToken) { + return { + authenticated: false, + oauthError: getSetting(KEYS.lastError) || undefined, + }; + } // Check if token is expired and no refresh token available const expiresAt = Number(getSetting(KEYS.expiresAt) || '0'); @@ -65,6 +72,7 @@ export function getOAuthStatus(): OpenAIOAuthStatus { plan: getSetting(KEYS.plan), accountId: getSetting(KEYS.accountId), needsRefresh, + oauthError: undefined, }; } @@ -132,6 +140,7 @@ function saveTokens(tokens: OAuthTokens): void { setSetting(KEYS.idToken, tokens.idToken); if (tokens.refreshToken) setSetting(KEYS.refreshToken, tokens.refreshToken); if (tokens.expiresAt) setSetting(KEYS.expiresAt, String(tokens.expiresAt)); + setSetting(KEYS.lastError, ''); const claims = parseIdTokenClaims(tokens.idToken); if (claims.email) setSetting(KEYS.email, claims.email); @@ -200,6 +209,7 @@ export async function startOAuthFlow(): Promise<{ authUrl: string; completion: P prevPending.reject(new Error('Superseded by new login attempt')); setPendingOAuth(undefined); } + setSetting(KEYS.lastError, ''); const flow = prepareOAuthFlow(); @@ -247,6 +257,7 @@ async function startOAuthServer(): Promise { if (error) { const msg = errorDesc || error; + setSetting(KEYS.lastError, msg); res.writeHead(200, { 'Content-Type': 'text/html' }); res.end(errorHtml(msg)); getPendingOAuth()?.reject(new Error(msg)); @@ -257,7 +268,8 @@ async function startOAuthServer(): Promise { const pending = getPendingOAuth(); if (!code || !pending || state !== pending.state) { - const msg = !code ? 'Missing authorization code' : 'Invalid state'; + const msg = !code ? 'Missing authorization code' : 'Invalid or expired state'; + setSetting(KEYS.lastError, msg); res.writeHead(400, { 'Content-Type': 'text/html' }); res.end(errorHtml(msg)); getPendingOAuth()?.reject(new Error(msg)); @@ -278,6 +290,7 @@ async function startOAuthServer(): Promise { current.resolve(tokens.accessToken); } catch (err) { const message = err instanceof Error ? err.message : String(err); + setSetting(KEYS.lastError, `Token exchange failed: ${message}`); res.writeHead(200, { 'Content-Type': 'text/html' }); res.end(errorHtml(`Token exchange failed: ${message}`)); current.reject(err instanceof Error ? err : new Error(message)); diff --git a/src/lib/openai-oauth.ts b/src/lib/openai-oauth.ts index 3ae62ae1e..a0482a818 100644 --- a/src/lib/openai-oauth.ts +++ b/src/lib/openai-oauth.ts @@ -115,6 +115,44 @@ async function sleep(ms: number): Promise { return new Promise((r) => setTimeout(r, ms)); } +function stringifyUnknown(value: unknown): string { + if (typeof value === 'string') return value; + if (value === null || value === undefined) return ''; + try { + return JSON.stringify(value); + } catch { + return String(value); + } +} + +export function extractOAuthErrorMessage(rawBody: string, fallbackErr?: unknown): string { + if (!rawBody) { + return fallbackErr instanceof Error ? fallbackErr.message : 'unknown'; + } + try { + const parsed = JSON.parse(rawBody) as Record; + const candidates: unknown[] = [ + parsed.error_description, + typeof parsed.error === 'object' && parsed.error !== null + ? (parsed.error as Record).message + : undefined, + typeof parsed.error === 'object' && parsed.error !== null + ? (parsed.error as Record).code + : undefined, + parsed.error, + parsed.message, + parsed, + ]; + for (const item of candidates) { + const text = stringifyUnknown(item).trim(); + if (text) return text; + } + return rawBody; + } catch { + return rawBody; + } +} + export async function exchangeCodeForTokens( code: string, codeVerifier: string, @@ -187,16 +225,9 @@ export async function exchangeCodeForTokens( break; } - // Out of retries — produce a useful error. JSON.stringify the body when - // possible so users (and Sentry) see structured fields instead of the - // legacy "[object Object]" placeholder that issue #464 complained about. - let msg: string; - try { - const j = JSON.parse(lastBody); - msg = j.error_description || j.error || JSON.stringify(j); - } catch { - msg = lastBody || (lastErr instanceof Error ? lastErr.message : 'unknown'); - } + // Out of retries — produce a useful error. Nested `{error:{message}}` + // bodies used to interpolate as "[object Object]" (issue #464). + const msg = extractOAuthErrorMessage(lastBody, lastErr); throw new Error(`Token exchange failed after ${MAX_ATTEMPTS} attempts: ${lastStatus ?? 'network'} - ${msg}`); } @@ -220,11 +251,7 @@ export async function refreshTokens(refreshToken: string): Promise if (!response.ok) { const text = await response.text(); - let msg: string; - try { - const j = JSON.parse(text); - msg = j.error_description || j.error || text; - } catch { msg = text; } + const msg = extractOAuthErrorMessage(text); throw new Error(`Token refresh failed: ${response.status} - ${msg}`); }