Connect: Show config file errors

web/packages/shared/components/Notification/Notification.tsx

@@ -150,7 +150,7 @@ function getRenderedContent(
           `}
         >
           <Text
-            fontSize={14}
+            fontSize={13}
             bold
             mr="30px"
             css={`

web/packages/teleterm/src/main.ts

@@ -39,22 +39,15 @@ if (app.requestSingleInstanceLock()) {
   app.exit(1);
 }
 
-function initializeApp(): void {
+async function initializeApp(): Promise<void> {
   let devRelaunchScheduled = false;
   const settings = getRuntimeSettings();
   const logger = initMainLogger(settings);
-  const appStateFileStorage = createFileStorage({
-    filePath: path.join(settings.userDataDir, 'app_state.json'),
-    debounceWrites: true,
-  });
-  const configFileStorage = createFileStorage({
-    filePath: path.join(settings.userDataDir, 'app_config.json'),
-    debounceWrites: false,
-  });
-  const configJsonSchemaFileStorage = createFileStorage({
-    filePath: path.join(settings.userDataDir, 'schema_app_config.json'),
-    debounceWrites: false,
-  });
+  const {
+    appStateFileStorage,
+    configFileStorage,
+    configJsonSchemaFileStorage,
+  } = await createFileStorages(settings.userDataDir);
 
   const configService = createConfigService({
     configFile: configFileStorage,
@@ -204,3 +197,25 @@ function initMainLogger(settings: types.RuntimeSettings) {
 
   return new Logger('Main');
 }
+
+function createFileStorages(userDataDir: string) {
+  return Promise.all([
+    createFileStorage({
+      filePath: path.join(userDataDir, 'app_state.json'),
+      debounceWrites: true,
+    }),
+    createFileStorage({
+      filePath: path.join(userDataDir, 'app_config.json'),
+      debounceWrites: false,
+      discardUpdatesWhenLoadingFileFailed: true,
+    }),
+    createFileStorage({
+      filePath: path.join(userDataDir, 'schema_app_config.json'),
+      debounceWrites: false,
+    }),
+  ]).then(storages => ({
+    appStateFileStorage: storages[0],
+    configFileStorage: storages[1],
+    configJsonSchemaFileStorage: storages[2],
+  }));
+}

web/packages/teleterm/src/mainProcess/types.ts

@@ -125,7 +125,7 @@ export enum TabContextMenuEventType {
 export enum ConfigServiceEventType {
   Get = 'Get',
   Set = 'Set',
-  GetStoredConfigErrors = 'GetStoredConfigErrors',
+  GetConfigError = 'GetConfigError',
 }
 
 export enum FileStorageEventType {
@@ -134,4 +134,5 @@ export enum FileStorageEventType {
   WriteSync = 'WriteSync',
   Replace = 'Replace',
   GetFilePath = 'GetFilePath',
+  GetFileLoadingError = 'GetFileLoadingError',
 }

web/packages/teleterm/src/services/config/configService.test.ts

@@ -32,7 +32,7 @@ test('stored and default values are combined', () => {
     platform: 'darwin',
   });
 
-  expect(configService.getStoredConfigErrors()).toBeUndefined();
+  expect(configService.getConfigError()).toBeUndefined();
 
   const usageReportingEnabled = configService.get('usageReporting.enabled');
   expect(usageReportingEnabled.value).toBe(true);
@@ -52,15 +52,18 @@ test('in case of invalid value a default one is returned', () => {
     platform: 'darwin',
   });
 
-  expect(configService.getStoredConfigErrors()).toStrictEqual([
-    {
-      code: 'invalid_type',
-      expected: 'boolean',
-      received: 'string',
-      message: 'Expected boolean, received string',
-      path: ['usageReporting.enabled'],
-    },
-  ]);
+  expect(configService.getConfigError()).toStrictEqual({
+    source: 'validation',
+    errors: [
+      {
+        code: 'invalid_type',
+        expected: 'boolean',
+        received: 'string',
+        message: 'Expected boolean, received string',
+        path: ['usageReporting.enabled'],
+      },
+    ],
+  });
 
   const usageReportingEnabled = configService.get('usageReporting.enabled');
   expect(usageReportingEnabled.value).toBe(false);
@@ -71,6 +74,23 @@ test('in case of invalid value a default one is returned', () => {
   expect(terminalFontSize.metadata.isStored).toBe(false);
 });
 
+test('if config file failed to load correctly the error is returned', () => {
+  const configFile = createMockFileStorage();
+  const error = new Error('Failed to read');
+  jest.spyOn(configFile, 'getFileLoadingError').mockReturnValue(error);
+
+  const configService = createConfigService({
+    configFile,
+    jsonSchemaFile: createMockFileStorage(),
+    platform: 'darwin',
+  });
+
+  expect(configService.getConfigError()).toStrictEqual({
+    source: 'file-loading',
+    error,
+  });
+});
+
 test('calling set updated the value in store', () => {
   const configFile = createMockFileStorage();
   const configService = createConfigService({

web/packages/teleterm/src/services/config/configService.ts

@@ -31,14 +31,31 @@ import {
 
 const logger = new Logger('ConfigService');
 
+type FileLoadingError = {
+  source: 'file-loading';
+  error: Error;
+};
+
+type ValidationError = {
+  source: 'validation';
+  errors: ZodIssue[];
+};
+
+type ConfigError = FileLoadingError | ValidationError;
+
 export interface ConfigService {
   get<K extends keyof AppConfig>(
     key: K
   ): { value: AppConfig[K]; metadata: { isStored: boolean } };
 
   set<K extends keyof AppConfig>(key: K, value: AppConfig[K]): void;
 
-  getStoredConfigErrors(): ZodIssue[] | undefined;
+  /**
+   * Returns validation errors or an error that occurred during loading the config file (this means IO and syntax errors).
+   * If validation errors occur, the incorrect values are replaced with the defaults.
+   * In case of an error coming from loading the file, all values are replaced with the defaults.
+   * */
+  getConfigError(): ConfigError | undefined;
 }
 
 export function createConfigService({
@@ -53,10 +70,11 @@ export function createConfigService({
   const schema = createAppConfigSchema(platform);
   updateJsonSchema({ schema, configFile, jsonSchemaFile });
 
-  const { storedConfig, configWithDefaults, errors } = validateStoredConfig(
-    schema,
-    configFile
-  );
+  const {
+    storedConfig,
+    configWithDefaults,
+    errors: validationErrors,
+  } = validateStoredConfig(schema, configFile);
 
   return {
     get: key => ({
@@ -68,7 +86,21 @@ export function createConfigService({
       configWithDefaults[key] = value;
       storedConfig[key] = value;
     },
-    getStoredConfigErrors: () => errors,
+    getConfigError: () => {
+      const fileLoadingError = configFile.getFileLoadingError();
+      if (fileLoadingError) {
+        return {
+          source: 'file-loading',
+          error: fileLoadingError,
+        };
+      }
+      if (validationErrors) {
+        return {
+          source: 'validation',
+          errors: validationErrors,
+        };
+      }
+    },
   };
 }
 
@@ -96,8 +128,6 @@ function updateJsonSchema({
   }
 }
 
-//TODO (gzdunek): syntax errors of the JSON file are silently ignored, report
-// them to the user too
 function validateStoredConfig(
   schema: AppConfigSchema,
   configFile: FileStorage

web/packages/teleterm/src/services/config/configServiceClient.ts

@@ -33,8 +33,8 @@ export function subscribeToConfigServiceEvents(
           return (event.returnValue = configService.get(item.path));
         case ConfigServiceEventType.Set:
           return configService.set(item.path, item.value);
-        case ConfigServiceEventType.GetStoredConfigErrors:
-          return (event.returnValue = configService.getStoredConfigErrors());
+        case ConfigServiceEventType.GetConfigError:
+          return (event.returnValue = configService.getConfigError());
       }
     }
   );
@@ -54,10 +54,10 @@ export function createConfigServiceClient(): ConfigService {
         value,
       });
     },
-    getStoredConfigErrors: () => {
+    getConfigError: () => {
       return ipcRenderer.sendSync(
         ConfigServiceEventChannel,
-        ConfigServiceEventType.GetStoredConfigErrors
+        ConfigServiceEventType.GetConfigError
       );
     },
   };

web/packages/teleterm/src/services/config/fixtures/mocks.ts

@@ -27,8 +27,8 @@ export function createMockConfigService(
     set(key, value) {
       values[key] = value;
     },
-    getStoredConfigErrors() {
-      return [];
+    getConfigError() {
+      return undefined;
     },
   };
 }

web/packages/teleterm/src/services/fileStorage/fileStorage.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import fs, { existsSync, readFileSync, writeFileSync } from 'fs';
+import fs from 'fs';
 
 import { debounce } from 'shared/utils/highbar';
 
@@ -37,25 +37,37 @@ export interface FileStorage {
 
   /** Returns the file path used to create the storage. */
   getFilePath(): string;
+
+  /** Returns the error that could occur while reading and parsing the file. */
+  getFileLoadingError(): Error | undefined;
 }
 
-export function createFileStorage(opts: {
+export async function createFileStorage(opts: {
   filePath: string;
   debounceWrites: boolean;
-}): FileStorage {
+  /** Prevents state updates when the file has not been loaded correctly, so its content will not be overwritten. */
+  discardUpdatesWhenLoadingFileFailed?: boolean;
+}): Promise<FileStorage> {
   if (!opts || !opts.filePath) {
     throw Error('missing filePath');
   }
 
   const { filePath } = opts;
-  let state = loadState(opts.filePath);
+  let { state, error } = await loadState(opts.filePath);
+  const discardUpdates = error && opts.discardUpdatesWhenLoadingFileFailed;
 
   function put(key: string, json: any): void {
+    if (discardUpdates) {
+      return;
+    }
     state[key] = json;
     stringifyAndWrite();
   }
 
   function writeSync(): void {
+    if (discardUpdates) {
+      return;
+    }
     const text = stringify(state);
     try {
       fs.writeFileSync(filePath, text);
@@ -64,19 +76,26 @@ export function createFileStorage(opts: {
     }
   }
 
-  function get<T>(key?: string): T {
-    return key ? state[key] : (state as T);
-  }
-
   function replace(json: any): void {
+    if (discardUpdates) {
+      return;
+    }
     state = json;
     stringifyAndWrite();
   }
 
+  function get<T>(key?: string): T {
+    return key ? state[key] : (state as T);
+  }
+
   function getFilePath(): string {
     return opts.filePath;
   }
 
+  function getFileLoadingError(): Error | undefined {
+    return error;
+  }
+
   function stringifyAndWrite(): void {
     const text = stringify(state);
 
@@ -91,19 +110,38 @@ export function createFileStorage(opts: {
     get,
     replace,
     getFilePath,
+    getFileLoadingError,
   };
 }
 
-function loadState(filePath: string) {
+async function loadState(
+  filePath: string
+): Promise<{ state: any; error: Error | undefined }> {
   try {
-    if (!existsSync(filePath)) {
-      writeFileSync(filePath, '{}');
-    }
-
-    return JSON.parse(readFileSync(filePath, { encoding: 'utf-8' }));
+    const file = await readOrCreateFile(filePath);
+    return {
+      state: JSON.parse(file),
+      error: undefined,
+    };
   } catch (error) {
     logger.error(`Cannot read ${filePath} file`, error);
-    return {};
+    return {
+      state: {},
+      error: error,
+    };
+  }
+}
+
+async function readOrCreateFile(filePath: string): Promise<string> {
+  try {
+    return await fs.promises.readFile(filePath, { encoding: 'utf-8' });
+  } catch (error) {
+    const defaultValue = '{}';
+    if (error?.code === 'ENOENT') {
+      await fs.promises.writeFile(filePath, defaultValue);
+      return defaultValue;
+    }
+    throw error;
   }
 }