From 9406a273da1c2c7c3e074cfa5e103810f4cecf36 Mon Sep 17 00:00:00 2001 From: sam hoang Date: Sat, 1 Feb 2025 20:07:57 +0700 Subject: [PATCH 1/3] fix(api-config) error when creation of api config --- webview-ui/src/components/settings/ApiOptions.tsx | 5 ++++- webview-ui/src/context/ExtensionStateContext.tsx | 13 +++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index 4bdff0b061..277a532c48 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -128,7 +128,10 @@ const ApiOptions = ({ apiErrorMessage, modelIdErrorMessage }: ApiOptionsProps) = id="api-provider" value={selectedProvider} onChange={(value: unknown) => { - handleInputChange("apiProvider")({ + handleInputChange( + "apiProvider", + true, + )({ target: { value: (value as DropdownOption).value, }, diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 47db6bf6bb..c7daf643e3 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -71,7 +71,7 @@ export interface ExtensionStateContextType extends ExtensionState { setEnhancementApiConfigId: (value: string) => void setExperimentEnabled: (id: ExperimentId, enabled: boolean) => void setAutoApprovalEnabled: (value: boolean) => void - handleInputChange: (field: keyof ApiConfiguration) => (event: any) => void + handleInputChange: (field: keyof ApiConfiguration, softUpdate?: boolean) => (event: any) => void customModes: ModeConfig[] setCustomModes: (value: ModeConfig[]) => void } @@ -142,7 +142,16 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode }, []) const handleInputChange = useCallback( - (field: keyof ApiConfiguration) => (event: any) => { + (field: keyof ApiConfiguration, softUpdate?: boolean) => (event: any) => { + if (softUpdate === true) { + setState((currentState) => { + return { + ...currentState, + apiConfiguration: { ...currentState.apiConfiguration, [field]: event.target.value }, + } + }) + return + } setState((currentState) => { vscode.postMessage({ type: "upsertApiConfiguration", From 081faf34a041f52cb30718c764cb827bde24639c Mon Sep 17 00:00:00 2001 From: sam hoang Date: Mon, 3 Feb 2025 12:57:43 +0700 Subject: [PATCH 2/3] update test for test currently fail in upsertApiConfiguration --- .../webview/__tests__/ClineProvider.test.ts | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/src/core/webview/__tests__/ClineProvider.test.ts b/src/core/webview/__tests__/ClineProvider.test.ts index 664b7b5f0c..01c4faeb06 100644 --- a/src/core/webview/__tests__/ClineProvider.test.ts +++ b/src/core/webview/__tests__/ClineProvider.test.ts @@ -1252,4 +1252,129 @@ describe("ClineProvider", () => { ) }) }) + + describe("upsertApiConfiguration", () => { + test("handles error in upsertApiConfiguration gracefully", async () => { + provider.resolveWebviewView(mockWebviewView) + const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0] + + // Mock ConfigManager methods to simulate error + provider.configManager = { + setModeConfig: jest.fn().mockRejectedValue(new Error("Failed to update mode config")), + listConfig: jest + .fn() + .mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), + } as any + + // Mock getState to provide necessary data + jest.spyOn(provider, "getState").mockResolvedValue({ + mode: "code", + currentApiConfigName: "test-config", + } as any) + + // Trigger updateApiConfiguration + await messageHandler({ + type: "upsertApiConfiguration", + text: "test-config", + apiConfiguration: { + apiProvider: "anthropic", + apiKey: "test-key", + }, + }) + + // Verify error was logged and user was notified + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("Error create new api configuration"), + ) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Failed to create api configuration") + }) + + test("handles successful upsertApiConfiguration", async () => { + provider.resolveWebviewView(mockWebviewView) + const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0] + + // Mock ConfigManager methods + provider.configManager = { + saveConfig: jest.fn().mockResolvedValue(undefined), + listConfig: jest + .fn() + .mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), + } as any + + const testApiConfig = { + apiProvider: "anthropic" as const, + apiKey: "test-key", + } + + // Trigger upsertApiConfiguration + await messageHandler({ + type: "upsertApiConfiguration", + text: "test-config", + apiConfiguration: testApiConfig, + }) + + // Verify config was saved + expect(provider.configManager.saveConfig).toHaveBeenCalledWith("test-config", testApiConfig) + + // Verify state updates + expect(mockContext.globalState.update).toHaveBeenCalledWith("listApiConfigMeta", [ + { name: "test-config", id: "test-id", apiProvider: "anthropic" }, + ]) + expect(mockContext.globalState.update).toHaveBeenCalledWith("currentApiConfigName", "test-config") + + // Verify state was posted to webview + expect(mockPostMessage).toHaveBeenCalledWith(expect.objectContaining({ type: "state" })) + }) + + test("handles buildApiHandler error in updateApiConfiguration", async () => { + provider.resolveWebviewView(mockWebviewView) + const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as jest.Mock).mock.calls[0][0] + + // Mock buildApiHandler to throw an error + const { buildApiHandler } = require("../../../api") + ;(buildApiHandler as jest.Mock).mockImplementationOnce(() => { + throw new Error("API handler error") + }) + + // Mock ConfigManager methods + provider.configManager = { + saveConfig: jest.fn().mockResolvedValue(undefined), + listConfig: jest + .fn() + .mockResolvedValue([{ name: "test-config", id: "test-id", apiProvider: "anthropic" }]), + } as any + + // Setup mock Cline instance + const mockCline = { + api: undefined, + abortTask: jest.fn(), + } + // @ts-ignore - accessing private property for testing + provider.cline = mockCline + + const testApiConfig = { + apiProvider: "anthropic" as const, + apiKey: "test-key", + } + + // Trigger upsertApiConfiguration + await messageHandler({ + type: "upsertApiConfiguration", + text: "test-config", + apiConfiguration: testApiConfig, + }) + + // Verify error handling + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("Error create new api configuration"), + ) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Failed to create api configuration") + + // Verify state was still updated + expect(mockContext.globalState.update).toHaveBeenCalledWith("listApiConfigMeta", [ + { name: "test-config", id: "test-id", apiProvider: "anthropic" }, + ]) + expect(mockContext.globalState.update).toHaveBeenCalledWith("currentApiConfigName", "test-config") + }) + }) }) From 3d2ba7b361b70b4d36901eb10d1688a025e93781 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Mon, 3 Feb 2025 23:48:22 -0500 Subject: [PATCH 3/3] Code cleanup --- .../src/context/ExtensionStateContext.tsx | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index c7daf643e3..ac9243d572 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -142,23 +142,29 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode }, []) const handleInputChange = useCallback( + // Returns a function that handles an input change event for a specific API configuration field. + // The optional "softUpdate" flag determines whether to immediately update local state or send an external update. (field: keyof ApiConfiguration, softUpdate?: boolean) => (event: any) => { - if (softUpdate === true) { - setState((currentState) => { + // Use the functional form of setState to ensure the latest state is used in the update logic. + setState((currentState) => { + if (softUpdate) { + // Return a new state object with the updated apiConfiguration. + // This will trigger a re-render with the new configuration value. return { ...currentState, apiConfiguration: { ...currentState.apiConfiguration, [field]: event.target.value }, } - }) - return - } - setState((currentState) => { - vscode.postMessage({ - type: "upsertApiConfiguration", - text: currentState.currentApiConfigName, - apiConfiguration: { ...currentState.apiConfiguration, [field]: event.target.value }, - }) - return currentState // No state update needed + } else { + // For non-soft updates, send a message to the VS Code extension with the updated config. + // This side effect communicates the change without updating local React state. + vscode.postMessage({ + type: "upsertApiConfiguration", + text: currentState.currentApiConfigName, + apiConfiguration: { ...currentState.apiConfiguration, [field]: event.target.value }, + }) + // Return the unchanged state as no local state update is intended in this branch. + return currentState + } }) }, [],