From 0349bcb92cb823f2469388b4c2feca2852bc920a Mon Sep 17 00:00:00 2001 From: Basit Date: Wed, 27 Sep 2023 17:09:01 +0200 Subject: [PATCH 1/5] ensure prefereces are valid --- .../src/storage.spec.ts | 38 +++++++++++++++++++ .../compass-preferences-model/src/storage.ts | 13 +++++-- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/packages/compass-preferences-model/src/storage.spec.ts b/packages/compass-preferences-model/src/storage.spec.ts index cb181b46fa6..c308c2d0348 100644 --- a/packages/compass-preferences-model/src/storage.spec.ts +++ b/packages/compass-preferences-model/src/storage.spec.ts @@ -136,6 +136,44 @@ describe('storage', function () { ).to.deep.equal(getDefaultPreferences()); }); + it('when invalid json is stored, it sets the defaults', async function () { + const storage = new StoragePreferences(tmpDir); + + const preferencesFile = getPreferencesFile(tmpDir); + await fs.mkdir(getPreferencesFolder(tmpDir)); + await fs.writeFile(preferencesFile, '{}}', 'utf-8'); + + // Ensure it exists + expect(async () => await fs.access(preferencesFile)).to.not.throw; + + await storage.setup(); + + expect( + JSON.parse((await fs.readFile(preferencesFile)).toString()) + ).to.deep.equal(getDefaultPreferences()); + }); + + it('when invalid value is stored, it sets the defaults', async function () { + const storage = new StoragePreferences(tmpDir); + + const preferencesFile = getPreferencesFile(tmpDir); + await fs.mkdir(getPreferencesFolder(tmpDir)); + await fs.writeFile( + preferencesFile, + JSON.stringify({ enableMaps: 'a string' }), + 'utf-8' + ); + + // Ensure it exists + expect(async () => await fs.access(preferencesFile)).to.not.throw; + + await storage.setup(); + + expect( + JSON.parse((await fs.readFile(preferencesFile)).toString()) + ).to.deep.equal(getDefaultPreferences()); + }); + it('updates preferences', async function () { const storage = new StoragePreferences(tmpDir); await storage.setup(); diff --git a/packages/compass-preferences-model/src/storage.ts b/packages/compass-preferences-model/src/storage.ts index 5d119b9bc6d..9c94217ed60 100644 --- a/packages/compass-preferences-model/src/storage.ts +++ b/packages/compass-preferences-model/src/storage.ts @@ -74,11 +74,16 @@ export class StoragePreferences extends BasePreferencesStorage { try { this.preferences = await this.readPreferences(); } catch (e) { - if ((e as any).code !== 'ENOENT') { - throw e; + if ( + (e as any).code === 'ENOENT' || // First time user + e instanceof z.ZodError || // Invalid preference values + e instanceof SyntaxError // Invalid json + ) { + // Create the file for the first time + await this.userData.write(this.file, this.defaultPreferences); + return; } - // Create the file for the first time - await this.userData.write(this.file, this.defaultPreferences); + throw e; } } From e69f0eeaa430f94813f65328bf59a0ac4e4acbaf Mon Sep 17 00:00:00 2001 From: Basit Date: Wed, 27 Sep 2023 17:09:13 +0200 Subject: [PATCH 2/5] ensure writes to storage are atomic --- package-lock.json | 63 +++++++++++++++++++++ packages/compass-user-data/package.json | 2 + packages/compass-user-data/src/user-data.ts | 3 +- 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index f66a5e0834e..c95db0a4940 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14273,6 +14273,15 @@ "integrity": "sha512-8oDqyLC7eD4HM307boe2QWKyuzdzWBj56xI/imSl2cpL+U3tCMaTAkMJ4ee5JBZ/FsOJlvRGeIShiZDAl1qERA==", "dev": true }, + "node_modules/@types/write-file-atomic": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/@types/write-file-atomic/-/write-file-atomic-4.0.1.tgz", + "integrity": "sha512-oMJ8hauPkPOPZf7WAvwcyR0r3By7c/MeJr+WVpt1wWiU3ZiPc+BbVXy4Y756wqH+I2p3VR+o5AM5eMKyuHfDng==", + "dev": true, + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/ws": { "version": "8.5.3", "resolved": "https://registry.npmjs.org/@types/ws/-/ws-8.5.3.tgz", @@ -48538,6 +48547,7 @@ "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/sinon-chai": "^3.2.5", + "@types/write-file-atomic": "^4.0.1", "chai": "^4.3.6", "depcheck": "^1.4.1", "eslint": "^7.25.0", @@ -48547,6 +48557,7 @@ "prettier": "^2.7.1", "sinon": "^9.2.3", "typescript": "^5.0.4", + "write-file-atomic": "^5.0.1", "zod": "^3.22.2" } }, @@ -48559,6 +48570,18 @@ "node": ">=0.3.1" } }, + "packages/compass-user-data/node_modules/signal-exit": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", + "integrity": "sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==", + "dev": true, + "engines": { + "node": ">=14" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "packages/compass-user-data/node_modules/sinon": { "version": "9.2.4", "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.4.tgz", @@ -48577,6 +48600,19 @@ "url": "https://opencollective.com/sinon" } }, + "packages/compass-user-data/node_modules/write-file-atomic": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/write-file-atomic/-/write-file-atomic-5.0.1.tgz", + "integrity": "sha512-+QU2zd6OTD8XWIJCbffaiQeH9U73qIqafo1x6V1snCWYGJf6cVE0cDR4D8xRzcEnfI21IFrUPzPGtcPf8AC+Rw==", + "dev": true, + "dependencies": { + "imurmurhash": "^0.1.4", + "signal-exit": "^4.0.1" + }, + "engines": { + "node": "^14.17.0 || ^16.13.0 || >=18.0.0" + } + }, "packages/compass-utils": { "name": "@mongodb-js/compass-utils", "version": "0.4.0", @@ -61359,6 +61395,7 @@ "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/sinon-chai": "^3.2.5", + "@types/write-file-atomic": "^4.0.1", "chai": "^4.3.6", "depcheck": "^1.4.1", "eslint": "^7.25.0", @@ -61368,6 +61405,7 @@ "prettier": "^2.7.1", "sinon": "^9.2.3", "typescript": "^5.0.4", + "write-file-atomic": "*", "zod": "^3.22.2" }, "dependencies": { @@ -61377,6 +61415,12 @@ "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", "dev": true }, + "signal-exit": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", + "integrity": "sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==", + "dev": true + }, "sinon": { "version": "9.2.4", "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.4.tgz", @@ -61390,6 +61434,16 @@ "nise": "^4.0.4", "supports-color": "^7.1.0" } + }, + "write-file-atomic": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/write-file-atomic/-/write-file-atomic-5.0.1.tgz", + "integrity": "sha512-+QU2zd6OTD8XWIJCbffaiQeH9U73qIqafo1x6V1snCWYGJf6cVE0cDR4D8xRzcEnfI21IFrUPzPGtcPf8AC+Rw==", + "dev": true, + "requires": { + "imurmurhash": "^0.1.4", + "signal-exit": "^4.0.1" + } } } }, @@ -69199,6 +69253,15 @@ "integrity": "sha512-8oDqyLC7eD4HM307boe2QWKyuzdzWBj56xI/imSl2cpL+U3tCMaTAkMJ4ee5JBZ/FsOJlvRGeIShiZDAl1qERA==", "dev": true }, + "@types/write-file-atomic": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/@types/write-file-atomic/-/write-file-atomic-4.0.1.tgz", + "integrity": "sha512-oMJ8hauPkPOPZf7WAvwcyR0r3By7c/MeJr+WVpt1wWiU3ZiPc+BbVXy4Y756wqH+I2p3VR+o5AM5eMKyuHfDng==", + "dev": true, + "requires": { + "@types/node": "*" + } + }, "@types/ws": { "version": "8.5.3", "resolved": "https://registry.npmjs.org/@types/ws/-/ws-8.5.3.tgz", diff --git a/packages/compass-user-data/package.json b/packages/compass-user-data/package.json index 9ddc4b256f8..77ba19c2840 100644 --- a/packages/compass-user-data/package.json +++ b/packages/compass-user-data/package.json @@ -58,6 +58,7 @@ "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/sinon-chai": "^3.2.5", + "@types/write-file-atomic": "^4.0.1", "chai": "^4.3.6", "depcheck": "^1.4.1", "eslint": "^7.25.0", @@ -67,6 +68,7 @@ "prettier": "^2.7.1", "sinon": "^9.2.3", "typescript": "^5.0.4", + "write-file-atomic": "^5.0.1", "zod": "^3.22.2" } } diff --git a/packages/compass-user-data/src/user-data.ts b/packages/compass-user-data/src/user-data.ts index 18849e0b751..d2a18f218ff 100644 --- a/packages/compass-user-data/src/user-data.ts +++ b/packages/compass-user-data/src/user-data.ts @@ -4,6 +4,7 @@ import path from 'path'; import { createLoggerAndTelemetry } from '@mongodb-js/compass-logging'; import { getStoragePath } from '@mongodb-js/compass-utils'; import type { z } from 'zod'; +import writeFile from 'write-file-atomic'; const { log, mongoLogId } = createLoggerAndTelemetry('COMPASS-USER-STORAGE'); @@ -172,7 +173,7 @@ export class UserData { const filepath = this.getFileName(id); const absolutePath = await this.getFileAbsolutePath(filepath); try { - await fs.writeFile(absolutePath, this.serialize(content), { + await writeFile(absolutePath, this.serialize(content), { encoding: 'utf-8', }); return true; From d983b5dbc2f5b97fdc0949ac45e58929538d36b5 Mon Sep 17 00:00:00 2001 From: Basit Date: Wed, 27 Sep 2023 17:14:49 +0200 Subject: [PATCH 3/5] ensure errors are handled --- packages/compass/src/main/index.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/compass/src/main/index.ts b/packages/compass/src/main/index.ts index 7146b14e4e5..7c9987ce8d7 100644 --- a/packages/compass/src/main/index.ts +++ b/packages/compass/src/main/index.ts @@ -108,7 +108,14 @@ async function main(): Promise { }); } - await CompassApplication.init(mode, globalPreferences); + try { + await CompassApplication.init(mode, globalPreferences); + } catch (e) { + // eslint-disable-next-line no-console + await handleUncaughtException(e as Error).then((e) => console.error(e)); + // eslint-disable-next-line no-console + await CompassApplication.runExitHandlers().then((e) => console.error(e)); + } if (mode === 'CLI') { let exitCode = 0; From 38fd9432b7a4f2f3b72d5720aa9159303d624919 Mon Sep 17 00:00:00 2001 From: Basit <1305718+mabaasit@users.noreply.github.com> Date: Wed, 27 Sep 2023 17:26:54 +0200 Subject: [PATCH 4/5] Update packages/compass/src/main/index.ts Co-authored-by: Sergey Petushkov --- packages/compass/src/main/index.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/compass/src/main/index.ts b/packages/compass/src/main/index.ts index 7c9987ce8d7..c60c06f3d89 100644 --- a/packages/compass/src/main/index.ts +++ b/packages/compass/src/main/index.ts @@ -111,10 +111,9 @@ async function main(): Promise { try { await CompassApplication.init(mode, globalPreferences); } catch (e) { - // eslint-disable-next-line no-console - await handleUncaughtException(e as Error).then((e) => console.error(e)); - // eslint-disable-next-line no-console - await CompassApplication.runExitHandlers().then((e) => console.error(e)); + await handleUncaughtException(e as Error); + await CompassApplication.runExitHandlers().finally(() => app.exit(1)); + return; } if (mode === 'CLI') { From e17848cf06ea7034e11d43588ea17915f4119683 Mon Sep 17 00:00:00 2001 From: Basit Date: Wed, 27 Sep 2023 18:24:39 +0200 Subject: [PATCH 5/5] remove value check --- .../src/storage.spec.ts | 21 ------------------- .../compass-preferences-model/src/storage.ts | 1 - 2 files changed, 22 deletions(-) diff --git a/packages/compass-preferences-model/src/storage.spec.ts b/packages/compass-preferences-model/src/storage.spec.ts index c308c2d0348..3c4624f7f13 100644 --- a/packages/compass-preferences-model/src/storage.spec.ts +++ b/packages/compass-preferences-model/src/storage.spec.ts @@ -153,27 +153,6 @@ describe('storage', function () { ).to.deep.equal(getDefaultPreferences()); }); - it('when invalid value is stored, it sets the defaults', async function () { - const storage = new StoragePreferences(tmpDir); - - const preferencesFile = getPreferencesFile(tmpDir); - await fs.mkdir(getPreferencesFolder(tmpDir)); - await fs.writeFile( - preferencesFile, - JSON.stringify({ enableMaps: 'a string' }), - 'utf-8' - ); - - // Ensure it exists - expect(async () => await fs.access(preferencesFile)).to.not.throw; - - await storage.setup(); - - expect( - JSON.parse((await fs.readFile(preferencesFile)).toString()) - ).to.deep.equal(getDefaultPreferences()); - }); - it('updates preferences', async function () { const storage = new StoragePreferences(tmpDir); await storage.setup(); diff --git a/packages/compass-preferences-model/src/storage.ts b/packages/compass-preferences-model/src/storage.ts index 9c94217ed60..ef638aeea0f 100644 --- a/packages/compass-preferences-model/src/storage.ts +++ b/packages/compass-preferences-model/src/storage.ts @@ -76,7 +76,6 @@ export class StoragePreferences extends BasePreferencesStorage { } catch (e) { if ( (e as any).code === 'ENOENT' || // First time user - e instanceof z.ZodError || // Invalid preference values e instanceof SyntaxError // Invalid json ) { // Create the file for the first time