From 72deba43ddbcf59d414326f2d320b7e45b1df24c Mon Sep 17 00:00:00 2001 From: Andrew Pareles Date: Tue, 10 Dec 2024 17:41:13 -0800 Subject: [PATCH] consistent error messages --- CONTRIBUTING.md | 2 +- .../void/browser/llmMessageService.ts | 98 +++++------ src/vs/platform/void/common/configTypes.ts | 5 +- .../platform/void/common/llmMessageTypes.ts | 2 +- .../electron-main/llmMessage/anthropic.ts | 2 +- .../void/electron-main/llmMessage/gemini.ts | 2 +- .../void/electron-main/llmMessage/groq.ts | 2 +- .../void/electron-main/llmMessage/ollama.ts | 18 +- .../void/electron-main/llmMessage/openai.ts | 2 +- .../llmMessage/sendLLMMessage.ts | 5 +- .../void/electron-main/llmMessageChannel.ts | 4 +- .../react/src/sidebar-tsx/ErrorDisplay.tsx | 2 +- .../browser/react/src/util/ErrorDisplay.tsx | 161 ------------------ 13 files changed, 64 insertions(+), 241 deletions(-) delete mode 100644 src/vs/workbench/contrib/void/browser/react/src/util/ErrorDisplay.tsx diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b31fe30f..e66d8485 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,7 +70,7 @@ If you ran `npm run watch`, the build is done when you see something like this: -4. In a new terminal, run `./scripts/code.sh` (Mac/Linux) or `./scripts/code.bat` (Windows). This should open up the built IDE! +4. In a new terminal, run `./scripts/code.sh` (Mac/Linux) or `./scripts/code.bat` (Windows). This should open up the built IDE. You can always press Ctrl+Shift+P and run "Reload Window" inside the new window to see changes without re-building. Now that you're set up, feel free to check out our [Issues](https://github.com/voideditor/void/issues) page. diff --git a/src/vs/platform/void/browser/llmMessageService.ts b/src/vs/platform/void/browser/llmMessageService.ts index 79fbb138..a31d56b8 100644 --- a/src/vs/platform/void/browser/llmMessageService.ts +++ b/src/vs/platform/void/browser/llmMessageService.ts @@ -10,7 +10,7 @@ import { InstantiationType, registerSingleton } from '../../instantiation/common import { generateUuid } from '../../../base/common/uuid.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; import { Event } from '../../../base/common/event.js'; -import { IDisposable } from '../../../base/common/lifecycle.js'; +import { Disposable } from '../../../base/common/lifecycle.js'; // BROWSER IMPLEMENTATION OF SENDLLMMESSAGE @@ -24,72 +24,59 @@ export interface ISendLLMMessageService { } -export class SendLLMMessageService implements ISendLLMMessageService { +export class SendLLMMessageService extends Disposable implements ISendLLMMessageService { readonly _serviceBrand: undefined; private readonly channel: IChannel // LLMMessageChannel - private readonly _disposablesOfRequestId: Record = {} + private readonly onTextHooks: { [eventId: string]: ((params: ProxyOnTextPayload) => void) } = {} + private readonly onFinalMessageHooks: { [eventId: string]: ((params: ProxyOnFinalMessagePayload) => void) } = {} + private readonly onErrorHooks: { [eventId: string]: ((params: ProxyOnErrorPayload) => void) } = {} - private readonly onTextEvent: Event - private readonly onFinalMessageEvent: Event - private readonly onErrorEvent: Event constructor( @IMainProcessService mainProcessService: IMainProcessService // used as a renderer (only usable on client side) ) { - - - this.channel = mainProcessService.getChannel('void-channel-sendLLMMessage') - - console.log('setting up IPC') - - // this sets up an IPC channel and takes a few ms, so should happen immediately - this.onTextEvent = this.channel.listen('onText') - this.onFinalMessageEvent = this.channel.listen('onFinalMessage') - this.onErrorEvent = this.channel.listen('onError') + super() // const service = ProxyChannel.toService(mainProcessService.getChannel('void-channel-sendLLMMessage')); // lets you call it like a service - } + this.channel = mainProcessService.getChannel('void-channel-sendLLMMessage') - _addDisposable(requestId: string, disposable: IDisposable) { - if (!this._disposablesOfRequestId[requestId]) { - this._disposablesOfRequestId[requestId] = [] - } - this._disposablesOfRequestId[requestId].push(disposable) - } + // this sets up an IPC channel and takes a few ms, so we set up listeners immediately and add hooks to them instead + const onTextEvent: Event = this.channel.listen('onText') + const onFinalMessageEvent: Event = this.channel.listen('onFinalMessage') + const onErrorEvent: Event = this.channel.listen('onError') + this._register( + onTextEvent(e => { + this.onTextHooks[e.requestId]?.(e) + }) + ) + + this._register( + onFinalMessageEvent(e => { + this.onFinalMessageHooks[e.requestId]?.(e) + this._onRequestIdDone(e.requestId) + }) + ) + + this._register( + onErrorEvent(e => { + console.log('Error in SendLLMMessageService:', JSON.stringify(e)) + this.onErrorHooks[e.requestId]?.(e) + this._onRequestIdDone(e.requestId) + }) + ) + } sendLLMMessage(params: LLMMessageServiceParams) { const requestId_ = generateUuid(); const { onText, onFinalMessage, onError, ...proxyParams } = params; - // listen for listenerName='onText' | 'onFinalMessage' | 'onError', and call the original function on it + this.onTextHooks[requestId_] = onText + this.onFinalMessageHooks[requestId_] = onFinalMessage + this.onErrorHooks[requestId_] = onError - this._addDisposable(requestId_, - this.onTextEvent(e => { - if (requestId_ !== e.requestId) return; - onText(e) - }) - ) - - this._addDisposable(requestId_, - this.onFinalMessageEvent(e => { - if (requestId_ !== e.requestId) return; - onFinalMessage(e) - this._dispose(requestId_) - }) - ) - - this._addDisposable(requestId_, - this.onErrorEvent(e => { - console.log('sendLLMMessageService - error event received (havent checked req)') - if (requestId_ !== e.requestId) return; - console.log('sendLLMMessageService - error event received', JSON.stringify(e)) - onError(e) - this._dispose(requestId_) - }) - ) // params will be stripped of all its functions this.channel.call('sendLLMMessage', { ...proxyParams, requestId: requestId_ } satisfies ProxyLLMMessageParams); @@ -97,17 +84,16 @@ export class SendLLMMessageService implements ISendLLMMessageService { return requestId_ } - private _dispose(requestId: string) { - if (!(requestId in this._disposablesOfRequestId)) return - for (const disposable of this._disposablesOfRequestId[requestId]) { - disposable.dispose() - } - delete this._disposablesOfRequestId[requestId] - } abort(requestId: string) { this.channel.call('abort', { requestId } satisfies ProxyLLMMessageAbortParams); - this._dispose(requestId) + this._onRequestIdDone(requestId) + } + + _onRequestIdDone(requestId: string) { + delete this.onTextHooks[requestId] + delete this.onFinalMessageHooks[requestId] + delete this.onErrorHooks[requestId] } } diff --git a/src/vs/platform/void/common/configTypes.ts b/src/vs/platform/void/common/configTypes.ts index 8a81841a..37da9dd2 100644 --- a/src/vs/platform/void/common/configTypes.ts +++ b/src/vs/platform/void/common/configTypes.ts @@ -34,7 +34,7 @@ export const voidProviderDefaults = { }, openAICompatible: { apiKey: '', - endpoint: 'http://127.0.0.1:11434/v1', + endpoint: 'https://my-website.com/v1', }, gemini: { apiKey: '', @@ -248,7 +248,8 @@ export const displayInfoOfSettingName = (providerName: ProviderName, settingName providerName === 'openRouter' ? 'sk-or-abc123...' : // sk-or-v1-abc123 providerName === 'gemini' ? 'abc123...' : providerName === 'groq' ? 'gsk_abc123...' : - '(never)', + providerName === 'openAICompatible' ? 'sk-abc123...' : + '(never)', } } else if (settingName === 'endpoint') { diff --git a/src/vs/platform/void/common/llmMessageTypes.ts b/src/vs/platform/void/common/llmMessageTypes.ts index b38f3b5c..aad810f3 100644 --- a/src/vs/platform/void/common/llmMessageTypes.ts +++ b/src/vs/platform/void/common/llmMessageTypes.ts @@ -12,7 +12,7 @@ export type OnText = (p: { newText: string, fullText: string }) => void export type OnFinalMessage = (p: { fullText: string }) => void -export type OnError = (p: { error: Error | string }) => void +export type OnError = (p: { error: string }) => void export type AbortRef = { current: (() => void) | null } diff --git a/src/vs/platform/void/electron-main/llmMessage/anthropic.ts b/src/vs/platform/void/electron-main/llmMessage/anthropic.ts index c79413d6..9e433bec 100644 --- a/src/vs/platform/void/electron-main/llmMessage/anthropic.ts +++ b/src/vs/platform/void/electron-main/llmMessage/anthropic.ts @@ -47,7 +47,7 @@ export const sendAnthropicMsg: SendLLMMessageFnTypeInternal = ({ messages, onTex onError({ error: 'Invalid API key.' }) } else { - onError({ error }) + onError({ error: error + '' }) } }) diff --git a/src/vs/platform/void/electron-main/llmMessage/gemini.ts b/src/vs/platform/void/electron-main/llmMessage/gemini.ts index dfd37e94..ebefa6d8 100644 --- a/src/vs/platform/void/electron-main/llmMessage/gemini.ts +++ b/src/vs/platform/void/electron-main/llmMessage/gemini.ts @@ -42,7 +42,7 @@ export const sendGeminiMsg: SendLLMMessageFnTypeInternal = async ({ messages, on onError({ error: 'Invalid API key.' }); } else { - onError({ error }); + onError({ error: error + '' }); } }) } diff --git a/src/vs/platform/void/electron-main/llmMessage/groq.ts b/src/vs/platform/void/electron-main/llmMessage/groq.ts index 6506c4ab..7fbc3459 100644 --- a/src/vs/platform/void/electron-main/llmMessage/groq.ts +++ b/src/vs/platform/void/electron-main/llmMessage/groq.ts @@ -35,7 +35,7 @@ export const sendGroqMsg: SendLLMMessageFnTypeInternal = async ({ messages, onTe onFinalMessage({ fullText }); }) .catch(error => { - onError({ error }); + onError({ error: error + '' }); }) diff --git a/src/vs/platform/void/electron-main/llmMessage/ollama.ts b/src/vs/platform/void/electron-main/llmMessage/ollama.ts index 92754949..4e5bf52d 100644 --- a/src/vs/platform/void/electron-main/llmMessage/ollama.ts +++ b/src/vs/platform/void/electron-main/llmMessage/ollama.ts @@ -1,4 +1,4 @@ -import { Ollama, ErrorResponse } from 'ollama'; +import { Ollama } from 'ollama'; import { SendLLMMessageFnTypeInternal } from './util'; import { parseMaxTokensStr } from './util.js'; @@ -30,14 +30,14 @@ export const sendOllamaMsg: SendLLMMessageFnTypeInternal = ({ messages, onText, }) // when error/fail .catch((error) => { - if (typeof error === 'object') { - const e = error.error as ErrorResponse['error'] - if (e) { - const name = error.name ?? 'Error' - onError({ error: `${name}: ${e}` }) - return; - } - } + // if (typeof error === 'object') { + // const e = error.error as ErrorResponse['error'] + // if (e) { + // const name = error.name ?? 'Error' + // onError({ error: `${name}: ${e}` }) + // return; + // } + // } onError({ error }) }) diff --git a/src/vs/platform/void/electron-main/llmMessage/openai.ts b/src/vs/platform/void/electron-main/llmMessage/openai.ts index 7f59e5ac..89f5f40c 100644 --- a/src/vs/platform/void/electron-main/llmMessage/openai.ts +++ b/src/vs/platform/void/electron-main/llmMessage/openai.ts @@ -56,7 +56,7 @@ export const sendOpenAIMsg: SendLLMMessageFnTypeInternal = ({ messages, onText, onError({ error: 'Invalid API key.' }); } else { - onError({ error }); + onError({ error: error + '' }); } }) diff --git a/src/vs/platform/void/electron-main/llmMessage/sendLLMMessage.ts b/src/vs/platform/void/electron-main/llmMessage/sendLLMMessage.ts index e6daf1cb..664dc1c8 100644 --- a/src/vs/platform/void/electron-main/llmMessage/sendLLMMessage.ts +++ b/src/vs/platform/void/electron-main/llmMessage/sendLLMMessage.ts @@ -87,9 +87,6 @@ export const sendLLMMessage = ({ case 'ollama': sendOllamaMsg({ messages, onText, onFinalMessage, onError, voidConfig, _setAborter, providerName }); break; - // case 'greptile': - // sendGreptileMsg({ messages, onText, onFinalMessage, onError, voidConfig, _setAborter, providerName }); - // break; case 'groq': sendGroqMsg({ messages, onText, onFinalMessage, onError, voidConfig, _setAborter, providerName }); break; @@ -100,7 +97,7 @@ export const sendLLMMessage = ({ } catch (error) { - if (error instanceof Error) { onError({ error }) } + if (error instanceof Error) { onError({ error: error + '' }) } else { onError({ error: `Unexpected Error in sendLLMMessage: ${error}` }); } // ; (_aborter as any)?.() // _didAbort = true diff --git a/src/vs/platform/void/electron-main/llmMessageChannel.ts b/src/vs/platform/void/electron-main/llmMessageChannel.ts index 1ca4ece8..8f8c301a 100644 --- a/src/vs/platform/void/electron-main/llmMessageChannel.ts +++ b/src/vs/platform/void/electron-main/llmMessageChannel.ts @@ -76,8 +76,8 @@ export class LLMMessageChannel implements IServerChannel { const mainThreadParams: SendLLMMMessageParams = { ...params, - onText: ({ newText, fullText }) => { console.log('sendLLM: firing onText'); this._onText.fire({ requestId, newText, fullText }); }, - onFinalMessage: ({ fullText }) => { console.log('sendLLM: firing finalMsg'); this._onFinalMessage.fire({ requestId, fullText }); }, + onText: ({ newText, fullText }) => { this._onText.fire({ requestId, newText, fullText }); }, + onFinalMessage: ({ fullText }) => { this._onFinalMessage.fire({ requestId, fullText }); }, onError: ({ error }) => { console.log('sendLLM: firing err'); this._onError.fire({ requestId, error }); }, abortRef: this._abortRefOfRequestId[requestId], } diff --git a/src/vs/workbench/contrib/void/browser/react/src/sidebar-tsx/ErrorDisplay.tsx b/src/vs/workbench/contrib/void/browser/react/src/sidebar-tsx/ErrorDisplay.tsx index af205dd3..62acbd7a 100644 --- a/src/vs/workbench/contrib/void/browser/react/src/sidebar-tsx/ErrorDisplay.tsx +++ b/src/vs/workbench/contrib/void/browser/react/src/sidebar-tsx/ErrorDisplay.tsx @@ -55,7 +55,7 @@ const getErrorDetails = (error: unknown) => { } - // Collect any additional properties from the e + // Collect any additional properties from e for (let prop of Object.getOwnPropertyNames(e).filter((prop) => !Object.keys(details).includes(prop))) details.additional[prop] = (e as any)[prop] diff --git a/src/vs/workbench/contrib/void/browser/react/src/util/ErrorDisplay.tsx b/src/vs/workbench/contrib/void/browser/react/src/util/ErrorDisplay.tsx deleted file mode 100644 index ab11ff18..00000000 --- a/src/vs/workbench/contrib/void/browser/react/src/util/ErrorDisplay.tsx +++ /dev/null @@ -1,161 +0,0 @@ -import React, { useState } from 'react'; -import { AlertCircle, ChevronDown, ChevronUp, X } from 'lucide-react'; - -import { getCmdKey } from '../../../getCmdKey.js'; - -// const opaqueMessage = `\ -// Unfortunately, Void can't see the full error. However, you should be able to find more details by pressing ${getCmdKey()}+Shift+P, typing "Toggle Developer Tools", and looking at the console.\n -// This error often means you have an incorrect API key. If you're self-hosting your own server, it might mean your CORS headers are off, and you should make sure your server's response has the header "Access-Control-Allow-Origins" set to "*", or at least allows "vscode-file://vscode-app".` -// if ((error instanceof Error) && (error.cause + '').includes('TypeError: Failed to fetch')) { -// e = error as any -// e['Void Team'] = opaqueMessage -// } - - -type Details = { - message: string, - name: string, - stack: string | null, - cause: string | null, - code: string | null, - additional: Record -} - -// Get detailed error information -const getErrorDetails = (error: unknown) => { - - let details: Details; - - let e: Error & { [other: string]: undefined | any } - - // If fetch() fails, it gives an opaque message. We add extra details to the error. - if (error instanceof Error) { - e = error - } - // sometimes error is an object but not an Error - else if (typeof error === 'object') { - e = new Error(`The server didn't give a very useful error message. More details below.`, { cause: JSON.stringify(error) }) - - } - else { - e = new Error(String(error)) - } - // console.log('error display', JSON.stringify(e)) - - const message = e.message && e.error ? - (e.message + ':\n' + e.error) - : e.message || e.error || JSON.stringify(error) - - details = { - name: e.name || 'Error', - message: message, - stack: null, // e.stack is ignored because it's ugly and not very useful - cause: e.cause ? String(e.cause) : null, - code: e.code || null, - additional: {} - } - - - // Collect any additional properties from the e - for (let prop of Object.getOwnPropertyNames(e).filter((prop) => !Object.keys(details).includes(prop))) - details.additional[prop] = (e as any)[prop] - - return details; -}; - - - -export const ErrorDisplay = ({ - error, - onDismiss = null, - showDismiss = true, - className = '' -}: { - error: Error | object | string, - onDismiss: (() => void) | null, - showDismiss?: boolean, - className?: string -}) => { - const [isExpanded, setIsExpanded] = useState(false); - - const details = getErrorDetails(error); - const hasDetails = details.cause || Object.keys(details.additional).length > 0; - - return ( -
- {/* Header */} -
-
- -
-

- {details.name} -

-

- {details.message} -

-
-
- -
- {hasDetails && ( - - )} - {showDismiss && onDismiss && ( - - )} -
-
- - {/* Expandable Details */} - {isExpanded && hasDetails && ( -
- {details.code && ( -
- Error Code: - {details.code} -
- )} - - {details.cause && ( -
- Cause: - {details.cause} -
- )} - - {Object.keys(details.additional).length > 0 && ( -
- Additional Information: - 
-								{Object.keys(details.additional).map(key => `${key}:\n${details.additional[key]}`).join('\n')}
-
-
- )} - {/* {details.stack && ( -
- Stack Trace: - 
-								{details.stack}
-
-
- )} */} -
- )} -
- ); -};