Surface db config errors (#1730)

extensions/ql-vscode/src/common/value-result.ts

@@ -0,0 +1,50 @@
+/**
+ * Represents a result that can be either a value or some errors.
+ */
+export class ValueResult<TValue> {
+  private constructor(
+    private readonly errorMsgs: string[],
+    private readonly val?: TValue,
+  ) {
+  }
+
+  public static ok<TValue>(value: TValue): ValueResult<TValue> {
+    if (value === undefined) {
+      throw new Error('Value must be set for successful result');
+    }
+
+    return new ValueResult([], value);
+  }
+
+  public static fail<TValue>(errorMsgs: string[]): ValueResult<TValue> {
+    if (errorMsgs.length === 0) {
+      throw new Error('At least one error message must be set for a failed result');
+    }
+
+    return new ValueResult<TValue>(errorMsgs, undefined);
+  }
+
+  public get isOk(): boolean {
+    return this.errorMsgs.length === 0;
+  }
+
+  public get isFailure(): boolean {
+    return this.errorMsgs.length > 0;
+  }
+
+  public get errors(): string[] {
+    if (!this.errorMsgs) {
+      throw new Error('Cannot get error for successful result');
+    }
+
+    return this.errorMsgs;
+  }
+
+  public get value(): TValue {
+    if (this.val === undefined) {
+      throw new Error('Cannot get value for unsuccessful result');
+    }
+
+    return this.val;
+  }
+}

extensions/ql-vscode/src/databases/db-config-store.ts

@@ -4,12 +4,14 @@ import { cloneDbConfig, DbConfig } from './db-config';
 import * as chokidar from 'chokidar';
 import { DisposableObject } from '../pure/disposable-object';
 import { DbConfigValidator } from './db-config-validator';
+import { ValueResult } from '../common/value-result';
 
 export class DbConfigStore extends DisposableObject {
   private readonly configPath: string;
   private readonly configValidator: DbConfigValidator;
 
-  private config: DbConfig;
+  private config: DbConfig | undefined;
+  private configErrors: string[];
   private configWatcher: chokidar.FSWatcher | undefined;
 
   public constructor(
@@ -20,6 +22,7 @@ export class DbConfigStore extends DisposableObject {
     this.configPath = path.join(workspaceStoragePath, 'workspace-databases.json');
 
     this.config = this.createEmptyConfig();
+    this.configErrors = [];
     this.configWatcher = undefined;
     this.configValidator = new DbConfigValidator(extensionPath);
   }
@@ -33,19 +36,19 @@ export class DbConfigStore extends DisposableObject {
     this.configWatcher?.unwatch(this.configPath);
   }
 
-  public getConfig(): DbConfig {
-    // Clone the config so that it's not modified outside of this class.
-    return cloneDbConfig(this.config);
+  public getConfig(): ValueResult<DbConfig> {
+    if (this.config) {
+      // Clone the config so that it's not modified outside of this class.
+      return ValueResult.ok(cloneDbConfig(this.config));
+    } else {
+      return ValueResult.fail(this.configErrors);
+    }
   }
 
   public getConfigPath(): string {
     return this.configPath;
   }
 
-  public validateConfig(): string[] {
-    return this.configValidator.validate(this.config);
-  }
-
   private async loadConfig(): Promise<void> {
     if (!await fs.pathExists(this.configPath)) {
       await fs.writeJSON(this.configPath, this.createEmptyConfig(), { spaces: 2 });
@@ -55,11 +58,33 @@ export class DbConfigStore extends DisposableObject {
   }
 
   private async readConfig(): Promise<void> {
-    this.config = await fs.readJSON(this.configPath);
+    let newConfig: DbConfig | undefined = undefined;
+    try {
+      newConfig = await fs.readJSON(this.configPath);
+    } catch (e) {
+      this.configErrors = [`Failed to read config file: ${this.configPath}`];
+    }
+
+    if (newConfig) {
+      this.configErrors = this.configValidator.validate(newConfig);
+    }
+
+    this.config = this.configErrors.length === 0 ? newConfig : undefined;
   }
 
   private readConfigSync(): void {
-    this.config = fs.readJSONSync(this.configPath);
+    let newConfig: DbConfig | undefined = undefined;
+    try {
+      newConfig = fs.readJSONSync(this.configPath);
+    } catch (e) {
+      this.configErrors = [`Failed to read config file: ${this.configPath}`];
+    }
+
+    if (newConfig) {
+      this.configErrors = this.configValidator.validate(newConfig);
+    }
+
+    this.config = this.configErrors.length === 0 ? newConfig : undefined;
   }
 
   private watchConfig(): void {

extensions/ql-vscode/src/databases/db-manager.ts

@@ -1,3 +1,4 @@
+import { ValueResult } from '../common/value-result';
 import { DbConfigStore } from './db-config-store';
 import { DbItem } from './db-item';
 import { createLocalTree, createRemoteTree } from './db-tree-creator';
@@ -8,13 +9,16 @@ export class DbManager {
   ) {
   }
 
-  public getDbItems(): DbItem[] {
-    const config = this.dbConfigStore.getConfig();
+  public getDbItems(): ValueResult<DbItem[]> {
+    const configResult = this.dbConfigStore.getConfig();
+    if (configResult.isFailure) {
+      return ValueResult.fail(configResult.errors);
+    }
 
-    return [
-      createRemoteTree(config),
+    return ValueResult.ok([
+      createRemoteTree(configResult.value),
       createLocalTree()
-    ];
+    ]);
   }
 
   public getConfigPath(): string {

extensions/ql-vscode/src/databases/ui/db-tree-data-provider.ts

@@ -1,5 +1,5 @@
 import { ProviderResult, TreeDataProvider, TreeItem } from 'vscode';
-import { createDbTreeViewItemWarning, DbTreeViewItem } from './db-tree-view-item';
+import { createDbTreeViewItemError, DbTreeViewItem } from './db-tree-view-item';
 import { DbManager } from '../db-manager';
 import { mapDbItemToTreeViewItem } from './db-item-mapper';
 
@@ -36,14 +36,17 @@ export class DbTreeDataProvider implements TreeDataProvider<DbTreeViewItem> {
   }
 
   private createTree(): DbTreeViewItem[] {
-    const dbItems = this.dbManager.getDbItems();
+    const dbItemsResult = this.dbManager.getDbItems();
 
-    // Add a sample warning as a proof of concept.
-    const warningTreeViewItem = createDbTreeViewItemWarning(
-      'There was an error',
-      'Fix it'
-    );
+    if (dbItemsResult.isFailure) {
+      const errorTreeViewItem = createDbTreeViewItemError(
+        'Error when reading databases config',
+        'Please open your databases config and address errors'
+      );
 
-    return [...dbItems.map(mapDbItemToTreeViewItem), warningTreeViewItem];
+      return [errorTreeViewItem];
+    }
+
+    return dbItemsResult.value.map(mapDbItemToTreeViewItem);
   }
 }

extensions/ql-vscode/src/databases/ui/db-tree-view-item.ts

@@ -26,10 +26,10 @@ export class DbTreeViewItem extends vscode.TreeItem {
   }
 }
 
-export function createDbTreeViewItemWarning(label: string, tooltip: string): DbTreeViewItem {
+export function createDbTreeViewItemError(label: string, tooltip: string): DbTreeViewItem {
   return new DbTreeViewItem(
     undefined,
-    new vscode.ThemeIcon('warning', new vscode.ThemeColor('problemsWarningIcon.foreground')),
+    new vscode.ThemeIcon('error', new vscode.ThemeColor('problemsErrorIcon.foreground')),
     label,
     tooltip,
     vscode.TreeItemCollapsibleState.None,

extensions/ql-vscode/test/pure-tests/databases/db-config-store.test.ts

@@ -23,7 +23,7 @@ describe('db config store', async () => {
     await configStore.initialize();
 
     expect(await fs.pathExists(configPath)).to.be.true;
-    const config = configStore.getConfig();
+    const config = configStore.getConfig().value;
     expect(config.remote.repositoryLists).to.be.empty;
     expect(config.remote.owners).to.be.empty;
     expect(config.remote.repositories).to.be.empty;
@@ -33,7 +33,7 @@ describe('db config store', async () => {
     const configStore = new DbConfigStore(testDataStoragePath, extensionPath);
     await configStore.initialize();
 
-    const config = configStore.getConfig();
+    const config = configStore.getConfig().value;
     expect(config.remote.repositoryLists).to.have.length(1);
     expect(config.remote.repositoryLists[0]).to.deep.equal({
       'name': 'repoList1',
@@ -48,10 +48,10 @@ describe('db config store', async () => {
     const configStore = new DbConfigStore(testDataStoragePath, extensionPath);
     await configStore.initialize();
 
-    const config = configStore.getConfig();
+    const config = configStore.getConfig().value;
     config.remote.repositoryLists = [];
 
-    const reRetrievedConfig = configStore.getConfig();
+    const reRetrievedConfig = configStore.getConfig().value;
     expect(reRetrievedConfig.remote.repositoryLists).to.have.length(1);
   });
 });