openvibers/openvibe.ai Commits

Price book: a null transport and lookup mean the defaults (every read failed in production) (#332)

OP OpenVibers committed 6 days ago · c8cd4799e9 · 3 files, +39 −4

* Price book: a null transport and lookup mean the defaults (every read failed in production)

The service's own instance passes transport: null and lookup: null (only tests bring their own). A default parameter
replaces undefined, not null, so in production every listing failed with "transport is not a function" and the first
run logged "checked 0 listings"; a null lookup would also have skipped the egress guard. The reader now treats null as
the default, and the run's log line names each unreadable listing and why.

* Price book: a run that read no listing at all leaves the book due (the next tick tries again)

Changes

ddiff --git a/server/pricebook/job.js b/server/pricebook/job.js
iindex bd598ee..687641e 100644
---- a/server/pricebook/job.js
++++ b/server/pricebook/job.js
@@@ -303,8 +303,9 @@ function createPriceBook({ db, registry, config = {}, clock = { now: () => Date.
// The claim goes back whatever happened, so the next worker may run.
await state.release({ who });
}
-- // The run is over: the next one is due when the interval has passed (whether or not a price moved).
-- await state.markRun();
++ // The run is over: the next one is due when the interval has passed (whether or not a price moved). A run
++ // that read no listing at all did not check anything, so the book stays due and the next tick tries again.
++ if (report.sources.length || !sources.length) await state.markRun();
} catch (err) {
report.error = err && err.code ? err.code : 'error';
log.warn('[pricebook] the run stopped early: ' + report.error);
@@@ -312,7 +313,7 @@ function createPriceBook({ db, registry, config = {}, clock = { now: () => Date.
running = false;
}
last = report;
-- log.log(`[pricebook] checked ${report.sources.length} listings, ${report.models} models, ${report.applied.length} applied, ${report.pending.length} waiting for the owner`);
++ log.log(`[pricebook] checked ${report.sources.length} listings, ${report.models} models, ${report.applied.length} applied, ${report.pending.length} waiting for the owner${report.unreadable.length ? `; unreadable: ${report.unreadable.map((u) => `${u.source} (${u.error})`).join(', ')}` : ''}`);
return report;
}
ddiff --git a/server/pricebook/read.js b/server/pricebook/read.js
iindex 797132f..3a30363 100644
---- a/server/pricebook/read.js
++++ b/server/pricebook/read.js
@@@ -66,7 +66,12 @@ function defaultTransport({ method, url, headers, lookup, timeoutMs, maxBytes })
* Read a listing, following the redirects by hand so each hop is judged on its own.
* → { ok, status, body, final_url, error }
*/
--async function readListing(url, { transport = defaultTransport, lookup = egress.createSafeLookup(), timeoutMs = 20000, maxBytes = 8 * 1024 * 1024 } = {}) {
++async function readListing(url, { transport = null, lookup = null, timeoutMs = 20000, maxBytes = 8 * 1024 * 1024 } = {}) {
++ // A caller that has no transport or lookup of its own passes null (the job does, outside tests): null means the
++ // default here. A default parameter only replaces undefined, so a null transport failed every read in production
++ // ("checked 0 listings", 2026-11-06) and a null lookup would have skipped the egress guard.
++ transport = transport || defaultTransport;
++ lookup = lookup || egress.createSafeLookup();
let current = String(url);
for (let hop = 0; hop <= MAX_REDIRECTS; hop++) {
let parsed;
ddiff --git a/test/price-book.test.js b/test/price-book.test.js
iindex 405d31b..b5d3086 100644
---- a/test/price-book.test.js
++++ b/test/price-book.test.js
@@@ -263,6 +263,35 @@ t.test('two instances at the same moment: only one reads the listings', async ()
}
});
++// The service's own instance passes null for the transport and the lookup (only tests bring their own): null must
++// mean the defaults. In production every read failed with "transport is not a function" (2026-11-06).
++t.test('a null transport and lookup are the defaults, not a failure', async () => {
++ const { readListing } = require('../server/pricebook/read');
++ const viaGuard = await readListing('https://price-book-test.example.com/models', {
++ transport: null,
++ lookup: (host, opts, cb) => cb(new Error('looked up through the given lookup')),
++ timeoutMs: 3000,
++ });
++ assert.strictEqual(viaGuard.ok, false);
++ assert.match(viaGuard.error, /looked up through the given lookup/, 'the default transport made the request');
++ const bare = await readListing('https://price-book-test.invalid/models', { transport: null, lookup: null, timeoutMs: 3000 });
++ assert.strictEqual(bare.ok, false);
++ assert.doesNotMatch(String(bare.error), /not a function/, 'a null transport is the default transport');
++});
++
++t.test('a run that reads no listing leaves the book due', async () => {
++ // No registry: an earlier subtest leaves a provider in drill, and this run reads nothing anyway.
++ const book = createPriceBook({ db: h.db, registry: null, config: { nodeEnv: 'test' }, log: { log() {}, warn() {} },
++ transport: async () => { throw new Error('offline'); } });
++ await h.db.prepare("DELETE FROM price_book_state").run();
++ const report = await book.runOnce({ force: true });
++ assert.ok(Array.isArray(report.sources), 'the run ran: ' + JSON.stringify(report).slice(0, 200));
++ assert.strictEqual(report.sources.length, 0, 'nothing was read');
++ assert.ok(report.unreadable.length > 0, 'every listing is unreadable');
++ const row = await h.db.prepare("SELECT last_run_at FROM price_book_state WHERE id = 'pricebook'").get();
++ assert.ok(!row || row.last_run_at == null, 'the book is still due');
++});
++
t.test('stop', async () => {
await h.stop();
});