From c7defb25e2a2a041f01902764b7c022ae9635a50 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 20 May 2024 09:57:11 -0700 Subject: [PATCH 1/2] Properly handle errors thrown from renderer while invoking the chat agent If we throw out of ChatService, then the request can't be canceled and stays "generating" forever --- .../contrib/chat/common/chatAgents.ts | 8 +- .../contrib/chat/common/chatServiceImpl.ts | 18 +++++ .../ChatService_sendRequest_fails.0.snap | 81 +++++++++++++++++++ .../chat/test/common/chatService.test.ts | 11 +++ 4 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatService_sendRequest_fails.0.snap diff --git a/src/vs/workbench/contrib/chat/common/chatAgents.ts b/src/vs/workbench/contrib/chat/common/chatAgents.ts index 3fc918226a511..e89a7efe8be0d 100644 --- a/src/vs/workbench/contrib/chat/common/chatAgents.ts +++ b/src/vs/workbench/contrib/chat/common/chatAgents.ts @@ -309,7 +309,7 @@ export class ChatAgentService implements IChatAgentService { async invokeAgent(id: string, request: IChatAgentRequest, progress: (part: IChatProgress) => void, history: IChatAgentHistoryEntry[], token: CancellationToken): Promise { const data = this._getAgentEntry(id); if (!data?.impl) { - throw new Error(`No activated agent with id ${id}`); + throw new Error(`No activated agent with id "${id}"`); } return await data.impl.invoke(request, progress, history, token); @@ -318,7 +318,7 @@ export class ChatAgentService implements IChatAgentService { async getFollowups(id: string, request: IChatAgentRequest, result: IChatAgentResult, history: IChatAgentHistoryEntry[], token: CancellationToken): Promise { const data = this._getAgentEntry(id); if (!data?.impl) { - throw new Error(`No activated agent with id ${id}`); + throw new Error(`No activated agent with id "${id}"`); } if (!data.impl?.provideFollowups) { @@ -503,5 +503,9 @@ export function reviveSerializedAgent(raw: ISerializableChatAgentData): IChatAge agent.extensionDisplayName = ''; } + if (!('extensionId' in agent)) { + agent.extensionId = new ExtensionIdentifier(''); + } + return revive(agent); } diff --git a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts index a106ee694356b..296677d5bf386 100644 --- a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts @@ -6,6 +6,7 @@ import { coalesce } from 'vs/base/common/arrays'; import { DeferredPromise } from 'vs/base/common/async'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; +import { toErrorMessage } from 'vs/base/common/errorMessage'; import { ErrorNoTelemetry } from 'vs/base/common/errors'; import { Emitter, Event } from 'vs/base/common/event'; import { MarkdownString } from 'vs/base/common/htmlContent'; @@ -656,6 +657,23 @@ export class ChatService extends Disposable implements IChatService { }); } } + } catch (err) { + const result = 'error'; + this.telemetryService.publicLog2('interactiveSessionProviderInvoked', { + timeToFirstProgress: undefined, + totalTime: undefined, + result, + requestType, + agent: agentPart?.agent.id ?? '', + slashCommand: agentSlashCommandPart ? agentSlashCommandPart.command.name : commandPart?.slashCommand.command, + chatSessionId: model.sessionId, + location + }); + const rawResult: IChatAgentResult = { errorDetails: { message: err.message } }; + model.setResponse(request, rawResult); + completeResponseCreated(); + this.trace('sendRequest', `Error while handling request: ${toErrorMessage(err)}`); + model.completeResponse(request); } finally { listener.dispose(); } diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatService_sendRequest_fails.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatService_sendRequest_fails.0.snap new file mode 100644 index 0000000000000..305767c9feb77 --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatService_sendRequest_fails.0.snap @@ -0,0 +1,81 @@ +{ + requesterUsername: "test", + requesterAvatarIconUri: undefined, + responderUsername: "", + responderAvatarIconUri: undefined, + initialLocation: "panel", + welcomeMessage: undefined, + requests: [ + { + message: { + parts: [ + { + range: { + start: 0, + endExclusive: 28 + }, + editorRange: { + startLineNumber: 1, + startColumn: 1, + endLineNumber: 1, + endColumn: 29 + }, + agent: { + name: "ChatProviderWithUsedContext", + id: "ChatProviderWithUsedContext", + extensionId: { + value: "nullExtensionDescription", + _lower: "nullextensiondescription" + }, + extensionPublisherId: "", + publisherDisplayName: "", + extensionDisplayName: "", + locations: [ "panel" ], + metadata: { }, + slashCommands: [ ] + }, + kind: "agent" + }, + { + range: { + start: 28, + endExclusive: 41 + }, + editorRange: { + startLineNumber: 1, + startColumn: 29, + endLineNumber: 1, + endColumn: 42 + }, + text: " test request", + kind: "text" + } + ], + text: "@ChatProviderWithUsedContext test request" + }, + variableData: { variables: [ ] }, + response: [ ], + result: { errorDetails: { message: "No activated agent with id ChatProviderWithUsedContext" } }, + followups: undefined, + isCanceled: false, + vote: undefined, + agent: { + name: "ChatProviderWithUsedContext", + id: "ChatProviderWithUsedContext", + extensionId: { + value: "nullExtensionDescription", + _lower: "nullextensiondescription" + }, + extensionPublisherId: "", + publisherDisplayName: "", + extensionDisplayName: "", + locations: [ "panel" ], + metadata: { }, + slashCommands: [ ] + }, + slashCommand: undefined, + usedContext: undefined, + contentReferences: [ ] + } + ] +} \ No newline at end of file diff --git a/src/vs/workbench/contrib/chat/test/common/chatService.test.ts b/src/vs/workbench/contrib/chat/test/common/chatService.test.ts index 513f786bb59a0..1e9ad196f8b68 100644 --- a/src/vs/workbench/contrib/chat/test/common/chatService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/chatService.test.ts @@ -130,6 +130,17 @@ suite('ChatService', () => { assert.strictEqual(model.getRequests()[0].response?.response.asString(), 'test response'); }); + test('sendRequest fails', async () => { + const testService = testDisposables.add(instantiationService.createInstance(ChatService)); + + const model = testDisposables.add(testService.startSession(ChatAgentLocation.Panel, CancellationToken.None)); + const response = await testService.sendRequest(model.sessionId, `@${chatAgentWithUsedContextId} test request`); + assert(response); + await response.responseCompletePromise; + + await assertSnapshot(model.toExport()); + }); + test('can serialize', async () => { testDisposables.add(chatAgentService.registerAgentImplementation(chatAgentWithUsedContextId, chatAgentWithUsedContext)); chatAgentService.updateAgent(chatAgentWithUsedContextId, { requester: { name: 'test' } }); From b878f977324eea59b897571c24d0ab3af6ad9456 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 21 May 2024 14:35:19 -0700 Subject: [PATCH 2/2] Update snapshot --- .../common/__snapshots__/ChatService_sendRequest_fails.0.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatService_sendRequest_fails.0.snap b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatService_sendRequest_fails.0.snap index 305767c9feb77..a749780f18342 100644 --- a/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatService_sendRequest_fails.0.snap +++ b/src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatService_sendRequest_fails.0.snap @@ -55,7 +55,7 @@ }, variableData: { variables: [ ] }, response: [ ], - result: { errorDetails: { message: "No activated agent with id ChatProviderWithUsedContext" } }, + result: { errorDetails: { message: "No activated agent with id \"ChatProviderWithUsedContext\"" } }, followups: undefined, isCanceled: false, vote: undefined,