Show errors when parsing of host in .databrickscfg fails (#488)

Fixes #479

Co-authored-by: Fabian Jakobs <[email protected]>

Author: kartikgupta-db

packages/databricks-sdk-js/src/logging/loggingDecorators.ts

@@ -2,7 +2,11 @@ import "reflect-metadata";
 import {NamedLogger} from "./NamedLogger";
 import {getContextParamIndex, Context, ContextItems} from "../context";
 
-export function withLogContext(name: string, opName?: string) {
+export function withLogContext(
+    name: string,
+    opName?: string,
+    override = false
+) {
     return function (
         _target: any,
         _propertyKey: string,
@@ -23,7 +27,11 @@ export function withLogContext(name: string, opName?: string) {
             ).copy();
 
             const items: ContextItems = {
-                logger,
+                logger:
+                    (contextParam.logger && override) ||
+                    contextParam.logger === undefined
+                        ? logger
+                        : contextParam.logger,
                 opName: contextParam.opName ?? opName,
                 rootClassName:
                     contextParam.rootClassName ?? this.constructor.name,

packages/databricks-vscode/src/cli/CliWrapper.test.ts

@@ -12,6 +12,8 @@ import {writeFile} from "node:fs/promises";
 
 import {CliWrapper} from "./CliWrapper";
 import path from "node:path";
+import {Context} from "@databricks/databricks-sdk/dist/context";
+import {NamedLogger} from "@databricks/databricks-sdk/dist/logging";
 
 const execFile = promisify(execFileCb);
 
@@ -140,9 +142,28 @@ host = https://cloud.databricks.com/
 
 [missing-host-token]
 nothing = true
+
+[typo-host]
+host = example.com
 `
             );
-            const profiles = await cli.listProfiles(path);
+
+            const logs: {level: string; msg?: string; meta: any}[] = [];
+            const profiles = await cli.listProfiles(
+                path,
+                new Context({
+                    logger: NamedLogger.getOrCreate("cli-wrapper-test", {
+                        factory: (name) => {
+                            return {
+                                log: (level, msg, meta) => {
+                                    console.error("here");
+                                    logs.push({level, msg, meta});
+                                },
+                            };
+                        },
+                    }),
+                })
+            );
             assert.equal(profiles.length, 2);
 
             assert.equal(profiles[0].name, "correct");
@@ -152,6 +173,10 @@ nothing = true
             assert.equal(profiles[1].name, "no-token");
             assert.equal(profiles[1].host, "https://cloud.databricks.com/");
             assert.equal(profiles[1].authType, "");
+
+            const typoLog = logs.find((log) => log.msg?.includes("typo-host"));
+            assert.ok(typoLog !== undefined);
+            assert.ok(typoLog.level === "error");
         });
     });
 });

packages/databricks-vscode/src/cli/CliWrapper.ts

@@ -1,8 +1,11 @@
 import {execFile as execFileCb, spawn} from "child_process";
-import {ExtensionContext} from "vscode";
+import {ExtensionContext, window, commands} from "vscode";
 import {SyncDestinationMapper} from "../configuration/SyncDestination";
 import {workspaceConfigs} from "../vscode-objs/WorkspaceConfigs";
 import {promisify} from "node:util";
+import {withLogContext} from "@databricks/databricks-sdk/dist/logging";
+import {Loggers} from "../logger";
+import {Context, context} from "@databricks/databricks-sdk/dist/context";
 
 const execFile = promisify(execFileCb);
 
@@ -21,18 +24,26 @@ export interface ConfigEntry {
 }
 
 export type SyncType = "full" | "incremental";
+
+function getValidHost(host: string) {
+    if (host.match(/^(?:(?:https?):\/\/)?(.(?!:\/\/))+$/) !== null) {
+        return new URL(host);
+    } else {
+        throw new TypeError("Invalid type for host");
+    }
+}
 /**
  * Entrypoint for all wrapped CLI commands
  *
  * Righ now this is a placeholder for a future implementation
  * of the bricks CLI
  */
 export class CliWrapper {
-    constructor(private context: ExtensionContext) {}
+    constructor(private extensionContext: ExtensionContext) {}
 
     getTestBricksCommand(): Command {
         return {
-            command: this.context.asAbsolutePath("./bin/bricks"),
+            command: this.extensionContext.asAbsolutePath("./bin/bricks"),
             args: [],
         };
     }
@@ -44,7 +55,7 @@ export class CliWrapper {
         syncDestination: SyncDestinationMapper,
         syncType: SyncType
     ): Command {
-        const command = this.context.asAbsolutePath("./bin/bricks");
+        const command = this.extensionContext.asAbsolutePath("./bin/bricks");
         const args = [
             "sync",
             "--remote-path",
@@ -62,13 +73,15 @@ export class CliWrapper {
 
     private getListProfilesCommand(): Command {
         return {
-            command: this.context.asAbsolutePath("./bin/bricks"),
+            command: this.extensionContext.asAbsolutePath("./bin/bricks"),
             args: ["auth", "profiles", "--skip-validate"],
         };
     }
 
+    @withLogContext(Loggers.Extension)
     public async listProfiles(
-        configfilePath?: string
+        configfilePath?: string,
+        @context ctx?: Context
     ): Promise<Array<ConfigEntry>> {
         const cmd = await this.getListProfilesCommand();
         const res = await execFile(cmd.command, cmd.args, {
@@ -82,31 +95,55 @@ export class CliWrapper {
         });
         const profiles = JSON.parse(res.stdout).profiles || [];
         const result = [];
+
         for (const profile of profiles) {
-            result.push({
-                name: profile.name,
-                host: new URL(profile.host),
-                accountId: profile.account_id,
-                cloud: profile.cloud,
-                authType: profile.auth_type,
-                valid: profile.valid,
-            });
+            try {
+                result.push({
+                    name: profile.name,
+                    host: getValidHost(profile.host),
+                    accountId: profile.account_id,
+                    cloud: profile.cloud,
+                    authType: profile.auth_type,
+                    valid: profile.valid,
+                });
+            } catch (e: unknown) {
+                let msg: string;
+                if (e instanceof TypeError) {
+                    msg = `Can't parse host for profile ${profile.name}`;
+                } else {
+                    msg = `Error parsing profile ${profile.name}`;
+                }
+                ctx?.logger?.error(msg, e);
+                window
+                    .showWarningMessage(
+                        msg,
+                        "Open Databricks Config File",
+                        "Ignore"
+                    )
+                    .then((choice) => {
+                        if (choice === "Open Databricks Config File") {
+                            commands.executeCommand(
+                                "databricks.connection.openDatabricksConfigFile"
+                            );
+                        }
+                    });
+            }
         }
         return result;
     }
 
     public async getBundleSchema(): Promise<string> {
         const execFile = promisify(execFileCb);
         const {stdout} = await execFile(
-            this.context.asAbsolutePath("./bin/bricks"),
+            this.extensionContext.asAbsolutePath("./bin/bricks"),
             ["bundle", "schema"]
         );
         return stdout;
     }
 
     getAddProfileCommand(profile: string, host: URL): Command {
         return {
-            command: this.context.asAbsolutePath("./bin/bricks"),
+            command: this.extensionContext.asAbsolutePath("./bin/bricks"),
             args: [
                 "configure",
                 "--no-interactive",