Skip to content

Commit be838b7

Browse files
kartikgupta-dbkartikgupta-db
authored
[DECO-192] Improve error handling for .databrickscfg parsing (#123)
- Fixes [DECO-192](https://databricks.atlassian.net/browse/DECO-192) - Show errors in profiles as warnings and still display valid profiles for selection. - Also route debug logs to a dedicated output panel. https://user-images.githubusercontent.com/88345179/196211655-0aa22d40-e3bd-4fa1-8d38-cb8dfac23ef3.mov
1 parent 02e5ae7 commit be838b7

7 files changed

Lines changed: 153 additions & 34 deletions

File tree

‎packages/databricks-sdk-js/src/auth/configFile.test.ts‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
/* eslint-disable @typescript-eslint/naming-convention */
22

33
import assert from "node:assert";
4-
import {loadConfigFile, resolveConfigFilePath} from "./configFile";
4+
import {
5+
HostParsingError,
6+
isConfigFileParsingError,
7+
loadConfigFile,
8+
resolveConfigFilePath,
9+
TokenParsingError,
10+
} from "./configFile";
511
import {writeFile} from "node:fs/promises";
612
import {withFile} from "tmp-promise";
713
import {homedir} from "node:os";
@@ -48,11 +54,14 @@ token = dapitest54321
4854
const profiles = await loadConfigFile(path);
4955

5056
assert.equal(Object.keys(profiles).length, 2);
57+
assert.ok(!isConfigFileParsingError(profiles.DEFAULT));
5158
assert.equal(
5259
profiles.DEFAULT.host.href,
5360
"https://cloud.databricks.com/"
5461
);
5562
assert.equal(profiles.DEFAULT.token, "dapitest1234");
63+
64+
assert.ok(!isConfigFileParsingError(profiles.STAGING));
5665
assert.equal(
5766
profiles.STAGING.host.href,
5867
"https://staging.cloud.databricks.com/"
@@ -78,16 +87,72 @@ token = dapitest54321
7887
const profiles = await loadConfigFile(path);
7988

8089
assert.equal(Object.keys(profiles).length, 2);
90+
assert.ok(!isConfigFileParsingError(profiles.DEFAULT));
8191
assert.equal(
8292
profiles.DEFAULT.host.href,
8393
"https://cloud.databricks.com/"
8494
);
8595
assert.equal(profiles.DEFAULT.token, "dapitest1234");
96+
97+
assert.ok(!isConfigFileParsingError(profiles.STAGING));
8698
assert.equal(
8799
profiles.STAGING.host.href,
88100
"https://staging.cloud.databricks.com/"
89101
);
90102
assert.equal(profiles.STAGING.token, "dapitest54321");
91103
});
92104
});
105+
106+
it("should load all valid profiles and return errors for rest", async () => {
107+
await withFile(async ({path}) => {
108+
await writeFile(
109+
path,
110+
`[correct]
111+
host = https://cloud.databricks.com/
112+
token = dapitest1234
113+
114+
[no-host]
115+
token = dapitest54321
116+
117+
[wrong-host]
118+
host = wrong
119+
token = dapitest54321
120+
121+
[no-token]
122+
host = https://cloud.databricks.com/
123+
124+
[missing-host-token]
125+
nothing = true
126+
`
127+
);
128+
const profiles = await loadConfigFile(path);
129+
assert.equal(Object.keys(profiles).length, 5);
130+
assert.ok(!isConfigFileParsingError(profiles["correct"]));
131+
assert.deepEqual(profiles["correct"], {
132+
host: new URL("https://cloud.databricks.com/"),
133+
token: "dapitest1234",
134+
});
135+
136+
assert.ok(isConfigFileParsingError(profiles["no-host"]));
137+
assert.deepEqual(
138+
profiles["no-host"],
139+
new HostParsingError('"host" it not defined')
140+
);
141+
142+
assert.ok(isConfigFileParsingError(profiles["wrong-host"]));
143+
assert.ok(profiles["wrong-host"] instanceof HostParsingError);
144+
145+
assert.ok(isConfigFileParsingError(profiles["no-token"]));
146+
assert.deepEqual(
147+
profiles["no-token"],
148+
new TokenParsingError('"token" it not defined')
149+
);
150+
151+
assert.ok(isConfigFileParsingError(profiles["missing-host-token"]));
152+
assert.deepEqual(
153+
profiles["missing-host-token"],
154+
new HostParsingError('"host" it not defined')
155+
);
156+
});
157+
});
93158
});

