Skip to content

Make chat participant/provider calls resilient; handle exceptions and… #286627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sandorsett wants to merge 1 commit into
base: main
Choose a base branch
from
+119 −32
Open

Make chat participant/provider calls resilient; handle exceptions and… #286627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,23 @@ suite('chat', () => {
assert.strictEqual(calls, 1);
});

test('title provider exceptions are handled gracefully', async () => {
const participant = chat.createChatParticipant('api-test.participant', (_request, _context, _progress, _token) => {
return { metadata: { key: 'value' } };
});
participant.titleProvider = {
provideChatTitle(_context, _token) {
throw new Error('boom');
}
};
disposables.push(participant);

await commands.executeCommand('workbench.action.chat.newChat');
// Should not throw or crash when provider throws
commands.executeCommand('workbench.action.chat.open', { query: '@participant /hello friend' });
await delay(500);
});

test('can access node-pty module', async function () {
// Required for copilot cli in chat extension.
if (env.uiKind === UIKind.Web) {
Expand Down
25 changes: 20 additions & 5 deletions src/vs/workbench/api/browser/mainThreadChatAgents2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,28 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
return [];
}

return this._proxy.$provideFollowups(request, handle, result, { history }, token);
try {
return await this._proxy.$provideFollowups(request, handle, result, { history }, token) ?? [];
} catch (err) {
this._logService.error(err);
return [];
}
},
provideChatTitle: (history, token) => {
return this._proxy.$provideChatTitle(handle, history, token);
provideChatTitle: async (history, token) => {
try {
return await this._proxy.$provideChatTitle(handle, history, token);
} catch (err) {
this._logService.error(err);
return undefined;
}
},
provideChatSummary: (history, token) => {
return this._proxy.$provideChatSummary(handle, history, token);
provideChatSummary: async (history, token) => {
try {
return await this._proxy.$provideChatSummary(handle, history, token);
} catch (err) {
this._logService.error(err);
return undefined;
}
},
};

Expand Down
78 changes: 55 additions & 23 deletions src/vs/workbench/api/common/extHostChatAgents2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { ChatAgentLocation } from '../../contrib/chat/common/constants.js';
import { checkProposedApiEnabled, isProposedApiEnabled } from '../../services/extensions/common/extensions.js';
import { Dto } from '../../services/extensions/common/proxyIdentifier.js';
import { ExtHostChatAgentsShape2, IChatAgentCompletionItem, IChatAgentHistoryEntryDto, IChatAgentProgressShape, IChatProgressDto, IChatSessionContextDto, IExtensionChatAgentMetadata, IMainContext, MainContext, MainThreadChatAgentsShape2 } from './extHost.protocol.js';
import { onUnexpectedError } from '../../../base/common/errors.js';
import { CommandsConverter, ExtHostCommands } from './extHostCommands.js';
import { ExtHostDiagnostics } from './extHostDiagnostics.js';
import { ExtHostDocuments } from './extHostDocuments.js';
Expand Down Expand Up @@ -772,18 +773,24 @@ export class ExtHostChatAgents2 extends Disposable implements ExtHostChatAgentsS
const convertedHistory = await this.prepareHistoryTurns(agent.extension, agent.id, context);

const ehResult = typeConvert.ChatAgentResult.to(result);
return (await agent.provideFollowups(ehResult, { history: convertedHistory }, token))
.filter(f => {
// The followup must refer to a participant that exists from the same extension
const isValid = !f.participant || Iterable.some(
this._agents.values(),
a => a.id === f.participant && ExtensionIdentifier.equals(a.extension.identifier, agent.extension.identifier));
if (!isValid) {
this._logService.warn(`[@${agent.id}] ChatFollowup refers to an unknown participant: ${f.participant}`);
}
return isValid;
})
.map(f => typeConvert.ChatFollowup.from(f, request));
try {
const followups = await agent.provideFollowups(ehResult, { history: convertedHistory }, token);
return (followups ?? [])
.filter(f => {
// The followup must refer to a participant that exists from the same extension
const isValid = !f.participant || Iterable.some(
this._agents.values(),
a => a.id === f.participant && ExtensionIdentifier.equals(a.extension.identifier, agent.extension.identifier));
if (!isValid) {
this._logService.warn(`[@${agent.id}] ChatFollowup refers to an unknown participant: ${f.participant}`);
}
return isValid;
})
.map(f => typeConvert.ChatFollowup.from(f, request));
} catch (err) {
onUnexpectedError(err);
return [];
}
}

$acceptFeedback(handle: number, result: IChatAgentResult, voteAction: IChatVoteAction): void {
Expand Down Expand Up @@ -854,7 +861,12 @@ export class ExtHostChatAgents2 extends Disposable implements ExtHostChatAgentsS
}

const history = await this.prepareHistoryTurns(agent.extension, agent.id, { history: context });
return await agent.provideTitle({ history }, token);
try {
return await agent.provideTitle({ history }, token) ?? undefined;
} catch (err) {
onUnexpectedError(err);
return undefined;
}
}

