Add errorDetails to interactive session responses. (#176457)

* Add errorDetails to interactive session responses. Also avoid calling updateElementHeight during a renderElement * Fix hygiene
2026-04-24 10:38:59 +01:00 · 2023-03-07 22:30:46 -05:00
parent 40ad2f338c
commit 7114851c4a
9 changed files with 118 additions and 56 deletions
--- a/src/vs/workbench/api/common/extHost.protocol.ts
+++ b/src/vs/workbench/api/common/extHost.protocol.ts
@@ -51,7 +51,7 @@ import { SaveReason } from 'vs/workbench/common/editor';
 import { IRevealOptions, ITreeItem, IViewBadge } from 'vs/workbench/common/views';
 import { CallHierarchyItem } from 'vs/workbench/contrib/callHierarchy/common/callHierarchy';
 import { DebugConfigurationProviderTriggerKind, IAdapterDescriptor, IConfig, IDebugSessionReplMode } from 'vs/workbench/contrib/debug/common/debug';
-import { IInteractiveSessionResponseCommandFollowup } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionModel';
+import { IInteractiveResponseErrorDetails, IInteractiveSessionResponseCommandFollowup } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionModel';
 import * as notebookCommon from 'vs/workbench/contrib/notebook/common/notebookCommon';
 import { CellExecutionUpdateType } from 'vs/workbench/contrib/notebook/common/notebookExecutionService';
 import { ICellExecutionComplete, ICellExecutionStateUpdate } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
@@ -1102,6 +1102,7 @@ export interface IInteractiveRequestDto {
 export interface IInteractiveResponseDto {
 	followups?: string[];
 	commandFollowups?: IInteractiveSessionResponseCommandFollowup[];
+	errorDetails?: IInteractiveResponseErrorDetails;
 }

 export interface IInteractiveResponseProgressDto {
--- a/src/vs/workbench/api/common/extHostInteractiveSession.ts
+++ b/src/vs/workbench/api/common/extHostInteractiveSession.ts
@@ -6,6 +6,7 @@
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { toDisposable } from 'vs/base/common/lifecycle';
 import { withNullAsUndefined } from 'vs/base/common/types';
+import { localize } from 'vs/nls';
 import { IRelaxedExtensionDescription } from 'vs/platform/extensions/common/extensions';
 import { ILogService } from 'vs/platform/log/common/log';
 import { ExtHostInteractiveSessionShape, IInteractiveRequestDto, IInteractiveResponseDto, IInteractiveSessionDto, IMainContext, MainContext, MainThreadInteractiveSessionShape } from 'vs/workbench/api/common/extHost.protocol';
@@ -33,7 +34,7 @@ export class ExtHostInteractiveSession implements ExtHostInteractiveSessionShape

 	constructor(
 		mainContext: IMainContext,
-		_logService: ILogService
+		private readonly logService: ILogService
 	) {
 		this._proxy = mainContext.getProxy(MainContext.MainThreadInteractiveSession);
 	}
@@ -147,20 +148,30 @@ export class ExtHostInteractiveSession implements ExtHostInteractiveSessionShape
 			const progressObj: vscode.Progress<vscode.InteractiveProgress> = {
 				report: (progress: vscode.InteractiveProgress) => this._proxy.$acceptInteractiveResponseProgress(handle, sessionId, { responsePart: progress.content })
 			};
-			const res = await entry.provider.provideResponseWithProgress(requestObj, progressObj, token);
-			if (realSession.saveState) {
-				const newState = realSession.saveState();
-				this._proxy.$acceptInteractiveSessionState(sessionId, newState);
+			let result: vscode.InteractiveResponseForProgress | undefined | null;
+			try {
+				result = await entry.provider.provideResponseWithProgress(requestObj, progressObj, token);
+				if (!result) {
+					result = { errorDetails: { message: localize('emptyResponse', "Provider returned null response") } };
+				}
+			} catch (err) {
+				result = { errorDetails: { message: localize('errorResponse', "Error from provider: {0}", err.message) } };
+				this.logService.error(err);
 			}

-			if (!res) {
-				return;
+			try {
+				if (realSession.saveState) {
+					const newState = realSession.saveState();
+					this._proxy.$acceptInteractiveSessionState(sessionId, newState);
+				}
+			} catch (err) {
+				this.logService.warn(err);
 			}

-			return { followups: res.followups, commandFollowups: res.commands };
+			return { followups: result.followups, commandFollowups: result.commands, errorDetails: result.errorDetails };
 		}

-		throw new Error('provider must implement either provideResponse or provideResponseWithProgress');
+		throw new Error('Provider must implement either provideResponse or provideResponseWithProgress');
 	}

 	$releaseSession(sessionId: number) {
--- a/src/vs/workbench/contrib/interactiveSession/browser/interactiveSessionListRenderer.ts
+++ b/src/vs/workbench/contrib/interactiveSession/browser/interactiveSessionListRenderer.ts
@@ -5,6 +5,7 @@

 import * as dom from 'vs/base/browser/dom';
 import { Button } from 'vs/base/browser/ui/button/button';
+import { renderIcon } from 'vs/base/browser/ui/iconLabel/iconLabels';
 import { IListVirtualDelegate } from 'vs/base/browser/ui/list/list';
 import { IListAccessibilityProvider } from 'vs/base/browser/ui/list/listWidget';
 import { ITreeNode, ITreeRenderer } from 'vs/base/browser/ui/tree/tree';
@@ -60,7 +61,7 @@ interface IItemHeightChangeParams {
 	height: number;
 }

-const forceVerboseLayoutTracing = true;
+const forceVerboseLayoutTracing = false;

 export class InteractiveListItemRenderer extends Disposable implements ITreeRenderer<InteractiveTreeItem, FuzzyScore, IInteractiveListItemTemplate> {
 	static readonly cursorCharacter = '\u258c';
@@ -176,12 +177,12 @@ export class InteractiveListItemRenderer extends Disposable implements ITreeRend
 			this.traceLayout('renderElement', `start progressive render ${kind}, index=${index}`);
 			const progressiveRenderingDisposables = templateData.elementDisposables.add(new DisposableStore());
 			const timer = templateData.elementDisposables.add(new IntervalTimer());
-			const runProgressiveRender = () => {
-				if (this.doNextProgressiveRender(element, index, templateData, progressiveRenderingDisposables)) {
+			const runProgressiveRender = (initial?: boolean) => {
+				if (this.doNextProgressiveRender(element, index, templateData, !!initial, progressiveRenderingDisposables)) {
 					timer.cancel();
 				}
 			};
-			runProgressiveRender();
+			runProgressiveRender(true);
 			timer.cancelAndSet(runProgressiveRender, 100);
 		} else if (isResponseVM(element)) {
 			this.basicRenderElement(element.response.value, element, index, templateData);
@@ -196,6 +197,11 @@ export class InteractiveListItemRenderer extends Disposable implements ITreeRend
 		templateData.value.appendChild(result.element);
 		templateData.elementDisposables.add(result);

+		if (isResponseVM(element) && element.errorDetails) {
+			const errorDetails = dom.append(templateData.value, $('.interactive-response-error-details', undefined, renderIcon(Codicon.error)));
+			errorDetails.appendChild($('span', undefined, element.errorDetails.message));
+		}
+
 		if (isResponseVM(element) && index === this.delegate.getListLength() - 1) {
 			const followupsContainer = dom.append(templateData.value, $('.interactive-response-followups'));
 			const followups = element.commandFollowups ?? element.followups ?? [];
@@ -217,36 +223,43 @@ export class InteractiveListItemRenderer extends Disposable implements ITreeRend
 		}
 	}

-	private doNextProgressiveRender(element: IInteractiveResponseViewModel, index: number, templateData: IInteractiveListItemTemplate, disposables: DisposableStore): boolean {
+	private doNextProgressiveRender(element: IInteractiveResponseViewModel, index: number, templateData: IInteractiveListItemTemplate, isInRenderElement: boolean, disposables: DisposableStore): boolean {
 		disposables.clear();
-		const toRender = this.getProgressiveMarkdownToRender(element);
-		if (toRender) {
-			const isFullyRendered = element.renderData?.isFullyRendered;
-			if (isFullyRendered) {
-				this.traceLayout('runProgressiveRender', `end progressive render, index=${index}`);
-				if (element.isComplete) {
-					this.traceLayout('runProgressiveRender', `and disposing renderData, response is complete, index=${index}`);
-					element.renderData = undefined;
-				} else {
-					this.traceLayout('runProgressiveRender', `Rendered all available words, but model is not complete.`);
-				}
-				disposables.clear();
-				this.basicRenderElement(element.response.value, element, index, templateData);
-			} else {
-				const plusCursor = toRender.match(/```.*$/) ? toRender + `\n${InteractiveListItemRenderer.cursorCharacter}` : toRender + ` ${InteractiveListItemRenderer.cursorCharacter}`;
-				const result = this.renderMarkdown(element, index, new MarkdownString(plusCursor), disposables, templateData, true);
-				dom.clearNode(templateData.value);
-				templateData.value.appendChild(result.element);
-				disposables.add(result);
-			}

-			const height = templateData.rowContainer.offsetHeight;
-			element.currentRenderedHeight = height;
-			this._onDidChangeItemHeight.fire({ element, height: templateData.rowContainer.offsetHeight });
-			return !!isFullyRendered;
+		// TODO- this method has the side effect of updating element.renderData
+		const toRender = this.getProgressiveMarkdownToRender(element);
+		const isFullyRendered = element.renderData?.isFullyRendered;
+		if (isFullyRendered) {
+			// We've reached the end of the available content, so do a normal render
+			this.traceLayout('runProgressiveRender', `end progressive render, index=${index}`);
+			if (element.isComplete) {
+				this.traceLayout('runProgressiveRender', `and disposing renderData, response is complete, index=${index}`);
+				element.renderData = undefined;
+			} else {
+				this.traceLayout('runProgressiveRender', `Rendered all available words, but model is not complete.`);
+			}
+			disposables.clear();
+			this.basicRenderElement(element.response.value, element, index, templateData);
+		} else if (toRender) {
+			// Doing the progressive render
+			const plusCursor = toRender.match(/```.*$/) ? toRender + `\n${InteractiveListItemRenderer.cursorCharacter}` : toRender + ` ${InteractiveListItemRenderer.cursorCharacter}`;
+			const result = this.renderMarkdown(element, index, new MarkdownString(plusCursor), disposables, templateData, true);
+			dom.clearNode(templateData.value);
+			templateData.value.appendChild(result.element);
+			disposables.add(result);
+		} else {
+			// Nothing new to render, not done, keep waiting
+			return false;
 		}

-		return false;
+		// Some render happened - update the height
+		const height = templateData.rowContainer.offsetHeight;
+		element.currentRenderedHeight = height;
+		if (!isInRenderElement) {
+			this._onDidChangeItemHeight.fire({ element, height: templateData.rowContainer.offsetHeight });
+		}
+
+		return !!isFullyRendered;
 	}

 	private renderMarkdown(element: InteractiveTreeItem, index: number, markdown: IMarkdownString, disposables: DisposableStore, templateData: IInteractiveListItemTemplate, fillInIncompleteTokens = false): IMarkdownRenderResult {
--- a/src/vs/workbench/contrib/interactiveSession/browser/media/interactiveSession.css
+++ b/src/vs/workbench/contrib/interactiveSession/browser/media/interactiveSession.css
@@ -184,3 +184,13 @@
 	font-size: 11px;
 	font-weight: 600;
 }
+
+.interactive-session .interactive-response .interactive-response-error-details {
+	display: flex;
+	align-items: center;
+	gap: 6px;
+}
+
+.interactive-session .interactive-response .interactive-response-error-details .codicon {
+	color: var(--vscode-errorForeground);
+}
--- a/src/vs/workbench/contrib/interactiveSession/common/interactiveSessionModel.ts
+++ b/src/vs/workbench/contrib/interactiveSession/common/interactiveSessionModel.ts
@@ -8,7 +8,7 @@ import { IMarkdownString, MarkdownString } from 'vs/base/common/htmlContent';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { URI } from 'vs/base/common/uri';
 import { ILogService } from 'vs/platform/log/common/log';
-import { IInteractiveSession } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionService';
+import { IInteractiveResponse, IInteractiveSession } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionService';

 export interface IInteractiveRequestModel {
 	readonly id: string;
@@ -24,6 +24,11 @@ export interface IInteractiveSessionResponseCommandFollowup {
 	title: string; // supports codicon strings
 }

+export interface IInteractiveResponseErrorDetails {
+	message: string;
+	responseIsIncomplete?: boolean;
+}
+
 export interface IInteractiveResponseModel {
 	readonly onDidChange: Event<void>;
 	readonly id: string;
@@ -33,6 +38,7 @@ export interface IInteractiveResponseModel {
 	readonly isComplete: boolean;
 	readonly followups?: string[];
 	readonly commandFollowups?: IInteractiveSessionResponseCommandFollowup[];
+	readonly errorDetails?: IInteractiveResponseErrorDetails;
 }

 export function isRequest(item: unknown): item is IInteractiveRequestModel {
@@ -89,11 +95,17 @@ export class InteractiveResponseModel extends Disposable implements IInteractive
 		return this._response;
 	}

-	constructor(response: IMarkdownString, public readonly username: string, public readonly avatarIconUri?: URI, isComplete: boolean = false, followups?: string[]) {
+	private _errorDetails: IInteractiveResponseErrorDetails | undefined;
+	public get errorDetails(): IInteractiveResponseErrorDetails | undefined {
+		return this._errorDetails;
+	}
+
+	constructor(response: IMarkdownString, public readonly username: string, public readonly avatarIconUri?: URI, isComplete: boolean = false, errorDetails?: IInteractiveResponseErrorDetails, followups?: string[]) {
 		super();
 		this._response = response;
 		this._isComplete = isComplete;
 		this._followups = followups;
+		this._errorDetails = errorDetails;
 		this._id = 'response_' + InteractiveResponseModel.nextId++;
 	}

@@ -102,10 +114,11 @@ export class InteractiveResponseModel extends Disposable implements IInteractive
 		this._onDidChange.fire();
 	}

-	complete(followups: string[] | undefined, commandFollowups: IInteractiveSessionResponseCommandFollowup[] | undefined): void {
+	complete(followups: string[] | undefined, commandFollowups: IInteractiveSessionResponseCommandFollowup[] | undefined, errorDetails?: IInteractiveResponseErrorDetails): void {
 		this._isComplete = true;
 		this._followups = followups;
 		this._commandFollowups = commandFollowups;
+		this._errorDetails = errorDetails;
 		this._onDidChange.fire();
 	}
 }
@@ -125,6 +138,7 @@ export interface ISerializableInteractiveSessionsData {
 export interface ISerializableInteractiveSessionRequestData {
 	message: string;
 	response: string | undefined;
+	responseErrorDetails: IInteractiveResponseErrorDetails | undefined;
 }

 export interface ISerializableInteractiveSessionData {
@@ -181,10 +195,10 @@ export class InteractiveSessionModel extends Disposable implements IInteractiveS
 			return [];
 		}

-		return requests.map((r: ISerializableInteractiveSessionRequestData) => {
-			const request = new InteractiveRequestModel(r.message, this.session.requesterUsername, this.session.requesterAvatarIconUri);
-			if (r.response) {
-				request.response = new InteractiveResponseModel(new MarkdownString(r.response), this.session.responderUsername, this.session.responderAvatarIconUri, true);
+		return requests.map((raw: ISerializableInteractiveSessionRequestData) => {
+			const request = new InteractiveRequestModel(raw.message, this.session.requesterUsername, this.session.requesterAvatarIconUri);
+			if (raw.response || raw.responseErrorDetails) {
+				request.response = new InteractiveResponseModel(new MarkdownString(raw.response), this.session.responderUsername, this.session.responderAvatarIconUri, true, raw.responseErrorDetails);
 			}
 			return request;
 		});
@@ -224,8 +238,8 @@ export class InteractiveSessionModel extends Disposable implements IInteractiveS
 		}
 	}

-	completeResponse(request: InteractiveRequestModel, followups?: string[], commandFollowups?: IInteractiveSessionResponseCommandFollowup[]): void {
-		request.response!.complete(followups, commandFollowups);
+	completeResponse(request: InteractiveRequestModel, response: IInteractiveResponse): void {
+		request.response!.complete(response.followups, response.commandFollowups, response.errorDetails);
 	}

 	setResponse(request: InteractiveRequestModel, response: InteractiveResponseModel): void {
@@ -239,6 +253,7 @@ export class InteractiveSessionModel extends Disposable implements IInteractiveS
 				return {
 					message: r.message,
 					response: r.response ? r.response.response.value : undefined,
+					responseErrorDetails: r.response?.errorDetails
 				};
 			}),
 			providerId: this.providerId,
--- a/src/vs/workbench/contrib/interactiveSession/common/interactiveSessionService.ts
+++ b/src/vs/workbench/contrib/interactiveSession/common/interactiveSessionService.ts
@@ -8,7 +8,7 @@ import { IDisposable } from 'vs/base/common/lifecycle';
 import { URI } from 'vs/base/common/uri';
 import { ProviderResult } from 'vs/editor/common/languages';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
-import { IInteractiveSessionResponseCommandFollowup, InteractiveSessionModel } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionModel';
+import { IInteractiveResponseErrorDetails, IInteractiveSessionResponseCommandFollowup, InteractiveSessionModel } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionModel';

 export interface IInteractiveSession {
 	id: number;
@@ -28,6 +28,7 @@ export interface IInteractiveResponse {
 	session: IInteractiveSession;
 	followups?: string[];
 	commandFollowups?: IInteractiveSessionResponseCommandFollowup[];
+	errorDetails?: IInteractiveResponseErrorDetails;
 }

 export interface IInteractiveProgress {
--- a/src/vs/workbench/contrib/interactiveSession/common/interactiveSessionServiceImpl.ts
+++ b/src/vs/workbench/contrib/interactiveSession/common/interactiveSessionServiceImpl.ts
@@ -8,6 +8,7 @@ import { groupBy } from 'vs/base/common/collections';
 import { Iterable } from 'vs/base/common/iterator';
 import { Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
 import { withNullAsUndefined } from 'vs/base/common/types';
+import { localize } from 'vs/nls';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { ILogService } from 'vs/platform/log/common/log';
 import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
@@ -125,7 +126,6 @@ export class InteractiveSessionService extends Disposable implements IInteractiv
 			return false;
 		}

-		// TODO log failures, add dummy response with error message
 		const _sendRequest = async (): Promise<void> => {
 			try {
 				this._pendingRequestSessions.add(sessionId);
@@ -134,13 +134,13 @@ export class InteractiveSessionService extends Disposable implements IInteractiv
 					this.trace('sendRequest', `Provider returned progress for session ${sessionId}, ${progress.responsePart.length} chars`);
 					model.mergeResponseContent(request, progress.responsePart);
 				};
-				const rawResponse = await provider.provideReply({ session: model.session, message }, progressCallback, token);
+				let rawResponse = await provider.provideReply({ session: model.session, message }, progressCallback, token);
 				if (!rawResponse) {
 					this.trace('sendRequest', `Provider returned no response for session ${sessionId}`);
-					return;
+					rawResponse = { session: model.session, errorDetails: { message: localize('emptyResponse', "Provider returned null response") } };
 				}

-				model.completeResponse(request, rawResponse.followups, rawResponse.commandFollowups);
+				model.completeResponse(request, rawResponse);
 				this.trace('sendRequest', `Provider returned response for session ${sessionId} with ${rawResponse.followups} followups`);
 			} finally {
 				this._pendingRequestSessions.delete(sessionId);
--- a/src/vs/workbench/contrib/interactiveSession/common/interactiveSessionViewModel.ts
+++ b/src/vs/workbench/contrib/interactiveSession/common/interactiveSessionViewModel.ts
@@ -9,7 +9,7 @@ import { Disposable } from 'vs/base/common/lifecycle';
 import { URI } from 'vs/base/common/uri';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { ILogService } from 'vs/platform/log/common/log';
-import { IInteractiveRequestModel, IInteractiveResponseModel, IInteractiveSessionModel, IInteractiveSessionResponseCommandFollowup } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionModel';
+import { IInteractiveRequestModel, IInteractiveResponseErrorDetails, IInteractiveResponseModel, IInteractiveSessionModel, IInteractiveSessionResponseCommandFollowup } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionModel';
 import { IInteractiveSessionService } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionService';
 import { countWords } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionWordCounter';

@@ -58,6 +58,7 @@ export interface IInteractiveResponseViewModel {
 	readonly isComplete: boolean;
 	readonly followups?: string[];
 	readonly commandFollowups?: IInteractiveSessionResponseCommandFollowup[];
+	readonly errorDetails?: IInteractiveResponseErrorDetails;
 	readonly progressiveResponseRenderingEnabled: boolean;
 	readonly contentUpdateTimings?: IInteractiveSessionLiveUpdateData;
 	renderData?: IInteractiveResponseRenderData;
@@ -196,6 +197,10 @@ export class InteractiveResponseViewModel extends Disposable implements IInterac
 		return this._model.commandFollowups;
 	}

+	get errorDetails() {
+		return this._model.errorDetails;
+	}
+
 	renderData: IInteractiveResponseRenderData | undefined = undefined;

 	currentRenderedHeight: number | undefined;
@@ -224,7 +229,7 @@ export class InteractiveResponseViewModel extends Disposable implements IInterac
 		}

 		this._register(_model.onDidChange(() => {
-			if (this._isPlaceholder && _model.response.value) {
+			if (this._isPlaceholder && (_model.response.value || this.isComplete)) {
 				this._isPlaceholder = false;
 				if (this.renderData) {
 					this.renderData.renderedWordCount = 0;
--- a/src/vscode-dts/vscode.proposed.interactive.d.ts
+++ b/src/vscode-dts/vscode.proposed.interactive.d.ts
@@ -76,9 +76,15 @@ declare module 'vscode' {
 		followups?: string[];
 	}

+	export interface InteractiveResponseErrorDetails {
+		message: string;
+		responseIsIncomplete?: boolean;
+	}
+
 	export interface InteractiveResponseForProgress {
 		followups?: string[];
 		commands?: InteractiveResponseCommand[];
+		errorDetails?: InteractiveResponseErrorDetails;
 	}

 	export interface InteractiveProgress {