storage - do not fail state access for extensions (#132777) (#133071)

extensions/vscode-api-tests/src/singlefolder-tests/state.test.ts

@@ -0,0 +1,45 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import 'mocha';
+import { ExtensionContext, extensions } from 'vscode';
+
+suite('vscode API - globalState / workspaceState', () => {
+
+	let extensionContext: ExtensionContext;
+	suiteSetup(async () => {
+		// Trigger extension activation and grab the context as some tests depend on it
+		await extensions.getExtension('vscode.vscode-api-tests')?.activate();
+		extensionContext = (global as any).testExtensionContext;
+	});
+
+	test.only('state', async () => {
+		for (const state of [extensionContext.globalState, extensionContext.workspaceState]) {
+			let keys = state.keys();
+			assert.strictEqual(keys.length, 0);
+
+			let res = state.get('state.test.get', 'default');
+			assert.strictEqual(res, 'default');
+
+			await state.update('state.test.get', 'testvalue');
+
+			keys = state.keys();
+			assert.strictEqual(keys.length, 1);
+			assert.strictEqual(keys[0], 'state.test.get');
+
+			res = state.get('state.test.get', 'default');
+			assert.strictEqual(res, 'testvalue');
+
+			await state.update('state.test.get', undefined);
+
+			keys = state.keys();
+			assert.strictEqual(keys.length, 0);
+
+			res = state.get('state.test.get', 'default');
+			assert.strictEqual(res, 'default');
+		}
+	});
+});

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

@@ -8,6 +8,7 @@ import { MainThreadStorageShape, MainContext, IExtHostContext, ExtHostStorageSha
 import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
 import { IDisposable } from 'vs/base/common/lifecycle';
 import { IExtensionIdWithVersion, IExtensionsStorageSyncService } from 'vs/platform/userDataSync/common/extensionsStorageSync';
+import { ILogService } from 'vs/platform/log/common/log';
 
 @extHostNamedCustomer(MainContext.MainThreadStorage)
 export class MainThreadStorage implements MainThreadStorageShape {
@@ -22,6 +23,7 @@ export class MainThreadStorage implements MainThreadStorageShape {
 		extHostContext: IExtHostContext,
 		@IStorageService storageService: IStorageService,
 		@IExtensionsStorageSyncService extensionsStorageSyncService: IExtensionsStorageSyncService,
+		@ILogService private readonly _logService: ILogService
 	) {
 		this._storageService = storageService;
 		this._extensionsStorageSyncService = extensionsStorageSyncService;
@@ -30,11 +32,7 @@ export class MainThreadStorage implements MainThreadStorageShape {
 		this._storageListener = this._storageService.onDidChangeValue(e => {
 			const shared = e.scope === StorageScope.GLOBAL;
 			if (shared && this._sharedStorageKeysToWatch.has(e.key)) {
-				try {
-					this._proxy.$acceptValue(shared, e.key, this._getValue(shared, e.key));
-				} catch (error) {
-					// ignore parsing errors that can happen
-				}
+				this._proxy.$acceptValue(shared, e.key, this._getValue(shared, e.key));
 			}
 		});
 	}
@@ -43,35 +41,30 @@ export class MainThreadStorage implements MainThreadStorageShape {
 		this._storageListener.dispose();
 	}
 
-	$getValue<T>(shared: boolean, key: string): Promise<T | undefined> {
+	async $getValue<T>(shared: boolean, key: string): Promise<T | undefined> {
 		if (shared) {
 			this._sharedStorageKeysToWatch.set(key, true);
 		}
-		try {
-			return Promise.resolve(this._getValue<T>(shared, key));
-		} catch (error) {
-			return Promise.reject(error);
-		}
+		return this._getValue<T>(shared, key);
 	}
 
 	private _getValue<T>(shared: boolean, key: string): T | undefined {
 		const jsonValue = this._storageService.get(key, shared ? StorageScope.GLOBAL : StorageScope.WORKSPACE);
-		if (!jsonValue) {
-			return undefined;
+		if (jsonValue) {
+			try {
+				return JSON.parse(jsonValue);
+			} catch (error) {
+				// Do not fail this call but log it for diagnostics
+				// https://github.com/microsoft/vscode/issues/132777
+				this._logService.error(`[mainThreadStorage] unexpected error parsing storage contents (key: ${key}, shared: ${shared}): ${error}`);
+			}
 		}
-		return JSON.parse(jsonValue);
+
+		return undefined;
 	}
 
-	$setValue(shared: boolean, key: string, value: object): Promise<void> {
-		let jsonValue: string;
-		try {
-			jsonValue = JSON.stringify(value);
-			// Extension state is synced separately through extensions
-			this._storageService.store(key, jsonValue, shared ? StorageScope.GLOBAL : StorageScope.WORKSPACE, StorageTarget.MACHINE);
-		} catch (err) {
-			return Promise.reject(err);
-		}
-		return Promise.resolve(undefined);
+	async $setValue(shared: boolean, key: string, value: object): Promise<void> {
+		this._storageService.store(key, JSON.stringify(value), shared ? StorageScope.GLOBAL : StorageScope.WORKSPACE, StorageTarget.MACHINE /* Extension state is synced separately through extensions */);
 	}
 
 	$registerExtensionStorageKeysToSync(extension: IExtensionIdWithVersion, keys: string[]): void {