async $provideChatSummary(handle: number, context: IChatAgentHistoryEntryDto[], token: CancellationToken): Promise<string | undefined> {
Expand All @@ -864,7 +876,12 @@ export class ExtHostChatAgents2 extends Disposable implements ExtHostChatAgentsS
}

const history = await this.prepareHistoryTurns(agent.extension, agent.id, { history: context });
return await agent.provideSummary({ history }, token);
try {
return await agent.provideSummary({ history }, token) ?? undefined;
} catch (err) {
onUnexpectedError(err);
return undefined;
}
}
}

Expand Down Expand Up @@ -930,31 +947,46 @@ class ExtHostChatAgent {
return [];
}

const followups = await this._followupProvider.provideFollowups(result, context, token);
if (!followups) {
try {
const followups = await this._followupProvider.provideFollowups(result, context, token);
if (!followups) {
return [];
}
return followups
// Filter out "command followups" from older providers
.filter(f => !(f && 'commandId' in f))
// Filter out followups from older providers before 'message' changed to 'prompt'
.filter(f => !(f && 'message' in f));
} catch (err) {
onUnexpectedError(err);
return [];
}
return followups
// Filter out "command followups" from older providers
.filter(f => !(f && 'commandId' in f))
// Filter out followups from older providers before 'message' changed to 'prompt'
.filter(f => !(f && 'message' in f));
}

async provideTitle(context: vscode.ChatContext, token: CancellationToken): Promise<string | undefined> {
if (!this._titleProvider) {
return;
}

return await this._titleProvider.provideChatTitle(context, token) ?? undefined;
try {
return await this._titleProvider.provideChatTitle(context, token) ?? undefined;
} catch (err) {
onUnexpectedError(err);
return undefined;
}
}

async provideSummary(context: vscode.ChatContext, token: CancellationToken): Promise<string | undefined> {
if (!this._summarizer) {
return;
}

return await this._summarizer.provideChatSummary(context, token) ?? undefined;
try {
return await this._summarizer.provideChatSummary(context, token) ?? undefined;
} catch (err) {
onUnexpectedError(err);
return undefined;
}
}

get apiAgent(): vscode.ChatParticipant {
Expand Down
31 changes: 27 additions & 4 deletions src/vs/workbench/contrib/chat/common/participants/chatAgents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { createDecorator } from '../../../../../platform/instantiation/common/in
import { ILogService } from '../../../../../platform/log/common/log.js';
import { IProductService } from '../../../../../platform/product/common/productService.js';
import { asJson, IRequestService } from '../../../../../platform/request/common/request.js';
import { onUnexpectedError } from '../../../../../base/common/errors.js';
import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
import { ChatContextKeys } from '../actions/chatContextKeys.js';
import { IChatAgentEditedFileEvent, IChatProgressHistoryResponseContent, IChatRequestModeInstructions, IChatRequestVariableData, ISerializableChatAgentData } from '../model/chatModel.js';
Expand Down Expand Up @@ -501,7 +502,12 @@ export class ChatAgentService extends Disposable implements IChatAgentService {
return [];
}

return data.impl.provideFollowups(request, result, history, token);
try {
return await data.impl.provideFollowups(request, result, history, token) ?? [];
} catch (err) {
onUnexpectedError(err);
return [];
}
}

async getChatTitle(id: string, history: IChatAgentHistoryEntry[], token: CancellationToken): Promise<string | undefined> {
Expand All @@ -510,7 +516,12 @@ export class ChatAgentService extends Disposable implements IChatAgentService {
return undefined;
}

return data.impl.provideChatTitle(history, token);
try {
return await data.impl.provideChatTitle(history, token) ?? undefined;
} catch (err) {
onUnexpectedError(err);
return undefined;
}
}

async getChatSummary(id: string, history: IChatAgentHistoryEntry[], token: CancellationToken): Promise<string | undefined> {
Expand All @@ -519,7 +530,12 @@ export class ChatAgentService extends Disposable implements IChatAgentService {
return undefined;
}

return data.impl.provideChatSummary(history, token);
try {
return await data.impl.provideChatSummary(history, token) ?? undefined;
} catch (err) {
onUnexpectedError(err);
return undefined;
}
}

registerChatParticipantDetectionProvider(handle: number, provider: IChatParticipantDetectionProvider) {
Expand Down Expand Up @@ -550,7 +566,14 @@ export class ChatAgentService extends Disposable implements IChatAgentService {
return acc;
}, []);

const result = await provider.provideParticipantDetection(request, history, { ...options, participants }, token);
let result;
try {
result = await provider.provideParticipantDetection(request, history, { ...options, participants }, token);
} catch (err) {
onUnexpectedError(err);
return;
}

if (!result) {
return;
}
Expand Down