‎packages/databricks-sdk-js/src/auth/configFile.ts‎

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,39 @@ import path from "node:path";
22
import {readFile, stat} from "node:fs/promises";
33
import {parse} from "ini";
44
import {homedir} from "node:os";
5-
import {defaultRedactor} from "../Redactor";
65

7-
export type Profiles = Record<
8-
string,
9-
{
10-
host: URL;
11-
token: string;
6+
export class ConfigFileError extends Error {}
7+
export class ConfigFileProfileParsingError extends Error {
8+
constructor(name?: string, message?: string) {
9+
super(message);
10+
this.name = name ?? "";
1211
}
13-
>;
12+
}
1413

15-
export class ConfigFileError extends Error {}
14+
export class HostParsingError extends ConfigFileProfileParsingError {
15+
constructor(message?: string) {
16+
super("HostParsingError", message);
17+
}
18+
}
19+
20+
export class TokenParsingError extends ConfigFileProfileParsingError {
21+
constructor(message?: string) {
22+
super("TokenParsingError", message);
23+
}
24+
}
25+
26+
export type Profile = {
27+
host: URL;
28+
token: string;
29+
};
30+
31+
export type Profiles = Record<string, Profile | ConfigFileProfileParsingError>;
32+
33+
export function isConfigFileParsingError(
34+
profileOrError: Profile | ConfigFileProfileParsingError
35+
): profileOrError is ConfigFileProfileParsingError {
36+
return profileOrError instanceof ConfigFileProfileParsingError;
37+
}
1638

1739
export function resolveConfigFilePath(filePath?: string): string {
1840
if (!filePath) {
@@ -26,9 +48,30 @@ export function resolveConfigFilePath(filePath?: string): string {
2648
return filePath;
2749
}
2850

29-
function getProfile(config: any) {
51+
function getProfileOrError(
52+
config: any
53+
): Profile | ConfigFileProfileParsingError {
54+
if (config.host === undefined) {
55+
return new HostParsingError('"host" it not defined');
56+
}
57+
58+
let host;
59+
try {
60+
host = new URL(config.host);
61+
} catch (e: unknown) {
62+
if (typeof e === "string") {
63+
return new HostParsingError(e);
64+
} else if (e instanceof Error) {
65+
return new HostParsingError(`${e.name}: ${e.message}`);
66+
}
67+
return new HostParsingError(String(e));
68+
}
69+
70+
if (config.token === undefined) {
71+
return new TokenParsingError('"token" it not defined');
72+
}
3073
return {
31-
host: new URL(config.host),
74+
host: host,
3275
token: config.token,
3376
};
3477
}
@@ -66,10 +109,10 @@ export async function loadConfigFile(filePath?: string): Promise<Profiles> {
66109
defaultSectionFound = true;
67110
continue;
68111
}
69-
profiles[key] = getProfile(config[key]);
112+
profiles[key] = getProfileOrError(config[key]);
70113
}
71114
if (defaultSectionFound) {
72-
profiles["DEFAULT"] = getProfile(defaultSection);
115+
profiles["DEFAULT"] = getProfileOrError(defaultSection);
73116
}
74117
} catch (e: unknown) {
75118
let message;

‎packages/databricks-sdk-js/src/auth/fromConfigFile.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,3 @@ token = dapitest54321`
4646
});
4747
});
4848
});
49-
50-
//let stub = sinon.stub(process.env, 'FOO').value('bar');

‎packages/databricks-sdk-js/src/auth/fromConfigFile.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
Credentials,
44
CredentialsProviderError,
55
} from "./types";
6-
import {loadConfigFile} from "./configFile";
6+
import {isConfigFileParsingError, loadConfigFile} from "./configFile";
77

88
export const DEFAULT_PROFILE = "DEFAULT";
99

@@ -20,13 +20,18 @@ export const fromConfigFile = (
2020

2121
const config = await loadConfigFile(configFile);
2222

23-
if (config[profile].host && config[profile].token) {
24-
cachedValue = config[profile];
25-
return cachedValue;
23+
if (!(config as Object).hasOwnProperty(profile)) {
24+
throw new CredentialsProviderError(`Can't find profile ${profile}`);
25+
}
26+
27+
const details = config[profile];
28+
if (isConfigFileParsingError(details)) {
29+
throw new CredentialsProviderError(
30+
`Can't load profile ${profile}: ${details.name}: ${details.message}`
31+
);
2632
}
2733

28-
throw new CredentialsProviderError(
29-
"Can't load credentials from config file"
30-
);
34+
cachedValue = details;
35+
return cachedValue;
3136
};
3237
};

