diff --git a/packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.test.ts b/packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.test.ts new file mode 100644 index 000000000..0e6d72fc6 --- /dev/null +++ b/packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.test.ts @@ -0,0 +1,81 @@ +import * as assert from "assert"; +import {instance, mock, when} from "ts-mockito"; +import {DatabricksCliCheck, LOGIN_TIMEOUT_SECONDS} from "./DatabricksCliCheck"; +import {DatabricksCliAuthProvider} from "./AuthProvider"; + +describe(__filename, () => { + const cliPath = "/path/to/bin/databricks"; + + function createProvider(profile?: string) { + const provider = mock(DatabricksCliAuthProvider); + when(provider.host).thenReturn( + new URL("https://test.cloud.databricks.com") + ); + when(provider.cliPath).thenReturn(cliPath); + when(provider.profile).thenReturn(profile); + return instance(provider); + } + + describe("login", () => { + it("passes a bounded --timeout to auth login so it cannot hang indefinitely", async () => { + let capturedArgs: string[] | undefined; + const check = new DatabricksCliCheck( + createProvider("dev"), + async (_file, args) => { + capturedArgs = args; + return {stdout: "", stderr: ""}; + } + ); + + await (check as any).login(); + + assert.ok(capturedArgs, "execFile should have been invoked"); + assert.deepStrictEqual(capturedArgs, [ + "auth", + "login", + "--profile", + "dev", + "--timeout", + `${LOGIN_TIMEOUT_SECONDS}s`, + ]); + }); + + it("uses --host when no profile is configured", async () => { + let capturedArgs: string[] | undefined; + const check = new DatabricksCliCheck( + createProvider(undefined), + async (_file, args) => { + capturedArgs = args; + return {stdout: "", stderr: ""}; + } + ); + + await (check as any).login(); + + assert.deepStrictEqual(capturedArgs, [ + "auth", + "login", + "--host", + "https://test.cloud.databricks.com", + "--timeout", + `${LOGIN_TIMEOUT_SECONDS}s`, + ]); + }); + + it("surfaces an actionable message when login fails (e.g. WSL browser hang/timeout)", async () => { + const check = new DatabricksCliCheck( + createProvider("dev"), + async () => { + throw {stderr: "context deadline exceeded"}; + } + ); + + await assert.rejects((check as any).login(), (e: Error) => { + assert.match(e.message, /context deadline exceeded/); + // Tells the user how to recover instead of leaving them stuck. + assert.match(e.message, /databricks auth login --profile dev/); + return true; + }); + }); + }); +}); diff --git a/packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.ts b/packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.ts index 479dffe62..9a5dc04b5 100644 --- a/packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.ts +++ b/packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.ts @@ -5,7 +5,11 @@ import { WorkspaceClient, logging, } from "@databricks/sdk-experimental"; -import {Disposable, window} from "vscode"; +import { + CancellationToken as VscodeCancellationToken, + Disposable, + window, +} from "vscode"; import {DatabricksCliAuthProvider} from "./AuthProvider"; import {orchestrate, OrchestrationLoopError, Step} from "./orchestrate"; import {Loggers} from "../../logger"; @@ -17,10 +21,38 @@ const extensionVersion = require("../../../package.json") type StepName = "tryLogin" | "login"; +/** + * Upper bound (in seconds) for the interactive `auth login` browser challenge. + * + * The bundled CLI defaults this to 1 hour. In environments where the OAuth + * browser/callback round-trip cannot complete (notably WSL, where the Linux + * CLI cannot open the Windows browser or receive the localhost callback), that + * default makes the extension appear to hang indefinitely on + * "Attempting to configure auth: databricks-cli" (databricks-vscode#1917). + * + * Five minutes is comfortably longer than a human needs to complete a browser + * login, but short enough that a broken environment fails fast with an + * actionable error instead of stalling for an hour. + */ +export const LOGIN_TIMEOUT_SECONDS = 300; + +/** + * Subset of {@link execFile} used by this class, injectable for testing. + */ +export type ExecFile = ( + file: string, + args: string[], + options?: Record, + cancellationToken?: VscodeCancellationToken +) => Promise<{stdout: string; stderr: string}>; + export class DatabricksCliCheck implements Disposable { private disposables: Disposable[] = []; - constructor(private authProvider: DatabricksCliAuthProvider) {} + constructor( + private authProvider: DatabricksCliAuthProvider, + private readonly execFileFn: ExecFile = execFile + ) {} dispose() { this.disposables.forEach((i) => i.dispose()); @@ -107,24 +139,36 @@ export class DatabricksCliCheck implements Disposable { } private async login(cancellationToken?: CancellationToken): Promise { + const host = this.authProvider.host.toString().replace(/\/+$/, ""); + const profile = this.authProvider.profile; + const args = ["auth", "login"]; + if (profile) { + args.push("--profile", profile); + } else { + args.push("--host", host); + } + // Bound the browser challenge so a non-completing OAuth flow (e.g. WSL) + // fails fast instead of stalling on the CLI's 1-hour default. + args.push("--timeout", `${LOGIN_TIMEOUT_SECONDS}s`); try { - const host = this.authProvider.host.toString().replace(/\/+$/, ""); - const profile = this.authProvider.profile; - const args = ["auth", "login"]; - if (profile) { - args.push("--profile", profile); - } else { - args.push("--host", host); - } - await execFile( + await this.execFileFn( this.authProvider.cliPath, args, {}, cancellationToken ); } catch (e: any) { + // The CLI's interactive login can fail to complete in environments + // where the browser/callback round-trip does not work (notably + // WSL). Point the user at running the same command themselves in a + // terminal, where the browser flow can be completed (or copied to + // the host browser), instead of surfacing only the raw CLI error. + const manualCommand = profile + ? `databricks auth login --profile ${profile}` + : `databricks auth login --host ${host}`; throw new Error( - `Login failed with Databricks CLI: ${e.stderr || e.message}` + `Login failed with Databricks CLI: ${e.stderr || e.message}. ` + + `Try running \`${manualCommand}\` in a terminal to complete the login, then reload.` ); } }