ouyang/cherry-studio
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: address PR review issues for Volcengine integration

Browse Source 
- Fix region field being ignored: pass user-configured region to listFoundationModels and listEndpoints
- Add user notification before silent fallback when API fails
- Throw error on credential corruption instead of returning null
- Remove redundant credentials (accessKeyId, secretAccessKey) from Redux store (they're securely stored via safeStorage)
- Add warnings field to ListModelsResult for partial API failures
- Fix Redux/IPC order: save to secure storage first, then update Redux on success
- Update related tests

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
suyao
2025-11-29 19:39:15 +08:00
parent 115cd80432
commit 76d48f9ccb
7 changed files with 66 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
src/main/services/VolcengineService.ts
View File

@@ -118,6 +118,7 @@ interface ModelInfo {
interface ListModelsResult {
models: ModelInfo[]
total?: number
warnings?: string[]
}
// Custom error class
@@ -401,19 +402,23 @@ class VolcengineService {
/**
* Load credentials from encrypted storage
* @throws VolcengineServiceError if credentials file exists but is corrupted
*/
private async loadCredentials(): Promise<VolcengineCredentials | null> {
try {
if (!fs.existsSync(this.credentialsFilePath)) {
return null
}
if (!fs.existsSync(this.credentialsFilePath)) {
return null
}
try {
const encryptedData = await fs.promises.readFile(this.credentialsFilePath)
const decryptedJson = safeStorage.decryptString(Buffer.from(encryptedData))
return JSON.parse(decryptedJson) as VolcengineCredentials
} catch (error) {
logger.error('Failed to load Volcengine credentials:', error as Error)
return null
throw new VolcengineServiceError(
'Credentials file exists but could not be loaded. Please re-enter your credentials.',
error
)
}
}
@@ -521,7 +526,7 @@ class VolcengineService {
/**
* List foundation models from Volcengine ARK
*/
private async listFoundationModels(): Promise<ListFoundationModelsResponse> {
private async listFoundationModels(region: string = CONFIG.DEFAULT_REGION): Promise<ListFoundationModelsResponse> {
const requestBody: ListFoundationModelsRequest = {
PageNumber: 1,
PageSize: CONFIG.DEFAULT_PAGE_SIZE
@@ -536,7 +541,7 @@ class VolcengineService {
{},
requestBody,
CONFIG.SERVICE_NAME,
CONFIG.DEFAULT_REGION
region
)
return ListFoundationModelsResponseSchema.parse(response)
@@ -545,7 +550,10 @@ class VolcengineService {
/**
* List user-created endpoints from Volcengine ARK
*/
private async listEndpoints(projectName?: string): Promise<ListEndpointsResponse> {
private async listEndpoints(
projectName?: string,
region: string = CONFIG.DEFAULT_REGION
): Promise<ListEndpointsResponse> {
const requestBody: ListEndpointsRequest = {
ProjectName: projectName || 'default',
PageNumber: 1,
@@ -561,7 +569,7 @@ class VolcengineService {
{},
requestBody,
CONFIG.SERVICE_NAME,
CONFIG.DEFAULT_REGION
region
)
return ListEndpointsResponseSchema.parse(response)
@@ -571,14 +579,20 @@ class VolcengineService {
* List all available models from Volcengine ARK
* Combines foundation models and user-created endpoints
*/
public listModels = async (_?: Electron.IpcMainInvokeEvent, projectName?: string): Promise<ListModelsResult> => {
public listModels = async (
_?: Electron.IpcMainInvokeEvent,
projectName?: string,
region?: string
): Promise<ListModelsResult> => {
try {
const effectiveRegion = region || CONFIG.DEFAULT_REGION
const [foundationModelsResult, endpointsResult] = await Promise.allSettled([
this.listFoundationModels(),
this.listEndpoints(projectName)
this.listFoundationModels(effectiveRegion),
this.listEndpoints(projectName, effectiveRegion)
])
const models: ModelInfo[] = []
const warnings: string[] = []
if (foundationModelsResult.status === 'fulfilled') {
const foundationModels = foundationModelsResult.value
@@ -591,7 +605,9 @@ class VolcengineService {
}
logger.info(`Found ${foundationModels.Result.Items.length} foundation models`)
} else {
logger.warn('Failed to fetch foundation models:', foundationModelsResult.reason)
const errorMsg = `Failed to fetch foundation models: ${foundationModelsResult.reason}`
logger.warn(errorMsg)
warnings.push(errorMsg)
}
// Process endpoints
@@ -618,7 +634,9 @@ class VolcengineService {
}
logger.info(`Found ${endpoints.Result.Items.length} endpoints`)
} else {
logger.warn('Failed to fetch endpoints:', endpointsResult.reason)
const errorMsg = `Failed to fetch endpoints: ${endpointsResult.reason}`
logger.warn(errorMsg)
warnings.push(errorMsg)
}
// If both failed, throw error
@@ -634,7 +652,8 @@ class VolcengineService {
return {
models,
total
total,
warnings: warnings.length > 0 ? warnings : undefined
}
} catch (error) {
logger.error('Failed to list Volcengine models:', error as Error)

6
src/preload/index.ts
View File

@@ -579,11 +579,13 @@ const api = {
hasCredentials: (): Promise<boolean> => ipcRenderer.invoke(IpcChannel.Volcengine_HasCredentials),
clearCredentials: (): Promise<void> => ipcRenderer.invoke(IpcChannel.Volcengine_ClearCredentials),
listModels: (
projectName?: string
projectName?: string,
region?: string
): Promise<{
models: Array<{ id: string; name: string; description?: string; created?: number }>
total?: number
}> => ipcRenderer.invoke(IpcChannel.Volcengine_ListModels, projectName),
warnings?: string[]
}> => ipcRenderer.invoke(IpcChannel.Volcengine_ListModels, projectName, region),
getAuthHeaders: (params: {
method: 'GET' | 'POST'
host: string

15
src/renderer/src/aiCore/legacy/clients/volcengine/VolcengineAPIClient.ts
View File

@@ -1,6 +1,6 @@
import type OpenAI from '@cherrystudio/openai'
import { loggerService } from '@logger'
import { getVolcengineProjectName } from '@renderer/hooks/useVolcengine'
import { getVolcengineProjectName, getVolcengineRegion } from '@renderer/hooks/useVolcengine'
import type { Provider } from '@renderer/types'
import { OpenAIAPIClient } from '../openai/OpenAIApiClient'
@@ -35,13 +35,22 @@ export class VolcengineAPIClient extends OpenAIAPIClient {
logger.info('Fetching models from Volcengine API using signed request')
const projectName = getVolcengineProjectName()
const response = await window.api.volcengine.listModels(projectName)
const region = getVolcengineRegion()
const response = await window.api.volcengine.listModels(projectName, region)
if (!response || !response.models) {
logger.warn('Empty response from Volcengine listModels')
return []
}
// Notify user of any partial failures
if (response.warnings && response.warnings.length > 0) {
for (const warning of response.warnings) {
logger.warn(warning)
}
window.toast?.warning('Some Volcengine models could not be fetched. Check logs for details.')
}
const models: OpenAI.Models.Model[] = response.models.map((model) => ({
id: model.id,
object: 'model' as const,
@@ -55,6 +64,8 @@ export class VolcengineAPIClient extends OpenAIAPIClient {
return models
} catch (error) {
logger.error('Failed to list Volcengine models:', error as Error)
// Notify user before falling back
window.toast?.warning('Failed to fetch Volcengine models. Check credentials if this persists.')
// Fall back to standard OpenAI-compatible API on error
logger.info('Falling back to OpenAI-compatible model list')
return super.listModels()

17
src/renderer/src/hooks/useVolcengine.ts
View File

@@ -1,10 +1,5 @@
import store, { useAppSelector } from '@renderer/store'
import {
setVolcengineAccessKeyId,
setVolcengineProjectName,
setVolcengineRegion,
setVolcengineSecretAccessKey
} from '@renderer/store/llm'
import { setVolcengineProjectName, setVolcengineRegion } from '@renderer/store/llm'
import { useDispatch } from 'react-redux'
export function useVolcengineSettings() {
@@ -13,8 +8,6 @@ export function useVolcengineSettings() {
return {
...settings,
setAccessKeyId: (accessKeyId: string) => dispatch(setVolcengineAccessKeyId(accessKeyId)),
setSecretAccessKey: (secretAccessKey: string) => dispatch(setVolcengineSecretAccessKey(secretAccessKey)),
setRegion: (region: string) => dispatch(setVolcengineRegion(region)),
setProjectName: (projectName: string) => dispatch(setVolcengineProjectName(projectName))
}
@@ -24,14 +17,6 @@ export function getVolcengineSettings() {
return store.getState().llm.settings.volcengine
}
export function getVolcengineAccessKeyId() {
return store.getState().llm.settings.volcengine.accessKeyId
}
export function getVolcengineSecretAccessKey() {
return store.getState().llm.settings.volcengine.secretAccessKey
}
export function getVolcengineRegion() {
return store.getState().llm.settings.volcengine.region
}

51
src/renderer/src/pages/settings/ProviderSettings/VolcengineSettings.tsx
View File

@@ -10,22 +10,13 @@ import { SettingHelpLink, SettingHelpText, SettingHelpTextRow, SettingSubtitle }
const VolcengineSettings: FC = () => {
const { t } = useTranslation()
const {
accessKeyId,
secretAccessKey,
region,
projectName,
setAccessKeyId,
setSecretAccessKey,
setRegion,
setProjectName
} = useVolcengineSettings()
const { region, projectName, setRegion, setProjectName } = useVolcengineSettings()
const providerConfig = PROVIDER_URLS['doubao']
const apiKeyWebsite = providerConfig?.websites?.apiKey
const [localAccessKeyId, setLocalAccessKeyId] = useState(accessKeyId)
const [localSecretAccessKey, setLocalSecretAccessKey] = useState(secretAccessKey)
const [localAccessKeyId, setLocalAccessKeyId] = useState('')
const [localSecretAccessKey, setLocalSecretAccessKey] = useState('')
const [localRegion, setLocalRegion] = useState(region)
const [localProjectName, setLocalProjectName] = useState(projectName)
const [saving, setSaving] = useState(false)
@@ -36,13 +27,11 @@ const VolcengineSettings: FC = () => {
window.api.volcengine.hasCredentials().then(setHasCredentials)
}, [])
// Sync local state with store
// Sync local state with store (only for region and projectName)
useEffect(() => {
setLocalAccessKeyId(accessKeyId)
setLocalSecretAccessKey(secretAccessKey)
setLocalRegion(region)
setLocalProjectName(projectName)
}, [accessKeyId, secretAccessKey, region, projectName])
}, [region, projectName])
const handleSaveCredentials = useCallback(async () => {
if (!localAccessKeyId || !localSecretAccessKey) {
@@ -52,38 +41,28 @@ const VolcengineSettings: FC = () => {
setSaving(true)
try {
// Save to Redux store
setAccessKeyId(localAccessKeyId)
setSecretAccessKey(localSecretAccessKey)
// Save credentials to secure storage via IPC first
await window.api.volcengine.saveCredentials(localAccessKeyId, localSecretAccessKey)
// Only update Redux after IPC success (for region and projectName only)
setRegion(localRegion)
setProjectName(localProjectName)
// Save to secure storage via IPC
await window.api.volcengine.saveCredentials(localAccessKeyId, localSecretAccessKey)
setHasCredentials(true)
// Clear local credential state after successful save (they're now in secure storage)
setLocalAccessKeyId('')
setLocalSecretAccessKey('')
window.toast.success(t('settings.provider.volcengine.credentials_saved'))
} catch (error) {
window.toast.error(String(error))
} finally {
setSaving(false)
}
}, [
localAccessKeyId,
localSecretAccessKey,
localRegion,
localProjectName,
setAccessKeyId,
setSecretAccessKey,
setRegion,
setProjectName,
t
])
}, [localAccessKeyId, localSecretAccessKey, localRegion, localProjectName, setRegion, setProjectName, t])
const handleClearCredentials = useCallback(async () => {
try {
await window.api.volcengine.clearCredentials()
setAccessKeyId('')
setSecretAccessKey('')
setLocalAccessKeyId('')
setLocalSecretAccessKey('')
setHasCredentials(false)
@@ -91,7 +70,7 @@ const VolcengineSettings: FC = () => {
} catch (error) {
window.toast.error(String(error))
}
}, [setAccessKeyId, setSecretAccessKey, t])
}, [t])
return (
<>
@@ -103,7 +82,6 @@ const VolcengineSettings: FC = () => {
value={localAccessKeyId}
placeholder="Access Key ID"
onChange={(e) => setLocalAccessKeyId(e.target.value)}
onBlur={() => setAccessKeyId(localAccessKeyId)}
style={{ marginTop: 5 }}
spellCheck={false}
/>
@@ -116,7 +94,6 @@ const VolcengineSettings: FC = () => {
value={localSecretAccessKey}
placeholder="Secret Access Key"
onChange={(e) => setLocalSecretAccessKey(e.target.value)}
onBlur={() => setSecretAccessKey(localSecretAccessKey)}
style={{ marginTop: 5 }}
spellCheck={false}
/>

2
src/renderer/src/services/__tests__/ApiService.test.ts
View File

@@ -244,8 +244,6 @@ vi.mock('@renderer/store/llm.ts', () => {
region: ''
},
volcengine: {
accessKeyId: '',
secretAccessKey: '',
region: 'cn-beijing',
projectName: 'default'
}

12
src/renderer/src/store/llm.ts
View File

@@ -32,8 +32,6 @@ type LlmSettings = {
region: string
}
volcengine: {
accessKeyId: string
secretAccessKey: string
region: string
projectName: string
}
@@ -83,8 +81,6 @@ export const initialState: LlmState = {
region: ''
},
volcengine: {
accessKeyId: '',
secretAccessKey: '',
region: 'cn-beijing',
projectName: 'default'
}
@@ -228,12 +224,6 @@ const llmSlice = createSlice({
setAwsBedrockRegion: (state, action: PayloadAction<string>) => {
state.settings.awsBedrock.region = action.payload
},
setVolcengineAccessKeyId: (state, action: PayloadAction<string>) => {
state.settings.volcengine.accessKeyId = action.payload
},
setVolcengineSecretAccessKey: (state, action: PayloadAction<string>) => {
state.settings.volcengine.secretAccessKey = action.payload
},
setVolcengineRegion: (state, action: PayloadAction<string>) => {
state.settings.volcengine.region = action.payload
},
@@ -281,8 +271,6 @@ export const {
setAwsBedrockSecretAccessKey,
setAwsBedrockApiKey,
setAwsBedrockRegion,
setVolcengineAccessKeyId,
setVolcengineSecretAccessKey,
setVolcengineRegion,
setVolcengineProjectName,
updateModel