Skip to content

Commit

Permalink
Show error for a corrupted config file (#955)
Browse files Browse the repository at this point in the history
Better error messaging when config file is corrupted. Should help with
cases like this:
#942

When we detect profile loading/parsing error (that comes from the CLI),
we proceed with the configuration wizard without any pre-defined
profiles, but also show a warning notification to the users, prompting
them to open their config file and fix it.
  • Loading branch information
ilia-db authored Nov 29, 2023
1 parent 33aff1e commit 8b5a944
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 24 deletions.
36 changes: 35 additions & 1 deletion packages/databricks-vscode/src/cli/CliWrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ host = example.com
path,
new Context({
logger: logging.NamedLogger.getOrCreate(
"cli-wrapper-test",
"cli-profile-format-test",
{
factory: () => {
return {
Expand Down Expand Up @@ -209,4 +209,38 @@ host = example.com
assert.ok(typoLog.level === "error");
});
});

it("should show error for corrupted config file and return empty profile list", async () => {
const logFilePath = getTempLogFilePath();
const cli = createCliWrapper(logFilePath);

await withFile(async ({path}) => {
await writeFile(path, `[bad]\ntest 123`);
const logs: {level: string; msg?: string; meta: any}[] = [];
const profiles = await cli.listProfiles(
path,
new Context({
logger: logging.NamedLogger.getOrCreate(
"cli-parsing-error-test",
{
factory: () => {
return {
log: (level, msg, meta) => {
logs.push({level, msg, meta});
},
};
},
}
),
})
);
const errorLog = logs.find(
(log) =>
log.msg?.includes("Failed to parse Databricks Config File")
);
assert.ok(errorLog !== undefined);
assert.ok(errorLog.level === "error");
assert.equal(profiles.length, 0);
});
});
});
64 changes: 41 additions & 23 deletions packages/databricks-vscode/src/cli/CliWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,32 @@ export class CliWrapper {
@context ctx?: Context
): Promise<Array<ConfigEntry>> {
const cmd = this.getListProfilesCommand();
const res = await execFile(cmd.command, cmd.args, {
env: {
/* eslint-disable @typescript-eslint/naming-convention */
HOME: process.env.HOME,
DATABRICKS_CONFIG_FILE:
configfilePath || process.env.DATABRICKS_CONFIG_FILE,
DATABRICKS_OUTPUT_FORMAT: "json",
/* eslint-enable @typescript-eslint/naming-convention */
},
});

let res;
try {
res = await execFile(cmd.command, cmd.args, {
env: {
/* eslint-disable @typescript-eslint/naming-convention */
HOME: process.env.HOME,
DATABRICKS_CONFIG_FILE:
configfilePath || process.env.DATABRICKS_CONFIG_FILE,
DATABRICKS_OUTPUT_FORMAT: "json",
/* eslint-enable @typescript-eslint/naming-convention */
},
});
} catch (e) {
let msg = "Failed to load Databricks Config File";
if (e instanceof Error) {
if (e.message.includes("cannot parse config file")) {
msg =
"Failed to parse Databricks Config File, please make sure it's in the correct ini format";
}
}
ctx?.logger?.error(msg, e);
this.showConfigFileWarning(msg);
return [];
}

const profiles = JSON.parse(res.stdout).profiles || [];
const result = [];

Expand All @@ -135,24 +151,26 @@ export class CliWrapper {
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"
);
}
});
this.showConfigFileWarning(msg);
}
}
return result;
}

private async showConfigFileWarning(msg: string) {
const openAction = "Open Databricks Config File";
const choice = await window.showWarningMessage(
msg,
openAction,
"Ignore"
);
if (choice === openAction) {
commands.executeCommand(
"databricks.connection.openDatabricksConfigFile"
);
}
}

public async getBundleSchema(): Promise<string> {
const execFile = promisify(execFileCb);
const {stdout} = await execFile(this.cliPath, ["bundle", "schema"]);
Expand Down

0 comments on commit 8b5a944

Please sign in to comment.