Skip to content

Commit cc1f5f9

Browse files
committed
Bound databricks-cli login timeout to fix indefinite WSL hang
*Why* With auth_type=databricks-cli, a stale/expired cached token makes the extension run `databricks auth login`, whose browser OAuth challenge defaults to a 1-hour timeout in the bundled CLI. In WSL the Linux CLI cannot open the Windows browser or receive the localhost callback, so the flow never completes and the extension appears to hang indefinitely on "Attempting to configure auth: databricks-cli" (#1917). The user is left with no feedback and no guidance. *What* - Pass `--timeout 300s` to `auth login` so a non-completing browser flow fails fast (5 min is ample for a human login, far below the 1h default). - On login failure, surface an actionable error telling the user to run `databricks auth login --profile <p>` (or `--host <h>`) in a terminal and reload, instead of only the raw CLI error. - Make the exec function injectable on DatabricksCliCheck so login argument-building and error handling are unit-testable without spawning the real CLI. This is the pragmatic fix for the reported symptom (the hang). The underlying token-cache host/profile split-brain is a separate CLI-side issue tracked upstream; it is not required to stop the hang. *Verification* - New DatabricksCliCheck.test.ts covers: bounded --timeout passed with a profile, --host used without a profile, and the actionable failure message. - tsc --noEmit clean; eslint and prettier clean on the changed files. - (The repo's VS Code mocha harness cannot run under this env's Node 26 due to an unrelated transitive dep; behavior was verified against the compiled output with the SDK/vscode modules stubbed.)
1 parent a27514c commit cc1f5f9

2 files changed

Lines changed: 137 additions & 12 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import * as assert from "assert";
2+
import {instance, mock, when} from "ts-mockito";
3+
import {DatabricksCliCheck, LOGIN_TIMEOUT_SECONDS} from "./DatabricksCliCheck";
4+
import {DatabricksCliAuthProvider} from "./AuthProvider";
5+
6+
describe(__filename, () => {
7+
const cliPath = "/path/to/bin/databricks";
8+
9+
function createProvider(profile?: string) {
10+
const provider = mock(DatabricksCliAuthProvider);
11+
when(provider.host).thenReturn(
12+
new URL("https://test.cloud.databricks.com")
13+
);
14+
when(provider.cliPath).thenReturn(cliPath);
15+
when(provider.profile).thenReturn(profile);
16+
return instance(provider);
17+
}
18+
19+
describe("login", () => {
20+
it("passes a bounded --timeout to auth login so it cannot hang indefinitely", async () => {
21+
let capturedArgs: string[] | undefined;
22+
const check = new DatabricksCliCheck(
23+
createProvider("dev"),
24+
async (_file, args) => {
25+
capturedArgs = args;
26+
return {stdout: "", stderr: ""};
27+
}
28+
);
29+
30+
await (check as any).login();
31+
32+
assert.ok(capturedArgs, "execFile should have been invoked");
33+
assert.deepStrictEqual(capturedArgs, [
34+
"auth",
35+
"login",
36+
"--profile",
37+
"dev",
38+
"--timeout",
39+
`${LOGIN_TIMEOUT_SECONDS}s`,
40+
]);
41+
});
42+
43+
it("uses --host when no profile is configured", async () => {
44+
let capturedArgs: string[] | undefined;
45+
const check = new DatabricksCliCheck(
46+
createProvider(undefined),
47+
async (_file, args) => {
48+
capturedArgs = args;
49+
return {stdout: "", stderr: ""};
50+
}
51+
);
52+
53+
await (check as any).login();
54+
55+
assert.deepStrictEqual(capturedArgs, [
56+
"auth",
57+
"login",
58+
"--host",
59+
"https://test.cloud.databricks.com",
60+
"--timeout",
61+
`${LOGIN_TIMEOUT_SECONDS}s`,
62+
]);
63+
});
64+
65+
it("surfaces an actionable message when login fails (e.g. WSL browser hang/timeout)", async () => {
66+
const check = new DatabricksCliCheck(
67+
createProvider("dev"),
68+
async () => {
69+
throw {stderr: "context deadline exceeded"};
70+
}
71+
);
72+
73+
await assert.rejects((check as any).login(), (e: Error) => {
74+
assert.match(e.message, /context deadline exceeded/);
75+
// Tells the user how to recover instead of leaving them stuck.
76+
assert.match(e.message, /databricks auth login --profile dev/);
77+
return true;
78+
});
79+
});
80+
});
81+
});

packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.ts

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import {
55
WorkspaceClient,
66
logging,
77
} from "@databricks/sdk-experimental";
8-
import {Disposable, window} from "vscode";
8+
import {
9+
CancellationToken as VscodeCancellationToken,
10+
Disposable,
11+
window,
12+
} from "vscode";
913
import {DatabricksCliAuthProvider} from "./AuthProvider";
1014
import {orchestrate, OrchestrationLoopError, Step} from "./orchestrate";
1115
import {Loggers} from "../../logger";
@@ -17,10 +21,38 @@ const extensionVersion = require("../../../package.json")
1721

1822
type StepName = "tryLogin" | "login";
1923

24+
/**
25+
* Upper bound (in seconds) for the interactive `auth login` browser challenge.
26+
*
27+
* The bundled CLI defaults this to 1 hour. In environments where the OAuth
28+
* browser/callback round-trip cannot complete (notably WSL, where the Linux
29+
* CLI cannot open the Windows browser or receive the localhost callback), that
30+
* default makes the extension appear to hang indefinitely on
31+
* "Attempting to configure auth: databricks-cli" (databricks-vscode#1917).
32+
*
33+
* Five minutes is comfortably longer than a human needs to complete a browser
34+
* login, but short enough that a broken environment fails fast with an
35+
* actionable error instead of stalling for an hour.
36+
*/
37+
export const LOGIN_TIMEOUT_SECONDS = 300;
38+
39+
/**
40+
* Subset of {@link execFile} used by this class, injectable for testing.
41+
*/
42+
export type ExecFile = (
43+
file: string,
44+
args: string[],
45+
options?: Record<string, unknown>,
46+
cancellationToken?: VscodeCancellationToken
47+
) => Promise<{stdout: string; stderr: string}>;
48+
2049
export class DatabricksCliCheck implements Disposable {
2150
private disposables: Disposable[] = [];
2251

23-
constructor(private authProvider: DatabricksCliAuthProvider) {}
52+
constructor(
53+
private authProvider: DatabricksCliAuthProvider,
54+
private readonly execFileFn: ExecFile = execFile
55+
) {}
2456

2557
dispose() {
2658
this.disposables.forEach((i) => i.dispose());
@@ -107,24 +139,36 @@ export class DatabricksCliCheck implements Disposable {
107139
}
108140

109141
private async login(cancellationToken?: CancellationToken): Promise<void> {
142+
const host = this.authProvider.host.toString().replace(/\/+$/, "");
143+
const profile = this.authProvider.profile;
144+
const args = ["auth", "login"];
145+
if (profile) {
146+
args.push("--profile", profile);
147+
} else {
148+
args.push("--host", host);
149+
}
150+
// Bound the browser challenge so a non-completing OAuth flow (e.g. WSL)
151+
// fails fast instead of stalling on the CLI's 1-hour default.
152+
args.push("--timeout", `${LOGIN_TIMEOUT_SECONDS}s`);
110153
try {
111-
const host = this.authProvider.host.toString().replace(/\/+$/, "");
112-
const profile = this.authProvider.profile;
113-
const args = ["auth", "login"];
114-
if (profile) {
115-
args.push("--profile", profile);
116-
} else {
117-
args.push("--host", host);
118-
}
119-
await execFile(
154+
await this.execFileFn(
120155
this.authProvider.cliPath,
121156
args,
122157
{},
123158
cancellationToken
124159
);
125160
} catch (e: any) {
161+
// The CLI's interactive login can fail to complete in environments
162+
// where the browser/callback round-trip does not work (notably
163+
// WSL). Point the user at running the same command themselves in a
164+
// terminal, where the browser flow can be completed (or copied to
165+
// the host browser), instead of surfacing only the raw CLI error.
166+
const manualCommand = profile
167+
? `databricks auth login --profile ${profile}`
168+
: `databricks auth login --host ${host}`;
126169
throw new Error(
127-
`Login failed with Databricks CLI: ${e.stderr || e.message}`
170+
`Login failed with Databricks CLI: ${e.stderr || e.message}. ` +
171+
`Try running \`${manualCommand}\` in a terminal to complete the login, then reload.`
128172
);
129173
}
130174
}

0 commit comments

Comments
 (0)