From 6b8a94ab3788932e06c4c154cc8810bbcfa85ea2 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Wed, 19 Aug 2026 21:07:24 +0000 Subject: [PATCH] Say which write path the process took, because the fallback is silent `REDIS_URL` has never been set on either production service. `connect()` falls back to the in-process queue when it is absent -- deliberately, as what a Redis outage should degrade to -- so every write has been serialized per process while the Redis queue (#132) and the folding built for it (#140) sat dormant. Nothing was broken and nothing was logged. "We have a write queue" and "the write queue is running" looked identical from outside until somebody read the environment, which is a bad property for the component that decides the crawler's throughput. So `connect()` now says which path it took and *why*: the reason is the point, since "in-process" alone cannot distinguish a deliberate local run from a production service missing its broker. Announced once per path per process, because the web app calls `connect()` per request. The decision moves into an exported `writePath()` that returns the path and the reason and opens nothing. That is what made this testable: the first attempt asserted against `connect()` itself and hung the suite, because the client it returns keeps the event loop alive. The decision is the part worth testing; the client is not. Co-Authored-By: Claude Opus 5 (1M context) --- packages/db/src/client.js | 47 +++++++++++- packages/db/test/write-path-announce.test.js | 79 ++++++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 packages/db/test/write-path-announce.test.js diff --git a/packages/db/src/client.js b/packages/db/src/client.js index 431e6f6..584ec00 100644 --- a/packages/db/src/client.js +++ b/packages/db/src/client.js @@ -50,13 +50,56 @@ export function connect(opts = {}) { // A file: URL is a local SQLite file — the tests and local development. It has // no remote transaction to protect and no second process contending for it, // and routing it through Redis would make the suite depend on a broker. - const wanted = opts.queue !== false && enabled && !url.startsWith('file:'); + const chosen = writePath({ url, redis, enabled, queue: opts.queue }); + announceWritePath(chosen); - if (wanted && redis) return queueWrites(client, { url: redis }); + if (chosen.path === 'redis') return queueWrites(client, { url: String(redis) }); return serializeWrites(client); } +/** + * Which write path these settings select, and why. + * + * Separated from `connect` so it can be asserted without opening a database. + * The decision is the part worth testing; the client it returns is not. + * + * @param {{ url: string, redis?: string, enabled: boolean, queue?: boolean }} settings + * @returns {{ path: 'redis'|'in-process', why: string }} + */ +export function writePath({ url, redis, enabled, queue }) { + if (queue === false) return { path: 'in-process', why: 'this caller drains the queue' }; + if (String(url).startsWith('file:')) return { path: 'in-process', why: 'local file database' }; + if (!enabled) return { path: 'in-process', why: 'WRITE_QUEUE is off' }; + if (!redis) return { path: 'in-process', why: 'REDIS_URL is not set' }; + return { path: 'redis', why: 'one writer per cluster' }; +} + +/** Paths already announced by this process, so a per-request `connect()` says it once. */ +const announced = new Set(); + +/** + * Say which write path this process took, once. + * + * The fallback to `serializeWrites` is deliberate and it is also silent, and + * the two together are a trap. `REDIS_URL` was never set on either production + * service, so every write went through the in-process queue while the Redis + * queue and the folding built for it sat dormant. Nothing was broken and + * nothing was logged, so "we have a write queue" and "the write queue is + * running" looked identical from outside until somebody read the environment. + * + * A line at boot is the whole fix. It names the reason rather than only the + * outcome, which is the difference between a degradation that was chosen and + * one that was inherited. + * + * @param {{ path: string, why: string }} chosen + */ +function announceWritePath(chosen) { + if (announced.has(chosen.path)) return; + announced.add(chosen.path); + console.log(`[db] write path: ${chosen.path} (${chosen.why})`); +} + /** * How long any single request to the database may take before it is abandoned. * diff --git a/packages/db/test/write-path-announce.test.js b/packages/db/test/write-path-announce.test.js new file mode 100644 index 0000000..eaa4ead --- /dev/null +++ b/packages/db/test/write-path-announce.test.js @@ -0,0 +1,79 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +import { writePath } from '../src/client.js'; + +/** + * Which write path a process takes, and whether it says so. + * + * The bug this exists for was not a failure. `REDIS_URL` was never set on + * either production service, so `connect()` silently fell back to the + * in-process queue while the Redis queue and its folding sat built and + * dormant. Everything worked; it simply was not the thing everyone believed + * was running, and nothing said otherwise. + */ + +const remote = 'libsql://example.turso.io'; + +test('a broker and a remote database select the Redis queue', () => { + const chosen = writePath({ url: remote, redis: 'redis://x:6379', enabled: true }); + + assert.equal(chosen.path, 'redis'); + assert.match(chosen.why, /cluster/); +}); + +test('no REDIS_URL falls back, and says that is why', () => { + // The exact production state this went unnoticed in, and the line that would + // have made it obvious. + const chosen = writePath({ url: remote, redis: undefined, enabled: true }); + + assert.equal(chosen.path, 'in-process'); + assert.match(chosen.why, /REDIS_URL/); +}); + +test('an empty REDIS_URL is treated as absent, not as a broker', () => { + const chosen = writePath({ url: remote, redis: '', enabled: true }); + + assert.equal(chosen.path, 'in-process'); + assert.match(chosen.why, /REDIS_URL/); +}); + +test('WRITE_QUEUE off is reported as a choice, not as a missing broker', () => { + const chosen = writePath({ url: remote, redis: 'redis://x:6379', enabled: false }); + + assert.equal(chosen.path, 'in-process'); + assert.match(chosen.why, /WRITE_QUEUE/); +}); + +test('the write worker itself never queues, however it is configured', () => { + // `queue: false` is the process that drains the queue. Routing it back + // through the queue would post every job straight back and nothing would + // ever reach the database. + const chosen = writePath({ url: remote, redis: 'redis://x:6379', enabled: true, queue: false }); + + assert.equal(chosen.path, 'in-process'); + assert.match(chosen.why, /drains/); +}); + +test('a local file database never reaches for a broker', () => { + // The test suite and local development. Routing a file: URL through Redis + // would make the suite depend on one. + const chosen = writePath({ url: 'file:/tmp/x.db', redis: 'redis://x:6379', enabled: true }); + + assert.equal(chosen.path, 'in-process'); + assert.match(chosen.why, /file/); +}); + +test('every path explains itself', () => { + // The reason is the point. "in-process" alone does not distinguish a + // deliberate local run from a production service missing its broker. + for (const settings of [ + { url: remote, redis: 'redis://x:6379', enabled: true }, + { url: remote, redis: undefined, enabled: true }, + { url: remote, redis: 'redis://x:6379', enabled: false }, + { url: 'file:/tmp/x.db', redis: undefined, enabled: true }, + ]) { + const chosen = writePath(settings); + assert.ok(chosen.why && chosen.why.length > 0, `no reason given for ${JSON.stringify(settings)}`); + } +});