Agenda schedules an unparseable cron expression as a zero-millisecond interval, causing continuous job execution

Aperta
#43,393 2 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@Karman580 ci sta già lavorando.

Dal 1/11/2026.

  • #43394 di @Karman580 — aperta

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Ferma
Stack tecnologico
mongodb, nodejs, typescript
Ambito
backend

Direzione di ricerca

Start in packages/agenda/src/Job.ts, in _computeFromInterval(), where humanInterval(interval) || 0 turns an unparseable schedule into a zero-millisecond interval. Then read the finally guard and computeNextRunAt() to see why nextRunAt ends up equal to lastRunAt, and check _processDbResult() in packages/agenda/src/Agenda.ts. Reproduce with 0 0 1 12 * and */0 * * * *. Done means these schedules are rejected or recorded as failed, not re-run back to back. Linked PR #43394 already targets this, so check it before starting.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

type: bug
Summary

When CronTime cannot parse a repeat interval, Job._computeFromInterval() in packages/agenda/src/Job.ts falls back to humanInterval(). For a cron string that is neither a valid cron expression nor a human interval, humanInterval() returns NaN, which is coerced to 0 by || 0. The job's nextRunAt is then set to lastRunAt + 0, the job is never marked as failed, and Agenda keeps picking it up again as soon as each run completes.

The result is that a job with an unparseable schedule runs back to back, instead of being rejected or recorded as a scheduling failure.

Two schedules that reach this path:

  • 0 0 1 12 *: month 12 is out of range for [email protected] (months are 0-11), but it is accepted by the validCron() setting rule added in #43254, which uses cron-validator (months 1-12).
  • */0 * * * *: a zero step, e.g. what apps/meteor/server/cron/userDataDownloads.ts builds from UserData_ProcessingFrequency = 0.
Technical details

_computeFromInterval():

  1. new CronTime(interval) throws (Field (12) value is out of range, or Field (0-59/0) has a step of zero).
  2. The catch block falls back to humanInterval(interval) (L85), which returns NaN for these strings; || 0 turns it into 0.
  3. With lastRunAt set, L91 sets nextRunAt = lastRun + 0. On job creation (no lastRunAt, numberInterval falsy) the same line sets nextRunAt to now.
  4. The finally guard (L99-L102) only calls fail() when nextRunAt is null or NaN, so it does not trigger and failReason stays empty.

Why the job runs back to back rather than once per processEvery:

  • Job.run() sets lastRunAt and calls computeNextRunAt() before running (L192-L195), so nextRunAt is already in the past.
  • When the run finishes, the job is unlocked and saved (L212-L214).
  • _processDbResult() sees nextRunAt < _nextScanAt and calls processJobs(job) immediately (Agenda.ts L471-L474), and _lockOnTheFly() (L752) locks it again.

Reproduction and actual test output

Environment
  • Rocket.Chat develop at 7a174fd056
  • packages/agenda/src (Agenda.ts, Job.ts) loaded unmodified with tsx
  • Real mongod 7.0.24 started with mongodb-memory-server-core
  • Node v26.0.0, macOS 26.6.2, TZ=UTC
  • Dependency versions as pinned in yarn.lock:
Package Version
cron 1.8.2
cron-validator 1.3.1
human-interval 2.0.1
moment-timezone 0.5.48
moment 2.31.0
mongodb 6.16.0
Steps
  1. Create an Agenda with the same options as packages/cron/src/index.ts (db: { collection: 'rocketchat_cron' }, defaultConcurrency: 1, processEvery: '1 minute').
  2. Define three jobs whose handler records a timestamp and takes ~50 ms.
  3. Schedule them with agenda.every(schedule, name, {}, {}), the same call cronJobs.add() makes:
    • 0 0 1 12 *
    • */0 * * * *
    • 0 0 1 11 * (valid control)
  4. Observe for 75 seconds, then read the job documents.
Reproduction script
// Runs packages/agenda/src (Agenda + Job) unmodified against a real mongod, using the same
// Agenda options as packages/cron/src/index.ts and the same every(schedule, name, {}, {}) call.
const { MongoMemoryServer } = require('mongodb-memory-server-core');
const { MongoClient } = require('mongodb');
const { Agenda } = require(`${process.env.REPO}/packages/agenda/src/Agenda.ts`);

const JOBS: Record<string, string> = { 'month-12': '0 0 1 12 *', 'step-zero': '*/0 * * * *', 'control-month-11': '0 0 1 11 *' };

