test(e2e): retry the dbconnect pip fallback through transient Windows file locks

packages/databricks-vscode/src/test/e2e/run_dbconnect.ucws.e2e.ts

@@ -16,6 +16,7 @@ import {
     getBasicBundleConfig,
     writeRootBundleConfig,
 } from "./utils/dabsFixtures.ts";
+import {isTransientFileLockError, retryOnTransientError} from "../retry.ts";
 
 const execFile = promisify(execFileCb);
 
@@ -51,7 +52,8 @@ const DBCONNECT_VERSION_SPEC = "17.3.*";
 // tests"), so the notebook cell never runs and no output file is written. We
 // install the deps directly to remove that dependency on the flaky UI step; the
 // pip call is idempotent, so on shards that already have them it is a fast
-// no-op.
+// no-op. The install is retried on transient Windows file locks (see
+// ensureVenvHasDbConnect for why).
 async function ensureVenvHasKernelDeps(projectDir: string) {
     const python = venvPython(projectDir);
     try {
@@ -63,13 +65,27 @@ async function ensureVenvHasKernelDeps(projectDir: string) {
         return;
     }
     try {
-        const {stdout, stderr} = await execFile(python, [
-            "-m",
-            "pip",
-            "install",
-            ...KERNEL_DEPS,
-            "--disable-pip-version-check",
-        ]);
+        const {stdout, stderr} = await retryOnTransientError(
+            () =>
+                execFile(python, [
+                    "-m",
+                    "pip",
+                    "install",
+                    ...KERNEL_DEPS,
+                    "--disable-pip-version-check",
+                ]),
+            {
+                attempts: 3,
+                delayMs: 5000,
+                isTransient: isTransientFileLockError,
+                onRetry: (attempt, e) =>
+                    console.log(
+                        "kernel-deps install hit a transient Windows file " +
+                            `lock (attempt ${attempt}); retrying.`,
+                        e
+                    ),
+            }
+        );
         console.log("ensureVenvHasKernelDeps stdout:", stdout);
         if (stderr) {
             console.log("ensureVenvHasKernelDeps stderr:", stderr);
@@ -89,7 +105,11 @@ async function ensureVenvHasKernelDeps(projectDir: string) {
 // installs it directly. The `console.warn` is deliberate: it keeps a real
 // Windows UX regression discoverable in nightly logs even though the direct
 // install turns the test green. The pip call is skipped when the package is
-// already present, so healthy shards pay no reinstall cost.
+// already present, so healthy shards pay no reinstall cost. The install itself
+// is retried on the transient Windows file lock (WinError 32 "used by another
+// process"): pip has to overwrite the venv's existing numpy, and if a scanner
+// or a still-running install holds one of its files open the first attempt dies
+// mid-write, but a moment later succeeds.
 async function ensureVenvHasDbConnect(projectDir: string) {
     const python = venvPython(projectDir);
     try {
@@ -117,13 +137,27 @@ async function ensureVenvHasDbConnect(projectDir: string) {
     }
 
     try {
-        const {stdout, stderr} = await execFile(python, [
-            "-m",
-            "pip",
-            "install",
-            `databricks-connect==${DBCONNECT_VERSION_SPEC}`,
-            "--disable-pip-version-check",
-        ]);
+        const {stdout, stderr} = await retryOnTransientError(
+            () =>
+                execFile(python, [
+                    "-m",
+                    "pip",
+                    "install",
+                    `databricks-connect==${DBCONNECT_VERSION_SPEC}`,
+                    "--disable-pip-version-check",
+                ]),
+            {
+                attempts: 3,
+                delayMs: 5000,
+                isTransient: isTransientFileLockError,
+                onRetry: (attempt, e) =>
+                    console.log(
+                        "databricks-connect install hit a transient Windows " +
+                            `file lock (attempt ${attempt}); retrying.`,
+                        e
+                    ),
+            }
+        );
         console.log("ensureVenvHasDbConnect stdout:", stdout);
         if (stderr) {
             console.log("ensureVenvHasDbConnect stderr:", stderr);

packages/databricks-vscode/src/test/retry.test.ts

@@ -0,0 +1,204 @@
+import {expect} from "chai";
+import {isTransientFileLockError, retryOnTransientError} from "./retry";
+
+// A pip failure Node's execFile surfaces: `.message` is only "Command failed:
+// <cmd>", while the diagnostic (the WinError line) lands in `.stderr`. The
+// predicate has to look past `.message` or it will miss every real lock.
+function pipError(stderr: string): Error {
+    return Object.assign(new Error("Command failed: python -m pip install"), {
+        stderr,
+    });
+}
+
+describe("retry", () => {
+    describe("isTransientFileLockError", () => {
+        it("matches the WinError 32 'used by another process' lock", () => {
+            const error = pipError(
+                "ERROR: Could not install packages due to an OSError: " +
+                    "[WinError 32] The process cannot access the file because " +
+                    "it is being used by another process: " +
+                    "'...\\.venv\\Lib\\site-packages\\numpy\\_core\\_add_newdocs_scalars.py'"
+            );
+            expect(isTransientFileLockError(error)).to.equal(true);
+        });
+
+        it("does not match a bare WinError 5 'Access is denied' (can be a permanent permission error)", () => {
+            const error = pipError(
+                "ERROR: Could not install packages due to an OSError: " +
+                    "[WinError 5] Access is denied: '...\\numpy\\core.pyd'"
+            );
+            expect(isTransientFileLockError(error)).to.equal(false);
+        });
+
+        it("does not match a longer error code that merely starts with 32", () => {
+            const error = pipError(
+                "ERROR: Could not install packages due to an OSError: " +
+                    "[WinError 320] something unrelated"
+            );
+            expect(isTransientFileLockError(error)).to.equal(false);
+        });
+
+        it("does not match a non-lock failure (version not found)", () => {
+            const error = pipError(
+                "ERROR: Could not find a version that satisfies the " +
+                    "requirement databricks-connect==17.3.*"
+            );
+            expect(isTransientFileLockError(error)).to.equal(false);
+        });
+    });
+
+    describe("retryOnTransientError", () => {
+        const alwaysTransient = () => true;
+        const neverTransient = () => false;
+        const noopSleep = async () => {};
+
+        it("returns the result without sleeping when the operation succeeds", async () => {
+            let calls = 0;
+            let slept = 0;
+            const result = await retryOnTransientError(
+                async () => {
+                    calls++;
+                    return "ok";
+                },
+                {
+                    attempts: 3,
+                    delayMs: 5,
+                    isTransient: alwaysTransient,
+                    sleep: async () => {
+                        slept++;
+                    },
+                }
+            );
+
+            expect(result).to.equal("ok");
+            expect(calls).to.equal(1);
+            expect(slept).to.equal(0);
+        });
+
+        it("retries a transient failure until the operation succeeds", async () => {
+            let calls = 0;
+            const result = await retryOnTransientError(
+                async () => {
+                    calls++;
+                    if (calls < 3) {
+                        throw new Error("transient");
+                    }
+                    return "ok";
+                },
+                {
+                    attempts: 3,
+                    delayMs: 5,
+                    isTransient: alwaysTransient,
+                    sleep: noopSleep,
+                }
+            );
+
+            expect(result).to.equal("ok");
+            expect(calls).to.equal(3);
+        });
+
+        it("waits with linear backoff between attempts", async () => {
+            const delays: number[] = [];
+            let calls = 0;
+            await retryOnTransientError(
+                async () => {
+                    calls++;
+                    if (calls < 3) {
+                        throw new Error("transient");
+                    }
+                    return "ok";
+                },
+                {
+                    attempts: 3,
+                    delayMs: 5,
+                    isTransient: alwaysTransient,
+                    sleep: async (ms) => {
+                        delays.push(ms);
+                    },
+                }
+            );
+
+            // delayMs * attempt-number: 5 before the 2nd try, 10 before the 3rd.
+            expect(delays).to.deep.equal([5, 10]);
+        });
+
+        it("gives up and rethrows after exhausting attempts on a persistent transient error", async () => {
+            let calls = 0;
+            let error: unknown;
+            try {
+                await retryOnTransientError(
+                    async () => {
+                        calls++;
+                        throw new Error("still locked");
+                    },
+                    {
+                        attempts: 3,
+                        delayMs: 5,
+                        isTransient: alwaysTransient,
+                        sleep: noopSleep,
+                    }
+                );
+            } catch (e) {
+                error = e;
+            }
+
+            expect(calls).to.equal(3);
+            expect((error as Error)?.message).to.equal("still locked");
+        });
+
+        it("rethrows a non-transient error immediately without retrying", async () => {
+            let calls = 0;
+            let error: unknown;
+            try {
+                await retryOnTransientError(
+                    async () => {
+                        calls++;
+                        throw new Error("fatal");
+                    },
+                    {
+                        attempts: 3,
+                        delayMs: 5,
+                        isTransient: neverTransient,
+                        sleep: noopSleep,
+                    }
+                );
+            } catch (e) {
+                error = e;
+            }
+
+            expect(calls).to.equal(1);
+            expect((error as Error)?.message).to.equal("fatal");
+        });
+
+        it("reports each retry through the onRetry callback", async () => {
+            const seen: Array<{attempt: number; message: string}> = [];
+            let calls = 0;
+            await retryOnTransientError(
+                async () => {
+                    calls++;
+                    if (calls < 3) {
+                        throw new Error(`fail ${calls}`);
+                    }
+                    return "ok";
+                },
+                {
+                    attempts: 3,
+                    delayMs: 5,
+                    isTransient: alwaysTransient,
+                    sleep: noopSleep,
+                    onRetry: (attempt, e) => {
+                        seen.push({
+                            attempt,
+                            message: (e as Error).message,
+                        });
+                    },
+                }
+            );
+
+            expect(seen).to.deep.equal([
+                {attempt: 1, message: "fail 1"},
+                {attempt: 2, message: "fail 2"},
+            ]);
+        });
+    });
+});

packages/databricks-vscode/src/test/retry.ts

@@ -0,0 +1,70 @@
+// Retry helper for the e2e test harness. Kept here (under `src/test/`, not
+// `src/test/e2e/`) because `tsconfig.json` excludes the e2e folder from the
+// unit build, so a colocated unit test only runs when the module lives outside
+// it; the e2e specs import it via an explicit `.ts` extension.
+
+export interface RetryOptions {
+    // Total number of tries, including the first (so `attempts: 3` == 1 try + 2
+    // retries).
+    attempts: number;
+    // Base delay; the wait before retry N is `delayMs * N` (linear backoff).
+    delayMs: number;
+    // Only failures this returns `true` for are retried; everything else
+    // rethrows immediately so genuine breakage still surfaces fast.
+    isTransient: (error: unknown) => boolean;
+    // Called before each retry (not on the final give-up), for logging.
+    onRetry?: (attempt: number, error: unknown) => void;
+    // Injectable so tests run without real timers.
+    sleep?: (ms: number) => Promise<void>;
+}
+
+const defaultSleep = (ms: number): Promise<void> =>
+    new Promise((resolve) => setTimeout(resolve, ms));
+
+// Recognises the transient Windows file lock that makes a pip install fail even
+// though a moment later it would succeed: another process (an AV scanner, an
+// indexer, a still-running install) holding a file open, reported as WinError
+// 32 "the process cannot access the file because it is being used by another
+// process". We deliberately do NOT match WinError 5 "Access is denied", which
+// is just as often a permanent permission failure — retrying that only delays
+// the real error. Node's `execFile` surfaces only "Command failed: <cmd>" in
+// `.message` and puts the real OSError in `.stderr`, so we search every stream
+// the failure might carry its text in. `\b` keeps "WinError 32" from matching
+// unrelated codes like "WinError 320".
+export function isTransientFileLockError(error: unknown): boolean {
+    const parts: string[] = [];
+    if (error instanceof Error) {
+        parts.push(error.message);
+    }
+    const streams = error as {stderr?: unknown; stdout?: unknown};
+    if (typeof streams?.stderr === "string") {
+        parts.push(streams.stderr);
+    }
+    if (typeof streams?.stdout === "string") {
+        parts.push(streams.stdout);
+    }
+    return /being used by another process|WinError 32\b/i.test(
+        parts.join("\n")
+    );
+}
+
+export async function retryOnTransientError<T>(
+    operation: () => Promise<T>,
+    options: RetryOptions
+): Promise<T> {
+    const sleep = options.sleep ?? defaultSleep;
+    for (let attempt = 1; attempt < options.attempts; attempt++) {
+        try {
+            return await operation();
+        } catch (error) {
+            if (!options.isTransient(error)) {
+                throw error;
+            }
+            options.onRetry?.(attempt, error);
+            await sleep(options.delayMs * attempt);
+        }
+    }
+    // Final attempt: nothing left to retry, so let success return or the error
+    // propagate directly.
+    return await operation();
+}