storage - do not eager parse extension storage (fix microsoft#163446) (microsoft#166281)

Author: bpasero

src/vs/platform/extensionManagement/common/extensionStorage.ts

@@ -27,6 +27,7 @@ export interface IExtensionStorageService {
 	readonly _serviceBrand: undefined;
 
 	getExtensionState(extension: IExtension | IGalleryExtension | string, global: boolean): IStringDictionary<any> | undefined;
+	getExtensionStateRaw(extension: IExtension | IGalleryExtension | string, global: boolean): string | undefined;
 	setExtensionState(extension: IExtension | IGalleryExtension | string, state: IStringDictionary<any> | undefined, global: boolean): void;
 
 	readonly onDidChangeExtensionStorageToSync: Event<void>;
@@ -43,6 +44,8 @@ export class ExtensionStorageService extends Disposable implements IExtensionSto
 
 	readonly _serviceBrand: undefined;
 
+	private static LARGE_STATE_WARNING_THRESHOLD = 512 * 1024;
+
 	private static toKey(extension: IExtensionIdWithVersion): string {
 		return `extensionKeys/${adoptToGalleryExtensionId(extension.id)}@${extension.version}`;
 	}
@@ -142,7 +145,7 @@ export class ExtensionStorageService extends Disposable implements IExtensionSto
 
 	getExtensionState(extension: IExtension | IGalleryExtension | string, global: boolean): IStringDictionary<any> | undefined {
 		const extensionId = this.getExtensionId(extension);
-		const jsonValue = this.storageService.get(extensionId, global ? StorageScope.PROFILE : StorageScope.WORKSPACE);
+		const jsonValue = this.getExtensionStateRaw(extension, global);
 		if (jsonValue) {
 			try {
 				return JSON.parse(jsonValue);
@@ -156,6 +159,17 @@ export class ExtensionStorageService extends Disposable implements IExtensionSto
 		return undefined;
 	}
 
+	getExtensionStateRaw(extension: IExtension | IGalleryExtension | string, global: boolean): string | undefined {
+		const extensionId = this.getExtensionId(extension);
+		const rawState = this.storageService.get(extensionId, global ? StorageScope.PROFILE : StorageScope.WORKSPACE);
+
+		if (rawState && rawState?.length > ExtensionStorageService.LARGE_STATE_WARNING_THRESHOLD) {
+			this.logService.warn(`[mainThreadStorage] large extension state detected (extensionId: ${extensionId}, global: ${global}): ${rawState.length / 1024}kb. Consider to use 'storageUri' or 'globalStorageUri' to store this data on disk instead.`);
+		}
+
+		return rawState;
+	}
+
 	setExtensionState(extension: IExtension | IGalleryExtension | string, state: IStringDictionary<any> | undefined, global: boolean): void {
 		const extensionId = this.getExtensionId(extension);
 		if (state === undefined) {

src/vs/workbench/api/browser/mainThreadStorage.ts

@@ -32,7 +32,10 @@ export class MainThreadStorage implements MainThreadStorageShape {
 		this._storageListener = this._storageService.onDidChangeValue(e => {
 			const shared = e.scope === StorageScope.PROFILE;
 			if (shared && this._sharedStorageKeysToWatch.has(e.key)) {
-				this._proxy.$acceptValue(shared, e.key, this._extensionStorageService.getExtensionState(e.key, shared));
+				const rawState = this._extensionStorageService.getExtensionStateRaw(e.key, shared);
+				if (typeof rawState === 'string') {
+					this._proxy.$acceptValue(shared, e.key, rawState);
+				}
 			}
 		});
 	}
@@ -41,14 +44,14 @@ export class MainThreadStorage implements MainThreadStorageShape {
 		this._storageListener.dispose();
 	}
 
-	async $initializeExtensionStorage(shared: boolean, extensionId: string): Promise<object | undefined> {
+	async $initializeExtensionStorage(shared: boolean, extensionId: string): Promise<string | undefined> {
 
 		await this.checkAndMigrateExtensionStorage(extensionId, shared);
 
 		if (shared) {
 			this._sharedStorageKeysToWatch.set(extensionId, true);
 		}
-		return this._extensionStorageService.getExtensionState(extensionId, shared);
+		return this._extensionStorageService.getExtensionStateRaw(extensionId, shared);
 	}
 
 	async $setValue(shared: boolean, key: string, value: object): Promise<void> {

src/vs/workbench/api/common/extHost.protocol.ts

@@ -594,7 +594,7 @@ export interface MainThreadStatusBarShape extends IDisposable {
 }
 
 export interface MainThreadStorageShape extends IDisposable {
-	$initializeExtensionStorage(shared: boolean, extensionId: string): Promise<object | undefined>;
+	$initializeExtensionStorage(shared: boolean, extensionId: string): Promise<string | undefined>;
 	$setValue(shared: boolean, extensionId: string, value: object): Promise<void>;
 	$registerExtensionStorageKeysToSync(extension: IExtensionIdWithVersion, keys: string[]): void;
 }
@@ -2139,7 +2139,7 @@ export interface ExtHostInteractiveShape {
 }
 
 export interface ExtHostStorageShape {
-	$acceptValue(shared: boolean, extensionId: string, value: object | undefined): void;
+	$acceptValue(shared: boolean, extensionId: string, value: string): void;
 }
 
 export interface ExtHostThemingShape {

src/vs/workbench/api/common/extHostExtensionService.ts

@@ -154,7 +154,7 @@ export abstract class AbstractExtHostExtensionService extends Disposable impleme
 		this._myRegistry = new ExtensionDescriptionRegistry(
 			filterExtensions(this._globalRegistry, myExtensionsSet)
 		);
-		this._storage = new ExtHostStorage(this._extHostContext);
+		this._storage = new ExtHostStorage(this._extHostContext, this._logService);
 		this._secretState = new ExtHostSecretState(this._extHostContext);
 		this._storagePath = storagePath;
 

src/vs/workbench/api/common/extHostStorage.ts

@@ -8,6 +8,7 @@ import { Emitter } from 'vs/base/common/event';
 import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { IExtensionIdWithVersion } from 'vs/platform/extensionManagement/common/extensionStorage';
+import { ILogService } from 'vs/platform/log/common/log';
 
 export interface IStorageChangeEvent {
 	shared: boolean;
@@ -24,24 +25,49 @@ export class ExtHostStorage implements ExtHostStorageShape {
 	private readonly _onDidChangeStorage = new Emitter<IStorageChangeEvent>();
 	readonly onDidChangeStorage = this._onDidChangeStorage.event;
 
-	constructor(mainContext: IExtHostRpcService) {
+	constructor(
+		mainContext: IExtHostRpcService,
+		private readonly _logService: ILogService
+	) {
 		this._proxy = mainContext.getProxy(MainContext.MainThreadStorage);
 	}
 
 	registerExtensionStorageKeysToSync(extension: IExtensionIdWithVersion, keys: string[]): void {
 		this._proxy.$registerExtensionStorageKeysToSync(extension, keys);
 	}
 
-	initializeExtensionStorage(shared: boolean, key: string, defaultValue?: object): Promise<object | undefined> {
-		return this._proxy.$initializeExtensionStorage(shared, key).then(value => value || defaultValue);
+	async initializeExtensionStorage(shared: boolean, key: string, defaultValue?: object): Promise<object | undefined> {
+		const value = await this._proxy.$initializeExtensionStorage(shared, key);
+
+		let parsedValue: object | undefined;
+		if (value) {
+			parsedValue = this.safeParseValue(shared, key, value);
+		}
+
+		return parsedValue || defaultValue;
 	}
 
 	setValue(shared: boolean, key: string, value: object): Promise<void> {
 		return this._proxy.$setValue(shared, key, value);
 	}
 
-	$acceptValue(shared: boolean, key: string, value: object): void {
-		this._onDidChangeStorage.fire({ shared, key, value });
+	$acceptValue(shared: boolean, key: string, value: string): void {
+		const parsedValue = this.safeParseValue(shared, key, value);
+		if (parsedValue) {
+			this._onDidChangeStorage.fire({ shared, key, value: parsedValue });
+		}
+	}
+
+	private safeParseValue(shared: boolean, key: string, value: string): object | undefined {
+		try {
+			return JSON.parse(value);
+		} catch (error) {
+			// Do not fail this call but log it for diagnostics
+			// https://github.com/microsoft/vscode/issues/132777
+			this._logService.error(`[extHostStorage] unexpected error parsing storage contents (extensionId: ${key}, global: ${shared}): ${error}`);
+		}
+
+		return undefined;
 	}
 }