(async () => {
  const mongod = await MongoMemoryServer.create();
  const client = await MongoClient.connect(mongod.getUri());
  const db = client.db('repro');
  const buildInfo = await db.admin().command({ buildInfo: 1 });
  console.log(`node ${process.version}, mongod ${buildInfo.version}, TZ=${process.env.TZ}`);

  const agenda = new Agenda({ mongo: db, db: { collection: 'rocketchat_cron' }, defaultConcurrency: 1, processEvery: '1 minute' });
  agenda.on('error', () => {});
  const runs: Record<string, number[]> = {};
  const t0 = Date.now();
  for (const name of Object.keys(JOBS)) {
    runs[name] = [];
    // each run takes ~50 ms
    agenda.define(name, async () => { runs[name].push(Date.now() - t0); await new Promise((r) => setTimeout(r, 50)); });
  }
  await agenda.start();
  for (const [name, schedule] of Object.entries(JOBS)) await agenda.every(schedule, name, {}, {});

  const observeMs = Number(process.env.OBSERVE_MS);
  await new Promise((r) => setTimeout(r, observeMs));
  console.log(`observed for ${observeMs} ms`);
  for (const [name, t] of Object.entries(runs)) console.log(`${name.padEnd(17)} "${JOBS[name]}"  runs: ${t.length}  first run offsets (ms): ${t.slice(0, 8).join(', ')}${t.length > 8 ? ' ...' : ''}`);
  const docs = await db.collection('rocketchat_cron').find({}).toArray();
  for (const d of docs) console.log(`db ${d.name.padEnd(17)} nextRunAt=${d.nextRunAt?.toISOString() ?? null} lastRunAt=${d.lastRunAt?.toISOString() ?? '-'} failReason=${d.failReason ?? '-'}`);
  await agenda.stop().catch(() => {});
  await new Promise((r) => setTimeout(r, 500));
  await client.close(); await mongod.stop();
  process.exit(0);
})().catch((e) => { console.error(e); process.exit(1); });

Run with: TZ=UTC OBSERVE_MS=75000 REPO=<path to Rocket.Chat checkout> tsx repro-agenda.ts

Actual output
node v26.0.0, mongod 7.0.24, TZ=UTC
observed for 75000 ms
month-12          "0 0 1 12 *"  runs: 1398  first run offsets (ms): 45, 98, 150, 204, 260, 316, 371, 425 ...
step-zero         "*/0 * * * *"  runs: 1399  first run offsets (ms): 45, 98, 151, 205, 261, 316, 371, 426 ...
control-month-11  "0 0 1 11 *"  runs: 0  first run offsets (ms): 
db month-12          nextRunAt=2026-11-01T06:07:33.459Z lastRunAt=2026-11-01T06:07:33.459Z failReason=-
db step-zero         nextRunAt=2026-11-01T06:07:33.459Z lastRunAt=2026-11-01T06:07:33.459Z failReason=-
db control-month-11  nextRunAt=2026-12-01T00:00:00.000Z lastRunAt=- failReason=-

An earlier run of the same setup with only the 0 0 1 12 * job and the 0 0 1 11 * control recorded:

observed for 75000 ms
m12 runs: 1413 first run offsets (ms): 46, 100, 152, 208, 263, 317, 372, 428 ...
m11 runs: 0 first run offsets (ms):  
db m12 nextRunAt= 2026-11-01T05:55:13.028Z lastRunAt= 2026-11-01T05:55:13.028Z failReason= -
db m11 nextRunAt= 2026-12-01T00:00:00.000Z lastRunAt= - failReason= -

humanInterval() (human-interval 2.0.1) on these inputs:

"0 0 1 12 *" -> NaN
"*/0 * * * *" -> NaN
"5 minutes" -> 300000
Validator vs. scheduler, per month value

This is Job.computeNextRunAt() from packages/agenda/src/Job.ts called directly on a job whose lastRunAt is 2026-11-01T10:00:00Z:

Expression isValidCron (cron-validator 1.3.1) CronTime (cron 1.8.2) nextRunAt nextRunAt == lastRunAt
0 0 1 0 * false accepted 2027-01-01T00:00:00Z false
0 0 1 1 * true accepted 2027-02-01T00:00:00Z false
0 0 1 6 * true accepted 2027-07-01T00:00:00Z false
0 0 1 11 * true accepted 2026-12-01T00:00:00Z false
0 0 1 12 * true throws Field (12) value is out of range 2026-11-01T10:00:00Z true

