Skip to content

Commit

Permalink
Properly handle errors thrown from renderer while invoking the chat a…
Browse files Browse the repository at this point in the history
…gent (microsoft#213080)

* 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

* Update snapshot
  • Loading branch information
roblourens authored and mustard-mh committed May 22, 2024
1 parent 549bb94 commit 5db57b0
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/vs/workbench/contrib/chat/common/chatAgents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,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);
Expand All @@ -342,7 +342,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) {
Expand Down Expand Up @@ -527,5 +527,9 @@ export function reviveSerializedAgent(raw: ISerializableChatAgentData): IChatAge
agent.extensionDisplayName = '';
}

if (!('extensionId' in agent)) {
agent.extensionId = new ExtensionIdentifier('');
}

return revive(agent);
}
18 changes: 18 additions & 0 deletions src/vs/workbench/contrib/chat/common/chatServiceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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: [ ]
}
]
}
11 changes: 11 additions & 0 deletions src/vs/workbench/contrib/chat/test/common/chatService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' } });
Expand Down

0 comments on commit 5db57b0

Please sign in to comment.