From 4dba9bfb03e0afbe833c250a75ba0945bbe8234e Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sat, 29 Aug 2026 18:57:21 -0700 Subject: [PATCH 1/3] 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 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. Two tests: 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. --- packages/drivers/src/resolve.ts | 14 ++++++++++-- packages/drivers/test/resolve-unit.test.ts | 25 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/drivers/src/resolve.ts b/packages/drivers/src/resolve.ts index 9913be68b..c7de33d81 100644 --- a/packages/drivers/src/resolve.ts +++ b/packages/drivers/src/resolve.ts @@ -90,11 +90,21 @@ 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 on a machine + // that has never installed a driver it returns nothing at all — the normal + // first-run state, given drivers are deliberately not shipped. That left the + // message ending in a bare "Searched 0 locations:" with nothing after the + // colon, which names nowhere and reads like a bug. Say plainly that there was + // nothing to search, and name the directory the install command creates. + const searchedLine = searched.length + ? `Searched ${searched.length} location${searched.length === 1 ? "" : "s"}: ${searched.join(", ")}` + : `Searched nothing: no driver directory exists yet, not even ${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 ca9e62dac..b3bee4d28 100644 --- a/packages/drivers/test/resolve-unit.test.ts +++ b/packages/drivers/test/resolve-unit.test.ts @@ -608,6 +608,31 @@ describe("manual-install hints are copy-pasteable", () => { expect(prefix!.startsWith("'") || prefix!.startsWith('"')).toBe(true) }) + test("DriverNotInstalledError names a location when nothing was searched", () => { + process.env["ALTIMATE_DRIVER_DIR"] = path.join(path.sep, "tmp", "altimate-drivers-absent") + + // The first-run state: no driver directory exists, so driverSearchRoots() + // returns nothing. The message must still name somewhere. + const err = new DriverNotInstalledError("duckdb", DRIVER_PACKAGES.duckdb, []) + + expect(err.message).not.toContain("Searched 0 locations:") + expect(err.message).toContain("Searched nothing") + expect(err.message).toContain(path.join("altimate-drivers-absent", "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 From 31a73e1493994dfd0eb943f4289ebab4487e5818 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sat, 29 Aug 2026 21:53:27 -0700 Subject: [PATCH 2/3] fix(drivers): make empty-root diagnostic precise --- packages/drivers/src/resolve.ts | 12 +++++------- packages/drivers/test/resolve-unit.test.ts | 14 ++++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/drivers/src/resolve.ts b/packages/drivers/src/resolve.ts index c26cbf21a..30193e4cd 100644 --- a/packages/drivers/src/resolve.ts +++ b/packages/drivers/src/resolve.ts @@ -92,15 +92,13 @@ 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 on a machine - // that has never installed a driver it returns nothing at all — the normal - // first-run state, given drivers are deliberately not shipped. That left the - // message ending in a bare "Searched 0 locations:" with nothing after the - // colon, which names nowhere and reads like a bug. Say plainly that there was - // nothing to search, and name the directory the install command creates. + // `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(", ")}` - : `Searched nothing: no driver directory exists yet, not even ${path.join(installDir, "node_modules")}.` + : `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` + diff --git a/packages/drivers/test/resolve-unit.test.ts b/packages/drivers/test/resolve-unit.test.ts index d8b8c013d..1a95d37a7 100644 --- a/packages/drivers/test/resolve-unit.test.ts +++ b/packages/drivers/test/resolve-unit.test.ts @@ -826,16 +826,18 @@ describe("manual-install hints are copy-pasteable", () => { expect(prefix!.startsWith("'") || prefix!.startsWith('"')).toBe(true) }) - test("DriverNotInstalledError names a location when nothing was searched", () => { - process.env["ALTIMATE_DRIVER_DIR"] = path.join(path.sep, "tmp", "altimate-drivers-absent") + test("DriverNotInstalledError names a location for an empty searched list", () => { + const installDir = path.join(tmpRoot, "absent") + process.env["ALTIMATE_DRIVER_DIR"] = installDir - // The first-run state: no driver directory exists, so driverSearchRoots() - // returns nothing. The message must still name somewhere. + // 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("Searched nothing") - expect(err.message).toContain(path.join("altimate-drivers-absent", "node_modules")) + 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") From 212f4546a70255155f5efb4284517f3ab4d2271a Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sat, 29 Aug 2026 22:30:32 -0700 Subject: [PATCH 3/3] test(drivers): make searched-root assertion deterministic --- packages/drivers/test/resolve-unit.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/drivers/test/resolve-unit.test.ts b/packages/drivers/test/resolve-unit.test.ts index 1a95d37a7..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 () => {