From 5e9a168b1d9ca4c8fd3b1d0b9f1bf3ed8b40dd51 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sun, 30 Aug 2026 02:04:56 -0700 Subject: [PATCH] fix(drivers): name a location when the missing-driver error searched nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-lands the change from #1192, which was merged but never reached `main`. Drivers are deliberately not shipped, so "driver not installed" is a normal state users hit by design rather than an edge case. `driverSearchRoots()` returns only directories that exist, so on a machine that has never installed a driver it returns nothing and the error ended in a bare "Searched 0 locations:" — a colon with nothing after it, naming nowhere to look. Say plainly that there was nothing to search, and name the `node_modules` directory the printed install command creates. Resolution behaviour is unchanged; this is the error text and one branch. Why this is reappearing: #1192 was squash-merged into its base branch, `fix/warehouse-driver-bootstrap` (#1122), rather than into `main`. #1122 had squash-merged to `main` 43 seconds earlier, so main's snapshot was already taken and this change was never carried across. Its merge commit `e005323` has the single parent `c49149a` — #1122's head, which is not an ancestor of `main` — and #1122 was the only PR from that branch, so nothing else was going to bring it over. Verified against live `main` rather than inferred: constructing `DriverNotInstalledError` with an empty searched list on `babc7cb` prints Searched 0 locations: with nothing after the colon, and none of this commit's text. The content is the cherry-pick of #1192's squashed result onto `main`. Ignoring blob hashes and hunk offsets it is byte-identical to that commit's own diff, and it applied without conflict, which independently confirms `main` already carries #1122's version of both files. Two tests, unchanged from #1192: the empty case must not render "Searched 0 locations:" and must name the install directory, and the populated case must still list the roots it actually searched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VqnuBDGkh1ZT65Ti7e6DHZ --- packages/drivers/src/resolve.ts | 12 ++++++-- packages/drivers/test/resolve-unit.test.ts | 32 +++++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/packages/drivers/src/resolve.ts b/packages/drivers/src/resolve.ts index 1d061ca7e..30193e4cd 100644 --- a/packages/drivers/src/resolve.ts +++ b/packages/drivers/src/resolve.ts @@ -91,11 +91,19 @@ export class DriverNotInstalledError extends Error { constructor(driver: DriverName, packages: readonly string[], searched: readonly string[]) { const label = DRIVER_LABELS[driver] + const installDir = driverInstallDir() + // `driverSearchRoots()` only returns directories that exist, so a compiled + // first run can have no searchable roots at all. The old message then ended + // in a bare "Searched 0 locations:" with nothing after the colon. Describe + // only what the empty list proves and name the managed location users need. + const searchedLine = searched.length + ? `Searched ${searched.length} location${searched.length === 1 ? "" : "s"}: ${searched.join(", ")}` + : `No searchable driver locations were found. Expected managed location: ${path.join(installDir, "node_modules")}.` super( `${label} driver not installed.\n` + `Install it with the warehouse_install_driver tool, or run:\n` + - ` npm install --prefix ${shellQuote(driverInstallDir())} ${packages.join(" ")}\n` + - `Searched ${searched.length} location${searched.length === 1 ? "" : "s"}: ${searched.join(", ")}`, + ` npm install --prefix ${shellQuote(installDir)} ${packages.join(" ")}\n` + + searchedLine, ) this.name = "DriverNotInstalledError" this.driver = driver diff --git a/packages/drivers/test/resolve-unit.test.ts b/packages/drivers/test/resolve-unit.test.ts index 2dd8732ee..cf0bcd535 100644 --- a/packages/drivers/test/resolve-unit.test.ts +++ b/packages/drivers/test/resolve-unit.test.ts @@ -313,7 +313,9 @@ describe("loadOptionalDriver", () => { }) test("throws DriverNotInstalledError naming the searched roots", async () => { - process.env["ALTIMATE_DRIVER_DIR"] = path.join(tmpRoot, "empty") + const managedRoot = path.join(tmpRoot, "empty", "node_modules") + fs.mkdirSync(managedRoot, { recursive: true }) + process.env["ALTIMATE_DRIVER_DIR"] = path.dirname(managedRoot) delete process.env["ALTIMATE_BIN_DIR"] delete process.env["NODE_PATH"] @@ -332,6 +334,7 @@ describe("loadOptionalDriver", () => { // target directory and no account of where we had looked. expect(err.message).toContain("--prefix") expect(err.message).toContain("Searched") + expect(err.message).toContain(managedRoot) }) test("reports a disk-resolved package that throws on import as a load failure", async () => { @@ -826,6 +829,33 @@ describe("manual-install hints are copy-pasteable", () => { expect(prefix!.startsWith("'") || prefix!.startsWith('"')).toBe(true) }) + test("DriverNotInstalledError names a location for an empty searched list", () => { + const installDir = path.join(tmpRoot, "absent") + process.env["ALTIMATE_DRIVER_DIR"] = installDir + + // Model the possible empty-root result directly. A unit-test checkout has + // legitimate executable/package roots, so forcing driverSearchRoots() to + // return [] here would be host-dependent. + const err = new DriverNotInstalledError("duckdb", DRIVER_PACKAGES.duckdb, []) + + expect(err.message).not.toContain("Searched 0 locations:") + expect(err.message).toContain("No searchable driver locations were found.") + expect(err.message).toContain(`Expected managed location: ${path.join(installDir, "node_modules")}.`) + // Still distinguishable from a broken install, and still actionable. + expect(err.message).toContain("DuckDB driver not installed.") + expect(err.message).toContain("npm install --prefix") + }) + + test("DriverNotInstalledError lists the roots it did search", () => { + const roots = [path.join(path.sep, "a", "node_modules"), path.join(path.sep, "b", "node_modules")] + + const err = new DriverNotInstalledError("duckdb", DRIVER_PACKAGES.duckdb, roots) + + expect(err.message).toContain("Searched 2 locations:") + for (const root of roots) expect(err.message).toContain(root) + expect(err.message).not.toContain("Searched nothing") + }) + test("no source builds a --prefix hint without shellQuote", () => { // Structural, because the behavioural tests can only cover the sites someone // remembered to write a case for. This fails when a NEW unquoted hint is