fix(events): core.lease keeps a module's retry: false on read/apply refusals #236
@@ -442,8 +442,12 @@ const ACTIONS = [
|
||||
|
||||
const baseline = await lease.read({ target })
|
||||
if (!baseline || baseline.ok !== true) {
|
||||
// A module that says `retry: false` has said a retry cannot help, so that
|
||||
// verdict is passed through; anything else stays undefined so the
|
||||
// dispatcher's own default (retry) still applies.
|
||||
return {
|
||||
ok: false,
|
||||
retry: baseline && baseline.retry === false ? false : undefined,
|
||||
error: baseline && baseline.error
|
||||
? String(baseline.error)
|
||||
: `could not read the current value of ${lease.label}`,
|
||||
@@ -494,7 +498,11 @@ const ACTIONS = [
|
||||
}
|
||||
if (!applied || applied.ok !== true) {
|
||||
await resourcesDb.markReverted(reserved.id)
|
||||
return { ok: false, error: applied && applied.error ? String(applied.error) : `${lease.label} refused the new value` }
|
||||
return {
|
||||
ok: false,
|
||||
retry: applied && applied.retry === false ? false : undefined,
|
||||
error: applied && applied.error ? String(applied.error) : `${lease.label} refused the new value`,
|
||||
}
|
||||
}
|
||||
|
||||
await resourcesDb.confirm(reserved.id)
|
||||
|
||||
@@ -813,6 +813,92 @@ test('a dry run of core.lease checks everything and takes nothing', async () =>
|
||||
assert.equal(bad.outcome, 'terminal')
|
||||
})
|
||||
|
||||
// ── A lease's refusal keeps its `retry: false` (website#206) ────────────────
|
||||
//
|
||||
// `core.lease` rebuilt both failure returns as `{ ok: false, error }`, so a module
|
||||
// that said "a retry cannot help" (a shard that refuses the value outright, a
|
||||
// target that does not exist) was dispatched as `retry: true` and the run paused
|
||||
// and re-asked on a loop. The module's `retry: false` now passes through, and
|
||||
// anything else is left undefined so the dispatcher's own default still applies.
|
||||
//
|
||||
// The reservation lands in MariaDB, which these tests do not have, so the two
|
||||
// calls `core.lease` makes on it are swapped for recorders and put back after.
|
||||
|
||||
const REFUSING_LEASES = `module.exports = (ctx, api) => {
|
||||
const lease = (id, read, apply) => ({
|
||||
id, label: 'Gain rate', type: 'float', min: 0.5, max: 5, maxDurationMs: 3600000,
|
||||
read, apply, async restore() { return { ok: true } },
|
||||
})
|
||||
const fine = async () => ({ ok: true, value: 1 })
|
||||
const accepts = async () => ({ ok: true })
|
||||
api.registerEventLeases([
|
||||
lease('demo.read.final', async () => ({ ok: false, retry: false, error: 'no such spawner' }), accepts),
|
||||
lease('demo.read.soft', async () => ({ ok: false, error: 'shard is restarting' }), accepts),
|
||||
lease('demo.apply.final', fine, async () => ({ ok: false, retry: false, error: 'value refused' })),
|
||||
lease('demo.apply.soft', fine, async () => ({ ok: false, error: 'shard is restarting' })),
|
||||
])
|
||||
}`
|
||||
|
||||
/** Run `fn` with the lease reservation calls recorded rather than sent to a database. */
|
||||
async function withStubbedReservations(fn) {
|
||||
// eslint-disable-next-line global-require
|
||||
const resourcesDb = require('../src/model/events/eventRunResources.db')
|
||||
const original = { reserve: resourcesDb.reserve, markReverted: resourcesDb.markReverted }
|
||||
const calls = { reserved: 0, reverted: [] }
|
||||
resourcesDb.reserve = async () => { calls.reserved += 1; return { ok: true, id: 77 } }
|
||||
resourcesDb.markReverted = async (id) => { calls.reverted.push(id) }
|
||||
try {
|
||||
return await fn(calls)
|
||||
} finally {
|
||||
Object.assign(resourcesDb, original)
|
||||
}
|
||||
}
|
||||
|
||||
const leaseStep = (lease) => step('core.lease', { lease, value: '3', minutes: 10 })
|
||||
|
||||
test('a lease read refusal that says retry: false is terminal, not retried', async () => {
|
||||
assertRegistered(loadModule('demo', REFUSING_LEASES))
|
||||
registries.registerCore()
|
||||
|
||||
await withStubbedReservations(async (calls) => {
|
||||
const result = await dispatch.dispatchStep(leaseStep('demo.read.final'), { run: RUN })
|
||||
assert.equal(result.outcome, 'terminal')
|
||||
assert.match(result.error, /no such spawner/)
|
||||
// Nothing was reserved, so there is nothing to give back.
|
||||
assert.equal(calls.reserved, 0)
|
||||
})
|
||||
})
|
||||
|
||||
test('a lease apply refusal that says retry: false is terminal, and the reservation is still released', async () => {
|
||||
assertRegistered(loadModule('demo', REFUSING_LEASES))
|
||||
registries.registerCore()
|
||||
|
||||
await withStubbedReservations(async (calls) => {
|
||||
const result = await dispatch.dispatchStep(leaseStep('demo.apply.final'), { run: RUN })
|
||||
assert.equal(result.outcome, 'terminal')
|
||||
assert.match(result.error, /value refused/)
|
||||
assert.equal(calls.reserved, 1)
|
||||
assert.deepEqual(calls.reverted, [77])
|
||||
})
|
||||
})
|
||||
|
||||
test('a lease refusal that does not say retry: false is still retried', async () => {
|
||||
assertRegistered(loadModule('demo', REFUSING_LEASES))
|
||||
registries.registerCore()
|
||||
|
||||
await withStubbedReservations(async (calls) => {
|
||||
const read = await dispatch.dispatchStep(leaseStep('demo.read.soft'), { run: RUN })
|
||||
assert.equal(read.outcome, 'retry')
|
||||
assert.match(read.error, /shard is restarting/)
|
||||
|
||||
const apply = await dispatch.dispatchStep(leaseStep('demo.apply.soft'), { run: RUN })
|
||||
assert.equal(apply.outcome, 'retry')
|
||||
assert.match(apply.error, /shard is restarting/)
|
||||
// The apply refusal released its reservation exactly as before.
|
||||
assert.deepEqual(calls.reverted, [77])
|
||||
})
|
||||
})
|
||||
|
||||
// ── Targeted leases and searchable sources (Phase 12b) ──────────────────────
|
||||
|
||||
test('a targeted lease is a family of values, and the target rides in the ref', async () => {
|
||||
|
||||
Reference in New Issue
Block a user