For 0 0 1 12 *, a newly created job (repeatEvery(), no lastRunAt) got nextRunAt equal to the creation time (0 ms ahead) and no failReason.

Expected behaviour

An interval that is neither a valid cron expression nor a valid human interval should not produce a recurring job. It should be rejected, or the job should be recorded as failed (for example with nextRunAt: null and a failReason, as the existing finally guard intends). It should never become a job that is immediately due again after every run.

Actual behaviour

The fallback produces a 0 ms interval. nextRunAt equals lastRunAt, no failure is recorded, and the job runs again as soon as the previous run completes.

Root cause
  • packages/agenda/src/Job.ts
    • L85: humanInterval(interval) || 0 turns an unparseable string into 0.
    • L87-L93: nextRunAt becomes lastRun + 0.
    • L99-L102: the failure guard only checks for a null or NaN date, not for a 0 ms interval.
  • packages/agenda/src/Agenda.ts
    • L471-L474: after each save, a job whose nextRunAt is before the next scan is processed immediately.
    • L752: _lockOnTheFly() re-locks it.
Impact
  • Every job scheduled through cronJobs.add() is exposed. Any job whose schedule string fails CronTime parsing runs continuously instead of on its schedule, and nothing in the job document shows a failure.
  • Each run writes to CronHistory. Jobs added through packages/cron are wrapped in runCronJobFunctionAndPersistResult() (packages/cron/src/index.ts L9-L40), which inserts and updates one document per run.
  • Settings can reach this path:
    • 0 0 1 12 * passes the validCron() rule on RetentionPolicy_Advanced_Precision_Cron and the other cron settings.
    • UserData_ProcessingFrequency = 0 produces */0 * * * *.
  • Limitations: I have not measured this on a running Rocket.Chat server. The run counts above are from the reproduction script with a ~50 ms job; real jobs (for example retention pruning) would loop at a rate set by their own duration.
Related prior work
  • #43254 (PR) added validCron() to the cron settings, using cron-validator.
    • Its automated review flagged month 12 (discussion): isValidCron accepts it while [email protected] cannot parse it.
    • That thread was resolved without a code change.
    • The comment did not cover the Agenda fallback or the continuous execution described here.
  • #43253 (PR) notes that the server's [email protected] counts months from 0. That is why 12 is out of range for the scheduler.
  • Open PRs that touch Job.ts but not this fallback:
    • #41902 and #43012 move the computeNextRunAt() call in run().
    • #43012 also adds a Jest setup for packages/agenda.
Suggested direction

Agenda should not silently turn an unparseable cron expression into a 0 ms interval. One option: in the fallback, treat a NaN or 0 result from humanInterval() as invalid and leave nextRunAt null, so the existing fail() path records the failure. Valid human intervals such as '5 minutes' must keep working.

Separately, the validCron() setting rule could also reject expressions that [email protected] cannot parse, so admins get feedback when they save. That would not cover values already stored or values set through OVERWRITE_SETTING_*, so it complements an Agenda-side fix rather than replacing it.

I'd appreciate maintainer feedback on the preferred approach.

Suggested regression tests
  • Invalid cron with month 12 (0 0 1 12 *): nextRunAt is not equal to lastRunAt, and the job is recorded as failed (nextRunAt null, failReason set).
  • Zero step (*/0 * * * *): same expectations.
  • Valid cron 0 0 1 11 * (lastRunAt 2026-11-01T10:00:00Z, TZ=UTC): nextRunAt is 2026-12-01T00:00:00Z.
  • Valid human interval '5 minutes': nextRunAt is lastRunAt + 300000.
  • A job with an invalid schedule is not re-executed after its first run (with a running Agenda and MongoDB).
  • repeatEvery() on job creation with an invalid schedule does not produce an immediately due job.
Server Setup Information
  • Version of Rocket.Chat Server: develop at 7a174fd056 (source, packages/agenda)
  • NodeJS Version: v26.0.0
  • MongoDB Version: 7.0.24 (reproduction only)
  • Deployment Method / License / Users / Instances: not applicable, reproduced at the package level
Lingua principale
TypeScript
Stelle
46.5k
Fork
14.1k
Merge medio
1g 21h
PR unite (30g)
381

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di RocketChat/Rocket.Chat

Tutte le issue di RocketChat/Rocket.Chat

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.