From 2c0c91d204545de600f4d7f08b25f4d38893050f Mon Sep 17 00:00:00 2001 From: sandorsett Date: Fri, 9 Jan 2026 00:34:01 +0200 Subject: [PATCH] Make chat participant/provider calls resilient; handle exceptions and return safe defaults (#286618) --- .../src/singlefolder-tests/chat.test.ts | 17 ++++ .../api/browser/mainThreadChatAgents2.ts | 25 ++++-- .../api/common/extHostChatAgents2.ts | 78 +++++++++++++------ .../chat/common/participants/chatAgents.ts | 31 +++++++- 4 files changed, 119 insertions(+), 32 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts index ff5b49d9b69a1..8fdaa76534c48 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/chat.test.ts @@ -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) { diff --git a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts index 1714bfacab9ca..d57f2fc6cf68b 100644 --- a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts +++ b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts @@ -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; + } }, }; diff --git a/src/vs/workbench/api/common/extHostChatAgents2.ts b/src/vs/workbench/api/common/extHostChatAgents2.ts index 31a52458824ee..ddecbf2e04f0d 100644 --- a/src/vs/workbench/api/common/extHostChatAgents2.ts +++ b/src/vs/workbench/api/common/extHostChatAgents2.ts @@ -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'; @@ -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 { @@ -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 { @@ -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; + } } } @@ -930,15 +947,20 @@ 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 { @@ -946,7 +968,12 @@ class ExtHostChatAgent { 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 { @@ -954,7 +981,12 @@ class ExtHostChatAgent { 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 { diff --git a/src/vs/workbench/contrib/chat/common/participants/chatAgents.ts b/src/vs/workbench/contrib/chat/common/participants/chatAgents.ts index 16d66aa198249..b0a9067f6d9cf 100644 --- a/src/vs/workbench/contrib/chat/common/participants/chatAgents.ts +++ b/src/vs/workbench/contrib/chat/common/participants/chatAgents.ts @@ -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'; @@ -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 { @@ -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 { @@ -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) { @@ -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; }