From 2a690066a215438ed519c2765cbcc52a1b43e727 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 25 Oct 2026 17:00:52 +0100 Subject: [PATCH] test: busy timeout for E2E sqlite3 CLI seeding The sqlite3 CLI has no busy timeout by default, so an E2E suite that shells out to it while a real server/ingestor process holds a write lock on the same DB fails at once with "database is locked" instead of waiting (#606). Give every sqlite3 CLI call in the E2E suites a 5s busy timeout via '-cmd', '.timeout 5000', and add scripts/check-sqlite3-busy-timeout.js (wired into the js-test CI job) as a static regression check that every sqlite3 CLI invocation anywhere in the repo sets one. Relates to #606 --- .github/workflows/deploy.yml | 6 ++ scripts/check-sqlite3-busy-timeout.js | 115 +++++++++++++++++++++ scripts/test-check-sqlite3-busy-timeout.js | 84 +++++++++++++++ test-channel-proposals-e2e.js | 2 +- test-issue-315-estimated-positions-e2e.js | 6 +- test-region-scope-approval-e2e.js | 2 +- 6 files changed, 210 insertions(+), 5 deletions(-) create mode 100644 scripts/check-sqlite3-busy-timeout.js create mode 100644 scripts/test-check-sqlite3-busy-timeout.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 376acf7e6..2946afca2 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -294,6 +294,12 @@ jobs: node scripts/check-css-vars.js node scripts/test-check-css-vars.js + - name: Lint sqlite3 CLI busy timeout (issue #606) + run: | + set -e + node scripts/check-sqlite3-busy-timeout.js + node scripts/test-check-sqlite3-busy-timeout.js + - name: Run JS unit tests (test-all.sh) # One list: test-all.sh runs every unit file and prints a summary of # the failed ones; the step fails if any file fails (#174). diff --git a/scripts/check-sqlite3-busy-timeout.js b/scripts/check-sqlite3-busy-timeout.js new file mode 100644 index 000000000..3075c2be0 --- /dev/null +++ b/scripts/check-sqlite3-busy-timeout.js @@ -0,0 +1,115 @@ +#!/usr/bin/env node +/* + * scripts/check-sqlite3-busy-timeout.js — issue #606 + * + * Walks .js files and asserts that every process call invoking the + * `sqlite3` CLI (execFileSync/execSync/spawnSync/spawn) sets a busy + * timeout, either via `-cmd`/`.timeout N` CLI args or an embedded + * `PRAGMA busy_timeout=N` statement. + * + * Why: the sqlite3 CLI has no busy timeout by default. An E2E suite that + * shells out to it while a real server/ingestor process holds a write + * lock on the same DB fails at once with "database is locked" instead of + * waiting (#606), which is flaky rather than deterministic. + * + * Matching is a heuristic, not a JS parser: it finds `fn('sqlite3'` call + * heads, then walks forward counting '(' / ')' characters (including + * ones inside string/SQL arguments — SQL statements here keep their + * parens balanced, so this still lands on the real matching close-paren) + * to capture the full call's argument text, and checks that text for a + * timeout marker. + * + * Exit code: 0 = clean, 1 = one or more sqlite3 CLI calls missing a busy + * timeout (with file:line locations), 2 = usage/environment error. + * + * Usage: + * node scripts/check-sqlite3-busy-timeout.js # lints the repo + * node scripts/check-sqlite3-busy-timeout.js --dir x # lint a different directory + */ +'use strict'; +const fs = require('fs'); +const path = require('path'); + +let dir = path.join(__dirname, '..'); +for (let i = 2; i < process.argv.length; i++) { + if (process.argv[i] === '--dir' && process.argv[i + 1]) { dir = process.argv[++i]; } +} + +if (!fs.existsSync(dir)) { + console.error('check-sqlite3-busy-timeout: directory not found: ' + dir); + process.exit(2); +} + +const SKIP_DIRS = new Set(['node_modules', 'vendor', '.git']); +// Its own self-test embeds deliberately-bad "no timeout" call fixtures as +// literal JS source strings (written out to a tmp dir and linted there); +// scanned here in place they'd read as real violations. +const SKIP_FILES = new Set(['test-check-sqlite3-busy-timeout.js']); +function walk(root) { + const out = []; + const stack = [root]; + while (stack.length) { + const cur = stack.pop(); + let entries; + try { entries = fs.readdirSync(cur, { withFileTypes: true }); } + catch (e) { continue; } + for (const ent of entries) { + const full = path.join(cur, ent.name); + if (ent.isDirectory()) { + if (SKIP_DIRS.has(ent.name) || ent.name.startsWith('_wt-')) continue; + stack.push(full); + } else if (ent.isFile() && path.extname(ent.name) === '.js' && !SKIP_FILES.has(ent.name)) { + out.push(full); + } + } + } + return out; +} + +// Matches the head of a process call whose first argument is a string +// starting with "sqlite3" (covers both `fn('sqlite3', [...])` and +// `fn('sqlite3 ' + rest)` shell-string forms). +const CALL_HEAD_RE = /\b(?:execFileSync|execSync|execFile|exec|spawnSync|spawn)\s*\(\s*[`'"]sqlite3\b/g; +const TIMEOUT_RE = /\.timeout\b|busy_timeout/i; + +function findMatchingClose(src, openParenIdx) { + let depth = 0; + for (let i = openParenIdx; i < src.length; i++) { + if (src[i] === '(') depth++; + else if (src[i] === ')') { + depth--; + if (depth === 0) return i; + } + } + return -1; +} + +const violations = []; // { file, line } +let calls = 0; + +function scanFile(f) { + const src = fs.readFileSync(f, 'utf8'); + CALL_HEAD_RE.lastIndex = 0; + let m; + while ((m = CALL_HEAD_RE.exec(src)) !== null) { + const openParenIdx = src.indexOf('(', m.index); + if (openParenIdx === -1) continue; + const closeParenIdx = findMatchingClose(src, openParenIdx); + const span = closeParenIdx === -1 ? src.slice(openParenIdx) : src.slice(openParenIdx, closeParenIdx + 1); + calls++; + if (!TIMEOUT_RE.test(span)) { + const line = src.slice(0, m.index).split('\n').length; + violations.push({ file: f, line }); + } + } +} + +for (const f of walk(dir)) scanFile(f); + +if (violations.length) { + console.error('check-sqlite3-busy-timeout: ' + violations.length + ' sqlite3 CLI call(s) missing a busy timeout:'); + for (const v of violations) console.error(' ' + v.file + ':' + v.line); + console.error("Fix: add ['-cmd', '.timeout 5000', ...] to the call's args, or embed a 'PRAGMA busy_timeout=5000;' statement."); + process.exit(1); +} +console.log('check-sqlite3-busy-timeout: OK — ' + calls + ' sqlite3 CLI call(s), 0 missing a busy timeout.'); diff --git a/scripts/test-check-sqlite3-busy-timeout.js b/scripts/test-check-sqlite3-busy-timeout.js new file mode 100644 index 000000000..3ea48c90a --- /dev/null +++ b/scripts/test-check-sqlite3-busy-timeout.js @@ -0,0 +1,84 @@ +#!/usr/bin/env node +/* + * scripts/test-check-sqlite3-busy-timeout.js — gates scripts/check-sqlite3-busy-timeout.js + * + * #606: confirms the lint catches a sqlite3 CLI call with no busy timeout, + * accepts both ways of setting one (-cmd '.timeout N' args, or an embedded + * PRAGMA busy_timeout statement), and still matches a call whose argument + * list spans multiple lines (the shape used by the real E2E helpers, e.g. + * a schema-extraction call piping one sqlite3 invocation's output into + * another). + */ +'use strict'; +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'check-sqlite3-busy-timeout-test-')); +const lintPath = path.join(__dirname, 'check-sqlite3-busy-timeout.js'); +let pass = 0, fail = 0; + +function check(name, cond, info) { + if (cond) { console.log('PASS', name); pass++; } + else { console.error('FAIL', name, info || ''); fail++; } +} + +function writeOnly(name, content) { + for (const f of fs.readdirSync(tmp)) fs.unlinkSync(path.join(tmp, f)); + fs.writeFileSync(path.join(tmp, name), content); +} + +try { + // 1. A call with a busy timeout via -cmd/.timeout args is clean. + writeOnly('ok.js', + "const { execFileSync } = require('child_process');\n" + + "execFileSync('sqlite3', ['-cmd', '.timeout 5000', db, sql]);\n"); + let r = spawnSync('node', [lintPath, '--dir', tmp], { encoding: 'utf8' }); + check('timeout via -cmd args -> exit 0', r.status === 0, r.stderr || r.stdout); + + // 2. The same call with no timeout at all MUST be flagged, by file:line. + writeOnly('broken.js', + "const { execFileSync } = require('child_process');\n" + + "execFileSync('sqlite3', [db, sql]);\n"); + r = spawnSync('node', [lintPath, '--dir', tmp], { encoding: 'utf8' }); + check('no timeout -> exit 1', r.status === 1, 'got status=' + r.status); + check('error names the offending file:line', + /broken\.js:2/.test(r.stderr), 'stderr=' + r.stderr); + + // 3. An embedded PRAGMA busy_timeout statement is an accepted alternative. + writeOnly('pragma.js', + "const { execFileSync } = require('child_process');\n" + + "execFileSync('sqlite3', [db], { input: 'PRAGMA busy_timeout=5000;\\nINSERT INTO t VALUES (1);' });\n"); + r = spawnSync('node', [lintPath, '--dir', tmp], { encoding: 'utf8' }); + check('embedded PRAGMA busy_timeout -> exit 0', r.status === 0, r.stderr || r.stdout); + + // 4. A call whose args span multiple lines, with the timeout on a later + // line, must still be matched as ONE call (regression: a naive + // same-line-only scan would misreport this as missing). + writeOnly('multiline.js', + "const { execFileSync } = require('child_process');\n" + + "const schema = execFileSync('sqlite3', ['-cmd', '.timeout 5000',\n" + + " path.join(ROOT, 'fixture.db'),\n" + + " \"SELECT sql FROM sqlite_master WHERE name NOT LIKE 'sqlite_%'\"]);\n"); + r = spawnSync('node', [lintPath, '--dir', tmp], { encoding: 'utf8' }); + check('multi-line call with timeout -> exit 0', r.status === 0, r.stderr || r.stdout); + + // 5. A multi-line call with NO timeout must still be flagged (same span + // logic, opposite outcome). + writeOnly('multiline-broken.js', + "const { execFileSync } = require('child_process');\n" + + "const schema = execFileSync('sqlite3', [\n" + + " path.join(ROOT, 'fixture.db'),\n" + + " \"SELECT sql FROM sqlite_master WHERE name NOT LIKE 'sqlite_%'\"]);\n"); + r = spawnSync('node', [lintPath, '--dir', tmp], { encoding: 'utf8' }); + check('multi-line call without timeout -> exit 1', r.status === 1, 'got status=' + r.status); + check('multi-line violation names line 2', + /multiline-broken\.js:2/.test(r.stderr), 'stderr=' + r.stderr); +} finally { + for (const f of fs.readdirSync(tmp)) fs.unlinkSync(path.join(tmp, f)); + fs.rmdirSync(tmp); +} + +console.log('\n=== test-check-sqlite3-busy-timeout: ' + pass + ' pass, ' + fail + ' fail ==='); +process.exit(fail > 0 ? 1 : 0); diff --git a/test-channel-proposals-e2e.js b/test-channel-proposals-e2e.js index 5daddad47..38ee30eb6 100644 --- a/test-channel-proposals-e2e.js +++ b/test-channel-proposals-e2e.js @@ -379,7 +379,7 @@ async function main() { // this message after the revoke below, so a hidden channel is only // hidden from the list, never emptied. const sql = "INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) VALUES ('EEFF', 'e2e251history0001', '2024-01-01T00:00:00Z', 1, 5, '{\"type\":\"CHAN\",\"channel\":\"" + NAME + "\",\"text\":\"alice: hello\",\"sender\":\"alice\"}', '" + NAME + "')"; - execFileSync('sqlite3', [env.db, sql]); + execFileSync('sqlite3', ['-cmd', '.timeout 5000', env.db, sql]); }); await step('restart: the approval survives restarting ingestor and server', async () => { diff --git a/test-issue-315-estimated-positions-e2e.js b/test-issue-315-estimated-positions-e2e.js index d98102141..58aecb82d 100644 --- a/test-issue-315-estimated-positions-e2e.js +++ b/test-issue-315-estimated-positions-e2e.js @@ -252,11 +252,11 @@ async function browserChecks(browser, base, enabled, mode, assets, shots) { const database = path.join(scratch, 'synthetic.db'); // Copy only the tracked fixture's schema: none of its real mesh records // enter this suite. Migrations remain owned by the dedicated Go binary. - const schema = execFileSync('sqlite3', [path.join(ROOT, 'test-fixtures/e2e-fixture.db'), + const schema = execFileSync('sqlite3', ['-cmd', '.timeout 5000', path.join(ROOT, 'test-fixtures/e2e-fixture.db'), "SELECT sql || ';' FROM sqlite_master WHERE sql IS NOT NULL AND name NOT LIKE 'sqlite_%' ORDER BY CASE type WHEN 'table' THEN 0 ELSE 1 END, rowid"]); - execFileSync('sqlite3', [database], { input: schema }); + execFileSync('sqlite3', ['-cmd', '.timeout 5000', database], { input: schema }); execFileSync(migrateBin, ['-db', database], { stdio: 'pipe' }); - execFileSync('sqlite3', [database], { input: fs.readFileSync(path.join(ROOT, 'test-fixtures/seed-315-estimated-positions.sql')) }); + execFileSync('sqlite3', ['-cmd', '.timeout 5000', database], { input: fs.readFileSync(path.join(ROOT, 'test-fixtures/seed-315-estimated-positions.sql')) }); browser = await chromium.launch({ headless: true, executablePath: process.env.CHROMIUM_PATH || undefined }); for (const mode of ['default', 'enabled', 'disabled']) { const enabled = mode !== 'disabled'; diff --git a/test-region-scope-approval-e2e.js b/test-region-scope-approval-e2e.js index 91dda0d08..68c8ae0bd 100644 --- a/test-region-scope-approval-e2e.js +++ b/test-region-scope-approval-e2e.js @@ -134,7 +134,7 @@ async function main() { }); return { ingestor, server }; } - const sql = (statement) => execFileSync('sqlite3', [env.db, statement], { encoding: 'utf8' }).trim(); + const sql = (statement) => execFileSync('sqlite3', ['-cmd', '.timeout 5000', env.db, statement], { encoding: 'utf8' }).trim(); // null while the transmission is not stored yet; '' when stored unknown-scoped. const scopeOf = (raw) => { const out = sql("SELECT '[' || COALESCE(scope_name, '') || ']' FROM transmissions WHERE upper(raw_hex) = '" + raw + "'");