From 965bf0a2dc0c37c9856185e0f76d3a8ab2378160 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 23 Oct 2026 14:48:26 +0200 Subject: [PATCH 1/6] Classify Deno KV cache damage in the runtime layer Deno KV reports a damaged cache through plain RangeError/TypeError/Error messages with no code or distinct class, so src/deno_ral/kv.ts classifies them by message text: a damaged value read through getKv is a miss with one warning per file, a damaged file raises KvDamagedError naming the folder to delete, and anything else is rethrown with the cache path. The tests damage real stores (node:sqlite edits, truncation, a text file, a zeroed b-tree page header) instead of mocking errors, so a Deno bump that rewords one of these messages fails a test rather than silently disabling the handling. printsMessage gains a count option so the once-per-file warning can be asserted. --- src/deno_ral/kv.ts | 91 ++++++++++++++ tests/unit/kv-damage-fixtures.ts | 117 ++++++++++++++++++ tests/unit/kv-damage.test.ts | 198 +++++++++++++++++++++++++++++++ tests/verify.ts | 17 ++- 4 files changed, 421 insertions(+), 2 deletions(-) create mode 100644 src/deno_ral/kv.ts create mode 100644 tests/unit/kv-damage-fixtures.ts create mode 100644 tests/unit/kv-damage.test.ts diff --git a/src/deno_ral/kv.ts b/src/deno_ral/kv.ts new file mode 100644 index 00000000000..154cedf0926 --- /dev/null +++ b/src/deno_ral/kv.ts @@ -0,0 +1,91 @@ +/* + * kv.ts + * + * Deno KV access that tells a damaged cache apart from other failures. + * + * Deno gives these errors no code, so classification is by message. + * + * Copyright (C) 2026 Posit Software, PBC + */ + +import { ErrorEx } from "../core/lib/error.ts"; +import { warning } from "./log.ts"; +import { dirname } from "./path.ts"; + +export class KvDamagedError extends ErrorEx { + readonly file: string; + constructor(file: string, cause: unknown) { + super( + "KvDamagedError", + `The Quarto cache ${file} is damaged (${messageOf(cause)}). ` + + `Delete the folder ${dirname(file)} and run Quarto again.`, + false, + false, + ); + this.file = file; + this.cause = cause; + } +} + +const messageOf = (e: unknown) => e instanceof Error ? e.message : String(e); + +const isFileDamage = (message: string) => + message.includes("database disk image is malformed") || + message.includes("file is not a database"); + +const isValueDamage = (e: unknown) => { + if (e instanceof RangeError) { + return e.message.includes("could not deserialize value"); + } + if (e instanceof TypeError) { + return (e.message.includes("value encoding") && + e.message.includes("is unknown")) || + e.message.startsWith("Invalid column type"); + } + return false; +}; + +const wrap = (file: string, e: unknown) => + new Error(`${messageOf(e)} (Quarto cache: ${file})`, { cause: e }); + +const warnedFiles = new Set(); + +export async function kvOp(file: string, op: () => Promise): Promise { + try { + return await op(); + } catch (e) { + if (isFileDamage(messageOf(e))) { + throw new KvDamagedError(file, e); + } + throw wrap(file, e); + } +} + +export function openKv(file: string): Promise { + return kvOp(file, () => Deno.openKv(file)); +} + +export async function getKv( + kv: Deno.Kv, + file: string, + key: Deno.KvKey, +): Promise { + try { + return (await kv.get(key)).value; + } catch (e) { + const message = messageOf(e); + if (isFileDamage(message)) { + throw new KvDamagedError(file, e); + } + if (isValueDamage(e)) { + if (!warnedFiles.has(file)) { + warnedFiles.add(file); + warning( + `Ignoring a damaged entry in the Quarto cache ${file} (${message}); it will be recomputed.`, + ); + } + return null; + } + throw wrap(file, e); + } +} diff --git a/tests/unit/kv-damage-fixtures.ts b/tests/unit/kv-damage-fixtures.ts new file mode 100644 index 00000000000..acb16bbd7c9 --- /dev/null +++ b/tests/unit/kv-damage-fixtures.ts @@ -0,0 +1,117 @@ +/* + * kv-damage-fixtures.ts + * + * Helpers that damage a closed Deno KV store the way real caches get damaged. + * + * Copyright (C) 2026 Posit Software, PBC + */ + +import { DatabaseSync } from "node:sqlite"; +import { assert, assertEquals } from "../asserts.ts"; + +// SET clauses for damageKvRow +export const kZeroValue = "v = zeroblob(length(v))"; +export const kUnknownEncoding = "v_encoding = 99"; +export const kTextValue = "v = 'text'"; + +const kTruncateSize = 16384; + +// Deno KV tuple encoding of an all-string key +export function kvKeyHex(key: string[]): string { + return key.map((part) => { + const hex = Array.from(new TextEncoder().encode(part)) + .map((b) => b.toString(16).padStart(2, "0")) + .join("") + .toUpperCase(); + return `02${hex}00`; + }).join(""); +} + +export function damageKvRow(file: string, key: string[], set: string): void { + const db = new DatabaseSync(file); + try { + const result = db + .prepare(`UPDATE kv SET ${set} WHERE hex(k) = ?`) + .run(kvKeyHex(key)); + assertEquals( + Number(result.changes), + 1, + `No kv row for key ${JSON.stringify(key)}`, + ); + } finally { + db.close(); + } +} + +export function truncateKvFile(file: string): void { + assert( + Deno.statSync(file).size > kTruncateSize, + `${file} is too small to truncate`, + ); + Deno.truncateSync(file, kTruncateSize); +} + +export function writeNonDatabase(file: string): void { + Deno.writeTextFileSync(file, "not a database"); +} + +// Zero the b-tree page header of the root page of the kv table. The store +// still opens; get, list and set then fail with "database disk image is +// malformed". +export function damageKvTableRoot(file: string): void { + let rootpage: number; + let pageSize: number; + const db = new DatabaseSync(file); + try { + const root = db + .prepare("SELECT rootpage FROM sqlite_master WHERE name = 'kv'") + .get() as { rootpage: number }; + const size = db.prepare("PRAGMA page_size").get() as { page_size: number }; + rootpage = Number(root.rootpage); + pageSize = Number(size.page_size); + } finally { + db.close(); + } + const f = Deno.openSync(file, { write: true }); + try { + f.seekSync((rootpage - 1) * pageSize, Deno.SeekMode.Start); + f.writeSync(new Uint8Array(8)); + } finally { + f.close(); + } +} + +const kLockHolder = `import { DatabaseSync } from "node:sqlite"; +const db = new DatabaseSync(Deno.args[0]); +db.exec("PRAGMA locking_mode=EXCLUSIVE; BEGIN EXCLUSIVE; COMMIT;"); +console.log("ready"); +for await (const _ of Deno.stdin.readable) {} +db.close();`; + +// Holds an exclusive lock on the store from a child process until released. +export async function holdKvLock( + file: string, +): Promise<{ release: () => Promise }> { + const child = new Deno.Command(Deno.execPath(), { + args: ["eval", kLockHolder, file], + stdin: "piped", + stdout: "piped", + stderr: "inherit", + }).spawn(); + const reader = child.stdout.pipeThrough(new TextDecoderStream()).getReader(); + let out = ""; + while (!out.includes("ready")) { + const { value, done } = await reader.read(); + if (done) { + throw new Error(`Lock holder exited before ready: ${out}`); + } + out += value; + } + return { + release: async () => { + await child.stdin.close(); + await reader.cancel(); + await child.status; + }, + }; +} diff --git a/tests/unit/kv-damage.test.ts b/tests/unit/kv-damage.test.ts new file mode 100644 index 00000000000..a8f8fbac578 --- /dev/null +++ b/tests/unit/kv-damage.test.ts @@ -0,0 +1,198 @@ +/* + * kv-damage.test.ts + * + * Copyright (C) 2026 Posit Software, PBC + */ + +import { + assert, + assertEquals, + assertRejects, + assertStringIncludes, +} from "../asserts.ts"; +import { test, unitTest } from "../test.ts"; +import { noErrors, printsMessage } from "../verify.ts"; +import { withTempDir } from "../utils.ts"; +import { dirname, join } from "../../src/deno_ral/path.ts"; +import { getKv, KvDamagedError, kvOp, openKv } from "../../src/deno_ral/kv.ts"; +import { + damageKvRow, + damageKvTableRoot, + kTextValue, + kUnknownEncoding, + kZeroValue, + truncateKvFile, + writeNonDatabase, +} from "./kv-damage-fixtures.ts"; + +const kWarnRegex = /Ignoring a damaged entry in the Quarto cache .*s\.kv/; + +async function buildStore( + file: string, + entries: Record, +) { + const kv = await Deno.openKv(file); + try { + for (const [k, v] of Object.entries(entries)) { + await kv.set([k], v); + } + } finally { + kv.close(); + } +} + +async function assertDamaged( + file: string, + open: () => Promise, + text: string, +) { + const before = Deno.readFileSync(file); + const e = await assertRejects(open, KvDamagedError); + assertStringIncludes(e.message, file); + assertStringIncludes(e.message, dirname(file)); + assertStringIncludes(e.message, text); + assert(!e.message.includes("\n")); + assertEquals(e.printName, false); + assertEquals(e.printStack, false); + assertEquals(e.file, file); + assertEquals(Deno.readFileSync(file), before); +} + +test({ + name: "kv - getKv reads a zeroed value as missing and warns once per file", + type: "unit", + context: {}, + execute: async () => { + await withTempDir(async (dir) => { + const file = join(dir, "s.kv"); + await buildStore(file, { a: 1, b: 2 }); + damageKvRow(file, ["a"], kZeroValue); + damageKvRow(file, ["b"], kZeroValue); + const kv = await openKv(file); + try { + assertEquals(await getKv(kv, file, ["a"]), null); + assertEquals(await getKv(kv, file, ["b"]), null); + } finally { + kv.close(); + } + }); + }, + verify: [ + noErrors, + printsMessage({ level: "WARN", regex: kWarnRegex, count: 1 }), + ], +}); + +test({ + name: + "kv - getKv reads an unknown value encoding or a text value as missing, and a set repairs it", + type: "unit", + context: {}, + execute: async () => { + await withTempDir(async (dir) => { + const file = join(dir, "s.kv"); + await buildStore(file, { a: 1, b: 2 }); + damageKvRow(file, ["a"], kUnknownEncoding); + damageKvRow(file, ["b"], kTextValue); + const kv = await openKv(file); + try { + assertEquals(await getKv(kv, file, ["a"]), null); + assertEquals(await getKv(kv, file, ["b"]), null); + await kv.set(["a"], 3); + await kv.set(["b"], 4); + assertEquals(await getKv(kv, file, ["a"]), 3); + assertEquals(await getKv(kv, file, ["b"]), 4); + } finally { + kv.close(); + } + }); + }, + verify: [ + noErrors, + printsMessage({ level: "WARN", regex: kWarnRegex, count: 1 }), + ], +}); + +unitTest( + "kv - openKv rejects a truncated store with KvDamagedError naming the folder", + async () => { + await withTempDir(async (dir) => { + const file = join(dir, "s.kv"); + await buildStore(file, { a: 1, b: 2 }); + truncateKvFile(file); + await assertDamaged( + file, + () => openKv(file), + "database disk image is malformed", + ); + }); + }, +); + +unitTest( + "kv - openKv rejects a non-database file with KvDamagedError", + async () => { + await withTempDir(async (dir) => { + const file = join(dir, "s.kv"); + writeNonDatabase(file); + await assertDamaged(file, () => openKv(file), "file is not a database"); + }); + }, +); + +unitTest( + "kv - a malformed page after open is file damage for getKv and for a whole list loop", + async () => { + await withTempDir(async (dir) => { + const file = join(dir, "s.kv"); + await buildStore(file, { + version: 1, + a: 1, + b: 2, + c: 3, + d: 4, + }); + damageKvTableRoot(file); + const kv = await openKv(file); + try { + await assertRejects( + () => getKv(kv, file, ["version"]), + KvDamagedError, + ); + await assertRejects( + () => + kvOp(file, async () => { + for await (const _ of kv.list({ prefix: [] })) { + // drain + } + }), + KvDamagedError, + ); + } finally { + kv.close(); + } + }); + }, +); + +unitTest("kv - a closed handle is not damage", async () => { + await withTempDir(async (dir) => { + const file = join(dir, "s.kv"); + const kv = await openKv(file); + kv.close(); + const e = await assertRejects(() => getKv(kv, file, ["a"]), Error); + assert(!(e instanceof KvDamagedError)); + assert(e.message.endsWith(`(Quarto cache: ${file})`), e.message); + assert(e.cause !== undefined); + }); +}); + +unitTest("kv - an unopenable path is not damage", async () => { + await withTempDir(async (dir) => { + const file = join(dir, "missing", "s.kv"); + const e = await assertRejects(() => openKv(file), Error); + assert(!(e instanceof KvDamagedError)); + assertStringIncludes(e.message, "unable to open database file"); + assertStringIncludes(e.message, `(Quarto cache: ${file})`); + }); +}); diff --git a/tests/verify.ts b/tests/verify.ts index d9b57dc9a6c..8efa89bd1ae 100644 --- a/tests/verify.ts +++ b/tests/verify.ts @@ -6,7 +6,7 @@ import { existsSync, walkSync } from "../src/deno_ral/fs.ts"; import { DOMParser, Element, NodeList } from "../src/core/deno-dom.ts"; -import { assert } from "testing/asserts"; +import { assert, assertEquals } from "testing/asserts"; import { basename, dirname, join, relative, resolve } from "../src/deno_ral/path.ts"; import { parseXmlDocument } from "slimdom"; import xpath from "fontoxpath"; @@ -183,8 +183,9 @@ export const printsMessage = (options: { level: "DEBUG" | "INFO" | "WARN" | "ERROR"; regex: string | RegExp; negate?: boolean; + count?: number; }): Verify => { - const { level, regex: regexPattern, negate = false } = options; // Set default here + const { level, regex: regexPattern, negate = false, count } = options; // Set default here return { name: `${level} matches ${String(regexPattern)}`, verify: (outputs: ExecuteOutput[]) => { @@ -192,6 +193,18 @@ export const printsMessage = (options: { ? new RegExp(regexPattern) : regexPattern; + if (count !== undefined) { + const found = outputs.filter((output) => + output.levelName === level && output.msg.match(regex) + ).length; + assertEquals( + found, + count, + `Expected ${count} ${level} ${String(regex)}, found ${found}`, + ); + return Promise.resolve(); + } + const printedMessage = outputs.some((output) => { return output.levelName === level && output.msg.match(regex); }); From 2433a52fc31cf91aa9d963c7b7ee446b443f306e Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 23 Oct 2026 15:03:57 +0200 Subject: [PATCH 2/6] Treat a damaged Sass cache entry as a miss (#15119) A damaged ["version"] row in the user-global Sass cache made every render fail with a bare "RangeError: could not deserialize value", the error in the report. Reading it through getKv turns the damaged value into null, so the existing null branch of checkVersion rewrites the row: the store repairs itself on the next render. A damaged entry row is a miss that the following setFromHash overwrites. A damaged file (truncated, or not SQLite) still fails the render, but the message now names the file and the v8- folder to delete. Quarto never deletes it: another process may hold the store, and on POSIX SQLite later removes the -wal/-shm files by path. If the version check fails after the store opened, the handle is closed so the folder stays deletable on Windows. --- src/core/sass/cache.ts | 54 ++++---- tests/unit/sass-cache.test.ts | 253 +++++++++++++++++++++++++++++++++- 2 files changed, 281 insertions(+), 26 deletions(-) diff --git a/src/core/sass/cache.ts b/src/core/sass/cache.ts index 53959dad64b..a757ee83b8f 100644 --- a/src/core/sass/cache.ts +++ b/src/core/sass/cache.ts @@ -16,18 +16,21 @@ import { safeRemoveIfExists } from "../path.ts"; import * as log from "../../deno_ral/log.ts"; import { onCleanup } from "../cleanup.ts"; import { Cloneable } from "../safe-clone-deep.ts"; +import { getKv, kvOp, openKv } from "../../deno_ral/kv.ts"; class SassCache implements Cloneable { kv: Deno.Kv; path: string; + kvFile: string; clone() { return this; } - constructor(kv: Deno.Kv, path: string) { + constructor(kv: Deno.Kv, path: string, kvFile: string) { this.kv = kv; this.path = path; + this.kvFile = kvFile; } async getFromHash( @@ -43,22 +46,20 @@ class SassCache implements Cloneable { throw new InternalError(`Invalid hash length: ${hash.length}`); } - const result = await this.kv.get(["entry", hash]); - if (result.value === null) { + const value = await getKv(this.kv, this.kvFile, ["entry", hash]); + if (value === null) { log.debug(` cache miss`); return null; } - if (typeof result.value !== "object") { + if (typeof value !== "object") { throw new InternalError( - `Unsupported SassCache entry type\nExpected SassCacheEntry, got ${typeof result - .value}`, + `Unsupported SassCache entry type\nExpected SassCacheEntry, got ${typeof value}`, ); } - const v = result.value as Record; + const v = value as Record; if (typeof v.key !== "string" || typeof v.hash !== "string") { throw new InternalError( - `Unsupported SassCache entry type\nExpected SassCacheEntry, got ${typeof result - .value}`, + `Unsupported SassCache entry type\nExpected SassCacheEntry, got ${typeof value}`, ); } const outputFilePath = join(this.path, `${hash}.css`); @@ -99,10 +100,11 @@ class SassCache implements Cloneable { } throw error; } - await this.kv.set(["entry", identifierHash], { - key: cacheIdentifier, - hash: inputHash, - }); + await kvOp(this.kvFile, () => + this.kv.set(["entry", identifierHash], { + key: cacheIdentifier, + hash: inputHash, + })); return outputFilePath; } @@ -174,24 +176,23 @@ const requiredQuartoVersions: Record = { }; async function checkVersion(kv: Deno.Kv, path: string) { - const version = await kv.get(["version"]); - if (version.value === null) { - await kv.set(["version"], 1); + const version = await getKv(kv, path, ["version"]); + if (version === null) { + await kvOp(path, () => kv.set(["version"], 1)); } else { - if (typeof version.value !== "number") { + if (typeof version !== "number") { throw new Error( - `Unsupported SassCache version type in ${path}\nExpected number, got ${typeof version - .value}`, + `Unsupported SassCache version type in ${path}\nExpected number, got ${typeof version}`, ); } - if (version.value < currentSassCacheVersion) { + if (version < currentSassCacheVersion) { // in the future we should clean this automatically, but this is v1 and there should be // no old data anywhere. throw new Error( `Found outdated SassCache version. Please clear ${path}.`, ); } - if (version.value > currentSassCacheVersion) { + if (version > currentSassCacheVersion) { throw new Error( `Found a SassCache version that's newer than supported. Please clear ${path} or upgrade Quarto to ${ requiredQuartoVersions[currentSassCacheVersion] @@ -212,9 +213,14 @@ export async function sassCache( log.debug(`Creating SassCache at ${storeDir}`); ensureDirSync(storeDir); const kvFile = join(storeDir, "sass.kv"); - const kv = await Deno.openKv(kvFile); - await checkVersion(kv, kvFile); - _sassCache[path] = new SassCache(kv, storeDir); + const kv = await openKv(kvFile); + try { + await checkVersion(kv, kvFile); + } catch (e) { + kv.close(); + throw e; + } + _sassCache[path] = new SassCache(kv, storeDir, kvFile); // register cleanup for this cache _sassCache[path].cleanup(temp, path); } diff --git a/tests/unit/sass-cache.test.ts b/tests/unit/sass-cache.test.ts index 6a26f690287..ebb694326fa 100644 --- a/tests/unit/sass-cache.test.ts +++ b/tests/unit/sass-cache.test.ts @@ -9,8 +9,14 @@ * Copyright (C) 2026 Posit Software, PBC */ -import { unitTest } from "../test.ts"; -import { assert, assertEquals, assertMatch } from "testing/asserts"; +import { test, unitTest, Verify } from "../test.ts"; +import { + assert, + assertEquals, + assertMatch, + assertRejects, + assertStringIncludes, +} from "../asserts.ts"; import { dirname, join } from "../../src/deno_ral/path.ts"; import { ensureDirSync, existsSync } from "../../src/deno_ral/fs.ts"; import { v8Branch } from "../../src/deno_ral/platform.ts"; @@ -18,6 +24,15 @@ import { safeRemoveIfExists } from "../../src/core/path.ts"; import { createTempContext, TempContext } from "../../src/core/temp.ts"; import { sassCache } from "../../src/core/sass/cache.ts"; import { withTempDir } from "../utils.ts"; +import { noErrors, printsMessage } from "../verify.ts"; +import { md5HashAsync } from "../../src/core/hash.ts"; +import { + damageKvRow, + damageKvTableRoot, + kZeroValue, + truncateKvFile, + writeNonDatabase, +} from "./kv-damage-fixtures.ts"; // A syntactically valid md5 hash that will never be present in a fresh cache, // so getFromHash exercises kv.get() and returns a clean cache miss (null). @@ -94,3 +109,237 @@ unitTest( }, "quarto-sass-cache-v8"); }, ); + +// Damaged-store tests (#15119). Each test owns a scratch dir created in setup +// and removed in teardown, so a handle leaked by a failed init cannot replace +// the primary failure. +const kStoreWarn = /Ignoring a damaged entry in the Quarto cache .*sass\.kv/; +const H1 = "b".repeat(32); +const H2 = "c".repeat(32); + +function sassDamageTest( + name: string, + body: (scratch: string) => Promise, + verify: Verify[] = [noErrors], +) { + let scratch = ""; + test({ + name, + type: "unit", + context: { + setup: () => { + scratch = Deno.makeTempDirSync({ prefix: "quarto-sass-damage" }); + return Promise.resolve(); + }, + teardown: () => { + Deno.removeSync(scratch, { recursive: true }); + return Promise.resolve(); + }, + }, + execute: () => body(scratch), + verify, + }); +} + +function storePaths(scratch: string) { + const cachePath = join(scratch, "sass"); + const storeDir = join(cachePath, `v8-${v8Branch}`); + return { cachePath, storeDir, file: join(storeDir, "sass.kv") }; +} + +// Writes a healthy store directly, so no sassCache() registry entry or +// process-global cleanup is involved. +async function seedStore(file: string, hashes: string[] = []) { + ensureDirSync(dirname(file)); + const kv = await Deno.openKv(file); + try { + await kv.set(["version"], 1); + for (const h of hashes) { + await kv.set(["entry", h], { key: "id", hash: INPUT_HASH }); + Deno.writeTextFileSync(join(dirname(file), `${h}.css`), ""); + } + } finally { + kv.close(); + } +} + +const writeCss = (p: string) => { + Deno.writeTextFileSync(p, "/* css */"); + return Promise.resolve(); +}; + +sassDamageTest( + "sassCache - a damaged version row warns once and is repaired (#15119)", + async (scratch) => { + const { cachePath, file } = storePaths(scratch); + await seedStore(file); + damageKvRow(file, ["version"], kZeroValue); + const temp = createTempContext(); + try { + const cache = await sassCache(cachePath, temp); + const second = await Deno.openKv(file); + try { + assertEquals((await second.get(["version"])).value, 1); + } finally { + second.close(); + } + const out = await cache.setFromHash(H1, INPUT_HASH, "id", writeCss); + assertEquals(await cache.getFromHash(H1, INPUT_HASH), out); + } finally { + temp.cleanup(); + } + }, + [noErrors, printsMessage({ level: "WARN", regex: kStoreWarn, count: 1 })], +); + +sassDamageTest( + "sassCache - a damaged entry is a miss and the next set overwrites it", + async (scratch) => { + const { cachePath, storeDir, file } = storePaths(scratch); + await seedStore(file, [H1, H2]); + damageKvRow(file, ["entry", H2], kZeroValue); + const temp = createTempContext(); + try { + const cache = await sassCache(cachePath, temp); + assertEquals( + await cache.getFromHash(H1, INPUT_HASH), + join(storeDir, `${H1}.css`), + ); + assertEquals(await cache.getFromHash(H2, INPUT_HASH), null); + const out = await cache.setFromHash(H2, INPUT_HASH, "id", writeCss); + assertEquals(await cache.getFromHash(H2, INPUT_HASH), out); + } finally { + temp.cleanup(); + } + }, + [noErrors, printsMessage({ level: "WARN", regex: kStoreWarn, count: 1 })], +); + +sassDamageTest( + "sassCache - compile failure after a damaged entry surfaces the compile error", + async (scratch) => { + const { cachePath, file } = storePaths(scratch); + const identifier = "id2"; + const h = await md5HashAsync(identifier); + await seedStore(file, [h]); + damageKvRow(file, ["entry", h], kZeroValue); + const temp = createTempContext(); + try { + const cache = await sassCache(cachePath, temp); + const e = await assertRejects( + () => + cache.getOrSet("input", identifier, () => { + throw new Error("sass boom"); + }), + Error, + ); + assertEquals(e.message, "sass boom"); + // The row must still be damaged: a failed compile never overwrites it. + const second = await Deno.openKv(file); + try { + const r = await assertRejects( + () => second.get(["entry", h]), + RangeError, + ); + assertStringIncludes(r.message, "could not deserialize value"); + } finally { + second.close(); + } + } finally { + temp.cleanup(); + } + }, + [noErrors, printsMessage({ level: "WARN", regex: kStoreWarn, count: 1 })], +); + +async function assertStoreRejected( + scratch: string, + damage: (file: string) => void, +) { + const { cachePath, storeDir, file } = storePaths(scratch); + await seedStore(file, [H1]); + damage(file); + const before = Deno.readFileSync(file); + const temp = createTempContext(); + try { + const e = await assertRejects(() => sassCache(cachePath, temp), Error); + assertStringIncludes(e.message, file); + assertStringIncludes(e.message, storeDir); + assertStringIncludes(e.message, "Delete the folder"); + assertEquals(Deno.readFileSync(file), before); + } finally { + temp.cleanup(); + } +} + +sassDamageTest( + "sassCache - a truncated store fails with the folder to delete and is left as is", + (scratch) => assertStoreRejected(scratch, truncateKvFile), +); + +sassDamageTest( + "sassCache - a non-database store fails the same way", + (scratch) => assertStoreRejected(scratch, writeNonDatabase), +); + +sassDamageTest( + "sassCache - a damaged store rejects again on the next resolve and stays deletable", + async (scratch) => { + const { cachePath, storeDir, file } = storePaths(scratch); + await seedStore(file, [H1]); + damageKvTableRoot(file); + const temp = createTempContext(); + try { + for (let i = 0; i < 2; i++) { + const e = await assertRejects(() => sassCache(cachePath, temp), Error); + assertStringIncludes(e.message, "Delete the folder"); + } + // Fails with os error 32 on Windows if a handle leaked. + Deno.removeSync(storeDir, { recursive: true }); + } finally { + temp.cleanup(); + } + }, +); + +sassDamageTest( + "sassCache - a damaged untagged legacy store is never opened", + async (scratch) => { + const { cachePath } = storePaths(scratch); + const legacy = join(cachePath, "sass.kv"); + await seedStore(legacy); + damageKvRow(legacy, ["version"], kZeroValue); + const before = Deno.readFileSync(legacy); + const temp = createTempContext(); + try { + const cache = await sassCache(cachePath, temp); + const out = await cache.setFromHash(H1, INPUT_HASH, "id", writeCss); + assertEquals(await cache.getFromHash(H1, INPUT_HASH), out); + } finally { + temp.cleanup(); + } + assertEquals(Deno.readFileSync(legacy), before); + }, + [noErrors, printsMessage({ level: "WARN", regex: /damaged/, negate: true })], +); + +sassDamageTest( + "sassCache - a repaired cache path still returns a live handle after cleanup (#14594)", + async (scratch) => { + const { cachePath, file } = storePaths(scratch); + await seedStore(file); + damageKvRow(file, ["version"], kZeroValue); + const temp1 = createTempContext(); + let temp2: TempContext | undefined; + try { + await sassCache(cachePath, temp1); + temp1.cleanup(); + temp2 = createTempContext(); + const cache = await sassCache(cachePath, temp2); + assertEquals(await cache.getFromHash(ABSENT_HASH, "ignored"), null); + } finally { + temp1.cleanup(); + temp2?.cleanup(); + } + }, +); From 03acee650251ba56202ed027585a50953d0166f2 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 23 Oct 2026 15:12:15 +0200 Subject: [PATCH 3/6] Treat a damaged project cache entry as a miss The project cache index (.quarto/project-cache/v8-/deno-kv-file) had the same failure shape as the Sass cache: a damaged version row or entry aborted the render with "could not deserialize value". Routing its KV calls through the runtime layer makes a damaged value a miss that the next write repairs, and a damaged file an error naming the folder to delete. createOnDisk() now awaits its set. Without the await, the repair write for a damaged version row ran detached, init() re-read the version before it landed, and a malformed-page failure rejected outside createProjectCache(). A failed init() closes the handle it opened, so a store that opens but fails its version read stays deletable on Windows. A locked index is not damage: it surfaces as Deno's "database is locked" with the cache path appended. --- src/core/cache/cache.ts | 130 ++++++++++++++++------------- tests/unit/project-cache.test.ts | 135 ++++++++++++++++++++++++++++++- 2 files changed, 209 insertions(+), 56 deletions(-) diff --git a/src/core/cache/cache.ts b/src/core/cache/cache.ts index 3d3ffc7a46e..bad960b2cd9 100644 --- a/src/core/cache/cache.ts +++ b/src/core/cache/cache.ts @@ -21,6 +21,7 @@ import { type ImmediateStringCacheEntry, } from "./cache-types.ts"; import { Cloneable } from "../safe-clone-deep.ts"; +import { getKv, kvOp, openKv } from "../../deno_ral/kv.ts"; export { type ProjectCache } from "./cache-types.ts"; const currentCacheVersion = "1"; @@ -31,6 +32,7 @@ const requiredQuartoVersions: Record = { class ProjectCacheImpl implements Cloneable { projectScratchDir: string; storeDir: string; + indexFile: string; index: Deno.Kv | null; constructor(_projectScratchDir: string) { @@ -40,6 +42,7 @@ class ProjectCacheImpl implements Cloneable { "project-cache", `v8-${v8Branch}`, ); + this.indexFile = join(this.storeDir, "deno-kv-file"); this.index = null; } @@ -56,47 +59,61 @@ class ProjectCacheImpl implements Cloneable { async clear(): Promise { assert(this.index); - const entries = this.index.list({ prefix: [] }); - for await (const entry of entries) { - const value = entry.value as CacheIndexEntry; - // immediate entries live in the index only; disk entries with - // identical content share one hash file - if ( - entry.key[0] !== "version" && - (value.type === "buffer" || value.type === "string") - ) { - const { hash } = value as DiskCacheEntry; - safeRemoveSync(join(this.storeDir, hash)); + const index = this.index; + await kvOp(this.indexFile, async () => { + const entries = index.list({ prefix: [] }); + for await (const entry of entries) { + const value = entry.value as CacheIndexEntry; + // immediate entries live in the index only; disk entries with + // identical content share one hash file + if ( + entry.key[0] !== "version" && + (value.type === "buffer" || value.type === "string") + ) { + const { hash } = value as DiskCacheEntry; + safeRemoveSync(join(this.storeDir, hash)); + } + await index.delete(entry.key); } - await this.index.delete(entry.key); - } + }); } async createOnDisk(): Promise { assert(this.index); - this.index.set(["version"], currentCacheVersion); + const index = this.index; + await kvOp( + this.indexFile, + () => index.set(["version"], currentCacheVersion), + ); } async init(): Promise { ensureDirSync(this.storeDir); - this.index = await Deno.openKv(join(this.storeDir, "deno-kv-file")); - let version = await this.index.get(["version"]); - if (version.value === null) { - await this.createOnDisk(); - version = await this.index.get(["version"]); - } - assert(typeof version.value === "string"); - const requiredVersion = requiredQuartoVersions[version.value]; - if ( - !requiredVersion || !satisfies(quartoConfig.version(), requiredVersion) - ) { - console.warn("Unknown project cache version."); - console.warn( - "Project cache was likely created by a newer version of Quarto.", - ); - console.warn("Quarto will clear the project cache."); - await this.clear(); - await this.createOnDisk(); + this.index = await openKv(this.indexFile); + try { + let version = await getKv(this.index, this.indexFile, [ + "version", + ]); + if (version === null) { + await this.createOnDisk(); + version = await getKv(this.index, this.indexFile, ["version"]); + } + assert(typeof version === "string"); + const requiredVersion = requiredQuartoVersions[version]; + if ( + !requiredVersion || !satisfies(quartoConfig.version(), requiredVersion) + ) { + console.warn("Unknown project cache version."); + console.warn( + "Project cache was likely created by a newer version of Quarto.", + ); + console.warn("Quarto will clear the project cache."); + await this.clear(); + await this.createOnDisk(); + } + } catch (e) { + this.close(); + throw e; } } @@ -104,10 +121,12 @@ class ProjectCacheImpl implements Cloneable { assert(this.index); const hash = await md5HashBytes(value); Deno.writeFileSync(join(this.storeDir, hash), value); - const result = await this.index.set(key, { - hash, - type: "buffer", - }); + const index = this.index; + const result = await kvOp(this.indexFile, () => + index.set(key, { + hash, + type: "buffer", + })); assert(result.ok); } @@ -116,38 +135,40 @@ class ProjectCacheImpl implements Cloneable { const buffer = new TextEncoder().encode(value); const hash = await md5HashBytes(buffer); Deno.writeTextFileSync(join(this.storeDir, hash), value); - const result = await this.index.set(key, { - hash, - type: "string", - }); + const index = this.index; + const result = await kvOp(this.indexFile, () => + index.set(key, { + hash, + type: "string", + })); assert(result.ok); } async addSmallBuffer(key: string[], value: Uint8Array): Promise { assert(this.index); - const result = await this.index.set(key, { - value, - type: "buffer-immediate", - }); + const index = this.index; + const result = await kvOp(this.indexFile, () => + index.set(key, { + value, + type: "buffer-immediate", + })); assert(result.ok); } async addSmallString(key: string[], value: string): Promise { assert(this.index); - const result = await this.index.set(key, { - value, - type: "string-immediate", - }); + const index = this.index; + const result = await kvOp(this.indexFile, () => + index.set(key, { + value, + type: "string-immediate", + })); assert(result.ok); } async get(key: string[]): Promise { assert(this.index); - const kvResult = await this.index.get(key); - if (kvResult.value === null) { - return null; - } - return kvResult.value as CacheIndexEntry; + return await getKv(this.index, this.indexFile, key); } async _getAs( @@ -168,7 +189,8 @@ class ProjectCacheImpl implements Cloneable { console.warn( `Entry ${result} not found -- clearing entry in index`, ); - await this.index.delete(key); + const index = this.index; + await kvOp(this.indexFile, () => index.delete(key)); return null; } } diff --git a/tests/unit/project-cache.test.ts b/tests/unit/project-cache.test.ts index 7ed72847b50..a3b5a285682 100644 --- a/tests/unit/project-cache.test.ts +++ b/tests/unit/project-cache.test.ts @@ -6,14 +6,27 @@ * Copyright (C) 2026 Posit Software, PBC */ -import { unitTest } from "../test.ts"; -import { assert, assertEquals } from "../asserts.ts"; +import { test, unitTest, Verify } from "../test.ts"; +import { + assert, + assertEquals, + assertRejects, + assertStringIncludes, +} from "../asserts.ts"; import { join } from "../../src/deno_ral/path.ts"; import { ensureDirSync, existsSync } from "../../src/deno_ral/fs.ts"; import { v8Branch } from "../../src/deno_ral/platform.ts"; import { md5HashBytes } from "../../src/core/hash.ts"; import { createProjectCache } from "../../src/core/cache/cache.ts"; import { withTempDir } from "../utils.ts"; +import { noErrors, printsMessage } from "../verify.ts"; +import { + damageKvRow, + damageKvTableRoot, + holdKvLock, + kZeroValue, + truncateKvFile, +} from "./kv-damage-fixtures.ts"; const LEGACY_SENTINEL = "legacy deno-kv-file owned by another Quarto version"; @@ -77,3 +90,121 @@ unitTest( }, "quarto-project-cache-clear"); }, ); + +// Damaged-store tests (#15119). Each test owns a scratch dir created in setup +// and removed in teardown, so a handle leaked by a failed init cannot replace +// the primary failure. +const kStoreWarn = + /Ignoring a damaged entry in the Quarto cache .*deno-kv-file/; + +function projectDamageTest( + name: string, + body: (scratch: string) => Promise, + verify: Verify[] = [noErrors], +) { + let scratch = ""; + test({ + name, + type: "unit", + context: { + setup: () => { + scratch = Deno.makeTempDirSync({ prefix: "quarto-project-damage" }); + return Promise.resolve(); + }, + teardown: () => { + Deno.removeSync(scratch, { recursive: true }); + return Promise.resolve(); + }, + }, + execute: () => body(scratch), + verify, + }); +} + +function storePaths(scratch: string) { + const storeDir = join(scratch, "project-cache", `v8-${v8Branch}`); + return { storeDir, file: join(storeDir, "deno-kv-file") }; +} + +async function seedStore(scratch: string) { + const cache = await createProjectCache(scratch); + try { + await cache.addString(["k"], "v"); + } finally { + cache.close(); + } +} + +projectDamageTest( + "projectCache - a damaged version row and entry warn once and are repaired", + async (scratch) => { + const { file } = storePaths(scratch); + await seedStore(scratch); + damageKvRow(file, ["version"], kZeroValue); + damageKvRow(file, ["k"], kZeroValue); + const cache = await createProjectCache(scratch); + try { + const second = await Deno.openKv(file); + try { + assertEquals((await second.get(["version"])).value, "1"); + } finally { + second.close(); + } + assertEquals(await cache.getString(["k"]), null); + await cache.addString(["k"], "w"); + assertEquals(await cache.getString(["k"]), "w"); + } finally { + cache.close(); + } + }, + [noErrors, printsMessage({ level: "WARN", regex: kStoreWarn, count: 1 })], +); + +projectDamageTest( + "projectCache - a truncated index fails with the folder to delete and is left as is", + async (scratch) => { + const { storeDir, file } = storePaths(scratch); + await seedStore(scratch); + truncateKvFile(file); + const before = Deno.readFileSync(file); + const e = await assertRejects(() => createProjectCache(scratch), Error); + assertStringIncludes(e.message, file); + assertStringIncludes(e.message, storeDir); + assertStringIncludes(e.message, "Delete the folder"); + assertEquals(Deno.readFileSync(file), before); + }, +); + +projectDamageTest( + "projectCache - a damaged index rejects again and stays deletable", + async (scratch) => { + const { storeDir, file } = storePaths(scratch); + await seedStore(scratch); + damageKvTableRoot(file); + for (let i = 0; i < 2; i++) { + const e = await assertRejects(() => createProjectCache(scratch), Error); + assertStringIncludes(e.message, "Delete the folder"); + } + // Fails with os error 32 on Windows if a handle leaked. + Deno.removeSync(storeDir, { recursive: true }); + }, +); + +projectDamageTest( + "projectCache - a locked index is not damage", + async (scratch) => { + const { file } = storePaths(scratch); + await seedStore(scratch); + const before = Deno.readFileSync(file); + const lock = await holdKvLock(file); + try { + const e = await assertRejects(() => createProjectCache(scratch), Error); + assertStringIncludes(e.message, "database is locked"); + assertStringIncludes(e.message, `(Quarto cache: ${file})`); + assert(!e.message.includes("Delete the folder")); + } finally { + await lock.release(); + } + assertEquals(Deno.readFileSync(file), before); + }, +); From b84f2be71cf0b3327180dddf5df0a90d25086595 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 23 Oct 2026 15:20:54 +0200 Subject: [PATCH 4/6] Accept the KV damage tests' Deno calls in the runtime boundary baseline src drops by two: both caches now open their store through src/deno_ral/kv.ts instead of calling Deno.openKv directly. tests rise because the damage fixtures build, damage and inspect real Deno KV stores (Deno.openKv, truncateSync, a lock-holding child process), which a mocked error could not replace. tests.stdImportFiles drops by one because sass-cache.test.ts now imports its assertions from tests/asserts.ts. --- tools/runtime-boundary-baseline.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/runtime-boundary-baseline.json b/tools/runtime-boundary-baseline.json index 2810b910744..40ba250ba8e 100644 --- a/tools/runtime-boundary-baseline.json +++ b/tools/runtime-boundary-baseline.json @@ -1,6 +1,6 @@ { "src": { - "denoCalls": 993, + "denoCalls": 991, "stdImportFiles": 49 }, "src-js": { @@ -16,7 +16,7 @@ "stdImportFiles": 3 }, "tests": { - "denoCalls": 838, - "stdImportFiles": 177 + "denoCalls": 870, + "stdImportFiles": 176 } } From 221d31af1fde38ae51ceb1c9754e7cd5a14d2ca4 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 23 Oct 2026 15:21:00 +0200 Subject: [PATCH 5/6] Document the KV damage handling Adds the kv.ts row to the deno_ral module table and the #15119 changelog entry. --- .claude/rules/typescript/deno-ral.md | 1 + news/changelog-1.11.md | 1 + 2 files changed, 2 insertions(+) diff --git a/.claude/rules/typescript/deno-ral.md b/.claude/rules/typescript/deno-ral.md index b0678808f31..8c9b82d3518 100644 --- a/.claude/rules/typescript/deno-ral.md +++ b/.claude/rules/typescript/deno-ral.md @@ -14,6 +14,7 @@ The `deno_ral/` directory provides a runtime abstraction layer over Deno's stand | `fs.ts` | File system | `existsSync`, `ensureDirSync`, `copySync`, `safeMoveSync`, `safeRemoveSync` | | `path.ts` | Path utilities | `join`, `dirname`, `basename`, `extname` | | `log.ts` | Logging | `debug`, `info`, `warning`, `error` | +| `kv.ts` | Deno KV cache stores | `openKv`, `getKv`, `kvOp`, `KvDamagedError` | | `platform.ts` | Platform detection | `isWindows` | | `process.ts` | Process execution | Process utilities | diff --git a/news/changelog-1.11.md b/news/changelog-1.11.md index 4c4907daaf0..e1b823c6e9e 100644 --- a/news/changelog-1.11.md +++ b/news/changelog-1.11.md @@ -188,6 +188,7 @@ All changes included in 1.11: - ([#14878](https://github.com/quarto-dev/quarto-cli/pull/14878)): Add `az` (Azerbaijani) language translation. (author: @abdanar) - ([#14983](https://github.com/quarto-dev/quarto-cli/issues/14983)): Fix `quarto render` treating a parent folder as a Hugo project when its `config/` folder can't be entered, which silently wrote `hugo-md` output or failed with a permission error. (author: @pinin4fjords) - ([#15097](https://github.com/quarto-dev/quarto-cli/pull/15097)): Fix the `.content-hidden unless-profile` attribute completion inserting invalid syntax (its `=` was missing), and typos in three attribute completion descriptions. (author: @rkskmt) +- ([#15119](https://github.com/quarto-dev/quarto-cli/issues/15119)): Fix renders failing with `RangeError: could not deserialize value` when a Quarto cache file is damaged. A damaged entry is now recomputed, and a damaged cache file is reported with the folder to delete. - ([#15126](https://github.com/quarto-dev/quarto-cli/issues/15126)): Translate the `revealjs-menu-button-label` language key in French. (author: @D3ller) - ([#15140](https://github.com/quarto-dev/quarto-cli/pull/15140)): Add support for definition lists written as a `definition-list` div holding a nested bullet list, the definition list syntax of Quarto 2. - ([#15180](https://github.com/quarto-dev/quarto-cli/pull/15180)): Fix older Quarto versions failing to render with `RangeError: could not deserialize value` once a Quarto built on a newer Deno has used the same cache. Also fix a project cache that already has entries failing to clear with `Path must be a string`. From 1cb6a368d5f472d7260964a7c270688379bcca16 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 23 Oct 2026 15:26:46 +0200 Subject: [PATCH 6/6] Check a damaged store rejects again with the same message quarto preview re-resolves the cache on every re-render, so a second resolve on the same damaged store must give the user the same actionable message, not a different error from leftover state. --- tests/unit/project-cache.test.ts | 3 +++ tests/unit/sass-cache.test.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/tests/unit/project-cache.test.ts b/tests/unit/project-cache.test.ts index a3b5a285682..92864fa51dd 100644 --- a/tests/unit/project-cache.test.ts +++ b/tests/unit/project-cache.test.ts @@ -181,10 +181,13 @@ projectDamageTest( const { storeDir, file } = storePaths(scratch); await seedStore(scratch); damageKvTableRoot(file); + const errors: Error[] = []; for (let i = 0; i < 2; i++) { const e = await assertRejects(() => createProjectCache(scratch), Error); assertStringIncludes(e.message, "Delete the folder"); + errors.push(e); } + assertEquals(errors[1].message, errors[0].message); // Fails with os error 32 on Windows if a handle leaked. Deno.removeSync(storeDir, { recursive: true }); }, diff --git a/tests/unit/sass-cache.test.ts b/tests/unit/sass-cache.test.ts index ebb694326fa..e75ae1f768b 100644 --- a/tests/unit/sass-cache.test.ts +++ b/tests/unit/sass-cache.test.ts @@ -290,10 +290,13 @@ sassDamageTest( damageKvTableRoot(file); const temp = createTempContext(); try { + const errors: Error[] = []; for (let i = 0; i < 2; i++) { const e = await assertRejects(() => sassCache(cachePath, temp), Error); assertStringIncludes(e.message, "Delete the folder"); + errors.push(e); } + assertEquals(errors[1].message, errors[0].message); // Fails with os error 32 on Windows if a handle leaked. Deno.removeSync(storeDir, { recursive: true }); } finally {