Skip to content

Commit b60ce14

Browse files
author
sandorsett
committed
Make chat participant/provider calls resilient; handle exceptions and return safe defaults (#286618)
1 parent dea178a commit b60ce14

File tree

4 files changed

+119
-32
lines changed

4 files changed

+119
-32
lines changed

‎extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,23 @@ suite('chat', () => {
217217
assert.strictEqual(calls, 1);
218218
});
219219

220+
test('title provider exceptions are handled gracefully', async () => {
221+
const participant = chat.createChatParticipant('api-test.participant', (_request, _context, _progress, _token) => {
222+
return { metadata: { key: 'value' } };
223+
});
224+
participant.titleProvider = {
225+
provideChatTitle(_context, _token) {
226+
throw new Error('boom');
227+
}
228+
};
229+
disposables.push(participant);
230+
231+
await commands.executeCommand('workbench.action.chat.newChat');
232+
// Should not throw or crash when provider throws
233+
commands.executeCommand('workbench.action.chat.open', { query: '@participant /hello friend' });
234+
await delay(500);
235+
});
236+
220237
test('can access node-pty module', async function () {
221238
// Required for copilot cli in chat extension.
222239
if (env.uiKind === UIKind.Web) {

‎src/vs/workbench/api/browser/mainThreadChatAgents2.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,28 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA
194194
return [];
195195
}
196196

197-
return this._proxy.$provideFollowups(request, handle, result, { history }, token);
197+
try {
198+
return await this._proxy.$provideFollowups(request, handle, result, { history }, token) ?? [];
199+
} catch (err) {
200+
this._logService.error(err);
201+
return [];
202+
}
198203
},
199-
provideChatTitle: (history, token) => {
200-
return this._proxy.$provideChatTitle(handle, history, token);
204+
provideChatTitle: async (history, token) => {
205+
try {
206+
return await this._proxy.$provideChatTitle(handle, history, token);
207+
} catch (err) {
208+
this._logService.error(err);
209+
return undefined;
210+
}
201211
},
202-
provideChatSummary: (history, token) => {
203-
return this._proxy.$provideChatSummary(handle, history, token);
212+
provideChatSummary: async (history, token) => {
213+
try {
214+
return await this._proxy.$provideChatSummary(handle, history, token);
215+
} catch (err) {
216+
this._logService.error(err);
217+
return undefined;
218+
}
204219
},
205220
};
206221

‎src/vs/workbench/api/common/extHostChatAgents2.ts‎

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { ChatAgentLocation } from '../../contrib/chat/common/constants.js';
2828
import { checkProposedApiEnabled, isProposedApiEnabled } from '../../services/extensions/common/extensions.js';
2929
import { Dto } from '../../services/extensions/common/proxyIdentifier.js';
3030
import { ExtHostChatAgentsShape2, IChatAgentCompletionItem, IChatAgentHistoryEntryDto, IChatAgentProgressShape, IChatProgressDto, IChatSessionContextDto, IExtensionChatAgentMetadata, IMainContext, MainContext, MainThreadChatAgentsShape2 } from './extHost.protocol.js';
31+
import { onUnexpectedError } from '../../../base/common/errors.js';
3132
import { CommandsConverter, ExtHostCommands } from './extHostCommands.js';
3233
import { ExtHostDiagnostics } from './extHostDiagnostics.js';
3334
import { ExtHostDocuments } from './extHostDocuments.js';
@@ -772,18 +773,24 @@ export class ExtHostChatAgents2 extends Disposable implements ExtHostChatAgentsS
772773
const convertedHistory = await this.prepareHistoryTurns(agent.extension, agent.id, context);
773774

774775
const ehResult = typeConvert.ChatAgentResult.to(result);
775-
return (await agent.provideFollowups(ehResult, { history: convertedHistory }, token))
776-
.filter(f => {
777-
// The followup must refer to a participant that exists from the same extension
778-
const isValid = !f.participant || Iterable.some(
779-
this._agents.values(),
780-
a => a.id === f.participant && ExtensionIdentifier.equals(a.extension.identifier, agent.extension.identifier));
781-
if (!isValid) {
782-
this._logService.warn(`[@${agent.id}] ChatFollowup refers to an unknown participant: ${f.participant}`);
783-
}
784-
return isValid;
785-
})
786-
.map(f => typeConvert.ChatFollowup.from(f, request));
776+
try {
777+
const followups = await agent.provideFollowups(ehResult, { history: convertedHistory }, token);
778+
return (followups ?? [])
779+
.filter(f => {
780+
// The followup must refer to a participant that exists from the same extension
781+
const isValid = !f.participant || Iterable.some(
782+
this._agents.values(),
783+
a => a.id === f.participant && ExtensionIdentifier.equals(a.extension.identifier, agent.extension.identifier));
784+
if (!isValid) {
785+
this._logService.warn(`[@${agent.id}] ChatFollowup refers to an unknown participant: ${f.participant}`);
786+
}
787+
return isValid;
788+
})
789+
.map(f => typeConvert.ChatFollowup.from(f, request));
790+
} catch (err) {
791+
onUnexpectedError(err);
792+
return [];
793+
}
787794
}
788795

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

856863
const history = await this.prepareHistoryTurns(agent.extension, agent.id, { history: context });
857-
return await agent.provideTitle({ history }, token);
864+
try {
865+
return await agent.provideTitle({ history }, token) ?? undefined;
866+
} catch (err) {
867+
onUnexpectedError(err);
868+
return undefined;
869+
}
858870
}
859871

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

866878
const history = await this.prepareHistoryTurns(agent.extension, agent.id, { history: context });
867-
return await agent.provideSummary({ history }, token);
879+
try {
880+
return await agent.provideSummary({ history }, token) ?? undefined;
881+
} catch (err) {
882+
onUnexpectedError(err);
883+
return undefined;
884+
}
868885
}
869886
}
870887

