sessions: be overly verbose about errors that happen during serialization (#298350)

Not sure the error is in here, but if it is, this should help.

Author: connor4312

src/vs/workbench/contrib/chat/common/model/chatSessionStore.ts

@@ -13,9 +13,11 @@ import { joinPath } from '../../../../../base/common/resources.js';
 import { URI } from '../../../../../base/common/uri.js';
 import { localize } from '../../../../../nls.js';
 import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
+import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js';
 import { IEnvironmentService } from '../../../../../platform/environment/common/environment.js';
 import { FileOperationResult, IFileService, toFileOperationResult } from '../../../../../platform/files/common/files.js';
 import { ILogService } from '../../../../../platform/log/common/log.js';
+import { IOpenerService } from '../../../../../platform/opener/common/opener.js';
 import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
 import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js';
 import { IUserDataProfilesService } from '../../../../../platform/userDataProfile/common/userDataProfile.js';
@@ -57,6 +59,8 @@ export class ChatSessionStore extends Disposable {
 		@IUserDataProfilesService private readonly userDataProfilesService: IUserDataProfilesService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
 		@IWorkspaceEditingService private readonly workspaceEditingService: IWorkspaceEditingService,
+		@IDialogService private readonly dialogService: IDialogService,
+		@IOpenerService private readonly openerService: IOpenerService,
 	) {
 		super();
 
@@ -339,6 +343,8 @@ export class ChatSessionStore extends Disposable {
 		}
 	}
 
+	private _didReportIssue = false;
+
 	private async writeSession(session: ChatModel | ISerializableChatData): Promise<void> {
 		try {
 			const index = this.internalGetIndex();
@@ -349,7 +355,32 @@ export class ChatSessionStore extends Disposable {
 						session.dataSerializer = new ChatSessionOperationLog();
 					}
 
-					const { op, data } = session.dataSerializer.write(session);
+					let op: 'append' | 'replace';
+					let data: VSBuffer;
+					try {
+						({ op, data } = session.dataSerializer.write(session));
+					} catch (e) {
+						// This is a big of an ugly prompt, but there is _something_ going on with
+						// missing sessions. Unfortunately it's hard to root cause because users would
+						// not notice an error until they reload the window, at which point any error
+						// is gone. Throw a very verbose dialog here so we can get some quality
+						// bug reports, if the issue is indeed in the serialized.
+						// todo@connor4312: remove after a little bit
+						if (!this._didReportIssue) {
+							this._didReportIssue = true;
+							this.dialogService.prompt({
+								custom: true, // so text is copyable
+								title: localize('chatSessionStore.serializationError', 'Error saving chat session'),
+								message: localize('chatSessionStore.writeError', 'Error serializing chat session for storage. The session will be lost if the window is closed. Please report this issue to the VS Code team:\n\n{0}', e.stack || toErrorMessage(e)),
+								buttons: [
+									{ label: localize('reportIssue', 'Report Issue'), run: () => this.openerService.open('https://github.com/microsoft/vscode/issues/new?template=bug_report.md') }
+								]
+							});
+						}
+
+						throw e;
+					}
+
 					if (data.byteLength > 0) {
 						await this.fileService.writeFile(storageLocation.log, data, { append: op === 'append' });
 					}

src/vs/workbench/contrib/chat/common/model/objectMutationLog.ts

@@ -7,6 +7,34 @@ import { assertNever } from '../../../../../base/common/assert.js';
 import { VSBuffer } from '../../../../../base/common/buffer.js';
 import { isUndefinedOrNull } from '../../../../../base/common/types.js';
 
+/**
+ * Updates an error's message and stack trace with a prefix. In V8 the stack
+ * string starts with "ErrorName: message\n  at …", so we rebuild the header
+ * after mutating the message.
+ */
+function prefixError(e: Error, prefix: string): void {
+	e.message = prefix + e.message;
+	if (e.stack) {
+		const nlIdx = e.stack.indexOf('\n');
+		e.stack = nlIdx !== -1
+			? `${e.name}: ${e.message}${e.stack.slice(nlIdx)}`
+			: `${e.name}: ${e.message}`;
+	}
+}
+
+/**
+ * Prepends a path segment to an error as it unwinds through nested extract
+ * calls. Each level adds its segment so the final message reads e.g.
+ * `.responses[2].content: Cannot read property 'x' of undefined`.
+ */
+function rethrowWithPathSegment(e: unknown, segment: string | number): never {
+	if (e instanceof Error) {
+		const part = typeof segment === 'number' ? `[${segment}]` : `.${segment}`;
+		const needsSep = !e.message.startsWith('[') && !e.message.startsWith('.');
+		prefixError(e, part + (needsSep ? ': ' : ''));
+	}
+	throw e;
+}
 
 /** IMPORTANT: `Key` comes first. Then we should sort in order of least->most expensive to diff */
 const enum TransformKind {
@@ -96,7 +124,13 @@ export function array<T, R>(schema: TransformObject<T, R> | TransformValue<T, R>
 	return {
 		kind: TransformKind.Array,
 		itemSchema: schema,
-		extract: from => from?.map(item => schema.extract(item)),
+		extract: from => from?.map((item, i) => {
+			try {
+				return schema.extract(item);
+			} catch (e) {
+				rethrowWithPathSegment(e, i);
+			}
+		}),
 	};
 }
 
@@ -124,7 +158,11 @@ export function object<T, R extends object>(schema: Schema<T, R>, options?: Obje
 
 			const result: Record<string, unknown> = Object.create(null);
 			for (const [key, transform] of entries) {
-				result[key] = transform.extract(from);
+				try {
+					result[key] = transform.extract(from);
+				} catch (e) {
+					rethrowWithPathSegment(e, key);
+				}
 			}
 			return result as R;
 		},
@@ -278,7 +316,15 @@ export class ObjectMutationLog<TFrom, TTo> {
 		// Generate diff entries
 		const entries: Entry[] = [];
 		const path: ObjectPath = [];
-		this._diff(this._transform, path, this._previous, currentValue, entries);
+		try {
+			this._diff(this._transform, path, this._previous, currentValue, entries);
+		} catch (e) {
+			if (e instanceof Error) {
+				const pathStr = path.map(s => typeof s === 'number' ? `[${s}]` : `.${s}`).join('') || '<root>';
+				prefixError(e, `error diffing at ${pathStr}: `);
+			}
+			throw e;
+		}
 
 		if (entries.length === 0) {
 			// No changes