‎packages/databricks-sdk-js/src/auth/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import {ConfigFileProfileParsingError} from "..";
2+
13
/**
24
* An object representing temporary or permanent AWS credentials.
35
*/

‎packages/databricks-vscode/.vscode/launch.json‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,8 @@
99
"name": "Run Extension",
1010
"type": "extensionHost",
1111
"request": "launch",
12-
"args": [
13-
"--extensionDevelopmentPath=${workspaceFolder}"
14-
],
15-
"outFiles": [
16-
"${workspaceFolder}/out/**/*.js"
17-
],
12+
"args": ["--extensionDevelopmentPath=${workspaceFolder}"],
13+
"outFiles": ["${workspaceFolder}/out/**/*.js"],
1814
"preLaunchTask": "${defaultBuildTask}",
1915
"env": {
2016
"DATABRICKS_DEBUG_HEADERS": "false"
@@ -28,9 +24,7 @@
2824
"--extensionDevelopmentPath=${workspaceFolder}",
2925
"--extensionTestsPath=${workspaceFolder}/out/test/suite"
3026
],
31-
"outFiles": [
32-
"${workspaceFolder}/out/**/*.js"
33-
],
27+
"outFiles": ["${workspaceFolder}/out/**/*.js"],
3428
"preLaunchTask": "${defaultBuildTask}",
3529
"env": {
3630
"DATABRICKS_DEBUG_HEADERS": "false"

‎packages/databricks-vscode/src/configuration/selectProfileWizard.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
ConfigFileError,
3+
isConfigFileParsingError,
34
loadConfigFile,
45
Profiles,
56
resolveConfigFilePath,
@@ -91,9 +92,20 @@ export async function selectProfile(
9192
}
9293
}
9394

94-
let items: Array<QuickPickItem> = Object.keys(profiles).map(
95-
(label) => ({label})
96-
);
95+
let items: Array<QuickPickItem> = Object.keys(profiles)
96+
.filter((label) => !isConfigFileParsingError(profiles[label]))
97+
.map((label) => ({label}));
98+
99+
Object.keys(profiles)
100+
.filter((label) => isConfigFileParsingError(profiles[label]))
101+
.forEach((label) => {
102+
const details = profiles[label];
103+
if (isConfigFileParsingError(details)) {
104+
window.showWarningMessage(
105+
`Can't parse profile "${label}": ${details.name}: ${details.message}`
106+
);
107+
}
108+
});
97109

98110
if (items.length) {
99111
items = [

0 commit comments

Comments
 (0)