Properly handle errors thrown from renderer while invoking the chat a…

…gent If we throw out of ChatService, then the request can't be canceled and stays "generating" forever
microsoft · May 20, 2024 · c7defb2 · c7defb2
1 parent 9f83563
commit c7defb2
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 2 deletions.
diff --git 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<IChatAgentResult> {
 		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<IChatFollowup[]> {
 		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
@@ -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<ChatProviderInvokedEvent, ChatProviderInvokedClassification>('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
@@ -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: [  ]
+    }
+  ]
+}
diff --git 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' } });