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