@@ -930,31 +947,46 @@ class ExtHostChatAgent {
930947
return [];
931948
}
932949

933-
const followups = await this._followupProvider.provideFollowups(result, context, token);
934-
if (!followups) {
950+
try {
951+
const followups = await this._followupProvider.provideFollowups(result, context, token);
952+
if (!followups) {
953+
return [];
954+
}
955+
return followups
956+
// Filter out "command followups" from older providers
957+
.filter(f => !(f && 'commandId' in f))
958+
// Filter out followups from older providers before 'message' changed to 'prompt'
959+
.filter(f => !(f && 'message' in f));
960+
} catch (err) {
961+
onUnexpectedError(err);
935962
return [];
936963
}
937-
return followups
938-
// Filter out "command followups" from older providers
939-
.filter(f => !(f && 'commandId' in f))
940-
// Filter out followups from older providers before 'message' changed to 'prompt'
941-
.filter(f => !(f && 'message' in f));
942964
}
943965

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

949-
return await this._titleProvider.provideChatTitle(context, token) ?? undefined;
971+
try {
972+
return await this._titleProvider.provideChatTitle(context, token) ?? undefined;
973+
} catch (err) {
974+
onUnexpectedError(err);
975+
return undefined;
976+
}
950977
}
951978

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

957-
return await this._summarizer.provideChatSummary(context, token) ?? undefined;
984+
try {
985+
return await this._summarizer.provideChatSummary(context, token) ?? undefined;
986+
} catch (err) {
987+
onUnexpectedError(err);
988+
return undefined;
989+
}
958990
}
959991

960992
get apiAgent(): vscode.ChatParticipant {

‎src/vs/workbench/contrib/chat/common/participants/chatAgents.ts‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { createDecorator } from '../../../../../platform/instantiation/common/in
2323
import { ILogService } from '../../../../../platform/log/common/log.js';
2424
import { IProductService } from '../../../../../platform/product/common/productService.js';
2525
import { asJson, IRequestService } from '../../../../../platform/request/common/request.js';
26+
import { onUnexpectedError } from '../../../../../base/common/errors.js';
2627
import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
2728
import { ChatContextKeys } from '../actions/chatContextKeys.js';
2829
import { IChatAgentEditedFileEvent, IChatProgressHistoryResponseContent, IChatRequestModeInstructions, IChatRequestVariableData, ISerializableChatAgentData } from '../model/chatModel.js';
@@ -501,7 +502,12 @@ export class ChatAgentService extends Disposable implements IChatAgentService {
501502
return [];
502503
}
503504

504-
return data.impl.provideFollowups(request, result, history, token);
505+
try {
506+
return await data.impl.provideFollowups(request, result, history, token) ?? [];
507+
} catch (err) {
508+
onUnexpectedError(err);
509+
return [];
510+
}
505511
}
506512

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

513-
return data.impl.provideChatTitle(history, token);
519+
try {
520+
return await data.impl.provideChatTitle(history, token) ?? undefined;
521+
} catch (err) {
522+
onUnexpectedError(err);
523+
return undefined;
524+
}
514525
}
515526

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

522-
return data.impl.provideChatSummary(history, token);
533+
try {
534+
return await data.impl.provideChatSummary(history, token) ?? undefined;
535+
} catch (err) {
536+
onUnexpectedError(err);
537+
return undefined;
538+
}
523539
}
524540

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

553-
const result = await provider.provideParticipantDetection(request, history, { ...options, participants }, token);
569+
let result;
570+
try {
571+
result = await provider.provideParticipantDetection(request, history, { ...options, participants }, token);
572+
} catch (err) {
573+
onUnexpectedError(err);
574+
return;
575+
}
576+
554577
if (!result) {
555578
return;
556579
}

0 commit comments

Comments
 (0)