Asymmetry in Memento API's JSON encoding/decoding

[hyangah]

I expected Memento's get to recover the JSON-stringifyable object but it doesn't always.

Repro: hyangah/vscode-extension-samples@194f02e

In this repro, I stored a Date object using update, and later attempted to retrieve the value using get<Date>.

	const KEY = 'TEST_TIMESTAMP_KEY';
	let value = context.globalState.get<Date>(KEY);
	if (value === undefined) {
		context.globalState.update(KEY, new Date());
		value = context.globalState.get<Date>(KEY);
	}
	console.log(`TEST_TIMESTAMP_KEY = ${value} type=${typeof value}`);
	if (value === undefined || !(value instanceof Date)) {
		throw Error(`unexpected date - ${value}`);
	}

The first round (immediately storing the value) works as expected, except that the value doesn't match the usual ISO 8601 format (JSON.stringify returns)

Congratulations, your extension "helloworld-sample" is now active!
extensionHostProcess.js:144
"2024-04-03T19:24:17.476Z"
extensionHostProcess.js:144
TEST_TIMESTAMP_KEY = Wed Apr 03 2024 15:24:17 GMT-0400 (Eastern Daylight Time) type=object

Restart the extension. Now this time, the error will be thrown, because context.globalState.get<Date>(KEY) is not a Date object. It's string. I am guessing get failed to parse the persisted JSON string (ISO 8601 format), and silently returned a string object.

Congratulations, your extension "helloworld-sample" is now active!
extensionHostProcess.js:144
"2024-04-03T19:25:06.045Z"
extensionHostProcess.js:144
TEST_TIMESTAMP_KEY = 2024-04-03T19:24:17.478Z type=string
(and crash)

I think this code used to work last year, but recently, we started to notice problems caused by this asymmetry.
Did Memento API get changed recently?

[bpasero]

Yeah good catch, I think we should make sure that the value we store is JSON.parse(JSON.stringify...) here:

vscode/src/vs/workbench/api/common/extHostMemento.ts

Line 79 in 3117776

this._value![key] = value;

So that the returned thing matches what you get when later asking.

[bpasero]

Notes to self: is there more custom revive logic here:

vscode/src/vs/workbench/services/extensions/common/rpcProtocol.ts

Line 24 in d06e212

function safeStringify(obj: any, replacer: JSONStringifyReplacer | null): string {

That could alter the result once it comes back from storage? If so, can we actually provide the right value before it comes back from the renderer?