fix(sqlite): only reach for an immutable URI where node:sqlite honours one

node:sqlite enables SQLITE_OPEN_URI from Node 22.15 on. Below that -- 22.13 is
the package floor -- a `file:...` location is a literal filename, so the
immutable open failed as CANTOPEN and the copy quietly stood in for it. That
was the right outcome by accident; the test asserted the newer behaviour and
failed on the floor.

The support question is now asked once per process, with an in-memory URI that
touches no filesystem whichever answer comes back, and the immutable open is
attempted only when the answer is yes. The test asks the same question rather
than skipping, so both CI lines assert something: rows are correct either way,
in place where URI filenames work and from a copy where they do not.
This commit is contained in:
iamtoruk 2026-08-18 10:38:01 -07:00
parent 9bfe9cc492
commit 60feaa8651
2 changed files with 31 additions and 6 deletions

View file

@ -153,6 +153,29 @@ function isSqliteSidecarError(err: unknown): boolean {
return typeof errcode === 'number' && (errcode & 0xff) === 14
}
let uriFilenamesSupported: boolean | null = null
/// node:sqlite only enables SQLITE_OPEN_URI from Node 22.15 on (measured: 22.13
/// and 22.14 fail, 22.15 and later work). Below that a `file:...` location is
/// taken literally and fails as CANTOPEN, so the immutable open is not attempted
/// there. The probe is an in-memory URI rather than a version comparison: it
/// answers the question directly and touches no filesystem either way.
export function sqliteSupportsUriFilenames(): boolean {
if (uriFilenamesSupported !== null) return uriFilenamesSupported
uriFilenamesSupported = false
const Driver = loadDriver() ? DatabaseSync : null
if (Driver !== null) {
try {
new Driver('file:codeburn-uri-probe?mode=memory', { readOnly: true }).close()
uriFilenamesSupported = true
} catch {
// An older build: locations are plain paths, and the copy fallback covers
// exactly the case the immutable open would have.
}
}
return uriFilenamesSupported
}
type DatabaseFingerprint = {
dev: number
ino: number
@ -362,11 +385,11 @@ function openReadonlyCache(path: string, originalError: unknown): DatabaseSyncIn
// An absent or empty -wal holds no frames, so there is nothing to go stale and
// nothing worth copying: immutable lets SQLite skip the -shm it cannot create
// and read the source in place.
if (fingerprint.walBytes === 0) {
if (fingerprint.walBytes === 0 && sqliteSupportsUriFilenames()) {
try {
return new Driver(`${pathToFileURL(path).href}?immutable=1`, { readOnly: true })
} catch {
// Older node:sqlite builds may not enable URI filenames. Copy instead.
// Understood but refused: the copy covers it.
}
}

View file

@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import {
isSqliteReadonlyError,
openDatabase,
sqliteSupportsUriFilenames,
} from '../src/sqlite.js'
import {
discoverSqliteSessions,
@ -158,7 +159,7 @@ describe('SQLite read-only parent fallback', () => {
expect(cachedDatabaseFiles()).toEqual([])
})
it('reads a read-only parent with no -wal in place, without copying it', ({ skip }) => {
it('reads a read-only parent with no -wal, in place where it can and by copy where it cannot', ({ skip }) => {
const dbPath = join(sourceRoot, 'state.vscdb')
createClosedWalDatabase(dbPath)
expect(existsSync(dbPath + '-wal')).toBe(false)
@ -169,9 +170,10 @@ describe('SQLite read-only parent fallback', () => {
expect(existsSync(dbPath + '-wal')).toBe(false)
expect(existsSync(dbPath + '-shm')).toBe(false)
// No WAL frames exist, so immutable reads the source in place: nothing to go
// stale, nothing to copy.
expect(cachedDatabaseFiles()).toEqual([])
// No WAL frames exist, so there is nothing to go stale and nothing worth
// copying: immutable reads the source in place. node:sqlite only honours
// URI filenames on newer builds, and on the 22.13 floor the copy stands in.
expect(cachedDatabaseFiles()).toHaveLength(sqliteSupportsUriFilenames() ? 0 : 1)
})
it('opens directly when a read-only parent already has WAL sidecars', ({ skip }) => {