← Back to PRs

#12122: fix(cron): ensure timer callback fires for scheduled jobs

by divol89 open 2026-02-08 21:54 View on GitHub →
stale
## Description Fixes #12081: Cron scheduler setTimeout callback never fires despite cronEnabled=true and nextWakeAtMs being set. ### Root Cause The `armTimer` function had a bug where it would unconditionally set `state.timer = null` before checking if a new timer should be created: ```typescript if (state.timer) { clearTimeout(state.timer); } state.timer = null; // <-- THIS HAPPENS ALWAYS if (!state.deps.cronEnabled) { return; // timer stays null! } const nextAt = nextWakeAtMs(state); if (!nextAt) { return; // timer stays null! } ``` If `cronEnabled` was false or `nextWakeAtMs` returned undefined, the timer would be cleared and set to null, but no new timer would be created. ### Changes 1. **Fixed timer clearing logic**: Only clear existing timer when we're about to create a new one 2. **Removed `unref()`**: The timer should keep the event loop alive for scheduled cron jobs 3. **Added debug logging**: Log timer arming details and callback invocation for easier diagnosis 4. **Maintain consistent state**: Set `state.timer = null` after `clearTimeout` ### Testing The fix ensures that: - When cron is disabled, existing timers are properly cleared - When no jobs are due (`nextWakeAtMs` returns undefined), timers are cleared - When jobs are due, a new timer is always created - Timer callback will fire at scheduled time - Event loop stays alive for scheduled jobs ### Checklist - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update **Wallet: BYCgQQpJTJT1odaunfvk6gtm5hVd7Xu93vYwbumFfqgHb3** <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR updates the cron timer arming logic in `src/cron/service/timer.ts` to avoid clearing timers in cases where no new timer will be scheduled, removes `unref()` to keep the event loop alive, and adds debug logging around timer arming and callback execution. It also changes the timer tick path to force-reload the cron store before running due jobs. However, the implementation currently calls `ensureLoaded` with an unsupported option (`skipRecompute`) and then performs an additional `recomputeNextRuns` after executing jobs, which can conflict with per-job scheduling updates already performed during `executeJob`. <h3>Confidence Score: 2/5</h3> - This PR is not safe to merge as-is due to a definite TypeScript API mismatch and scheduling logic that can override job state updates. - `ensureLoaded` is called with an option it does not accept, which should fail typechecking/build. Additionally, the new unconditional `recomputeNextRuns` after running due jobs can clobber `nextRunAtMs` values set during job execution, risking incorrect scheduling behavior. - src/cron/service/timer.ts (and its interaction with src/cron/service/store.ts and src/cron/service/jobs.ts) <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs