fix: Prevent double scheduling events and ensure long-running events work correctly #3561
Conversation
| // immediately. | ||
| if (event.recurring && eventDate <= now) { | ||
| this.#execute(event); | ||
| return; |
There was a problem hiding this comment.
Without this we would schedule after execution AND schedule below
|
|
||
| // If the existing event has not passed, start the timer. | ||
| if (eventDate >= now) { | ||
| this.#startTimer(event); |
There was a problem hiding this comment.
Without this we would ALWAYS reschedule cronjobs, even if their current date was valid
| cronjobController.destroy(); | ||
| }); | ||
|
|
||
| it('handles scheduled event close to current time gracefully', () => { |
There was a problem hiding this comment.
I couldn't find a good way to test this anymore since I eliminated the state update I relied on to replicate the race condition
| expect(handleRequest).toHaveBeenCalledTimes(1); | ||
|
|
||
| secondCronjobController.init(); | ||
| jest.advanceTimersByTime(inMilliseconds(26, Duration.Hour)); |
There was a problem hiding this comment.
This test did not seem to do what the title says, I re-used it to ensure that both of the fixes in this PR work
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3561 +/- ##
==========================================
- Coverage 98.29% 98.28% -0.02%
==========================================
Files 417 417
Lines 11759 11764 +5
Branches 1827 1829 +2
==========================================
+ Hits 11559 11562 +3
- Misses 200 202 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes two issues with the
CronjobController. We were double scheduling events that had not run since last boot since#executealso schedules the next call. Additionally we were not correctly scheduling events that run for longer than 24 hours, since the daily timer would re-calculate the execution date every day.