-
Notifications
You must be signed in to change notification settings - Fork 626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: replace loop timeout by max match date #686
Conversation
for posterity: @sheerlox and I had a conversation about this on Discord. we decided that it's ok to take a long time to find the next cron execution time since that could be up to 8 years away. if it takes more than a few seconds to find the next execution time then the execution time is almost certainly farther away than how long it takes to find it (for example if at 8:00:00 pm you look for the next execution time and it takes you until 8:00:08 it's probably because the next execution time is hours, days, months, or years away and it won't cause errors to take so long finding it). we feel good closing #467 because this will remove the 5 second timeout (which we decided isn't needed) and also #499 which would have added a parameter to bypass the 5 second timeout. the ~8 year check should in theory never trigger but if it does it will alert us that something is wrong in the code since no cron execution time should ever be more than 8 years away by definition from the spec |
🎉 This PR is included in version 2.4.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [3.0.0-beta.2](v3.0.0-beta.1...v3.0.0-beta.2) (2023-08-15) ### 🐛 Bug Fixes * replace loop timeout by max match date ([#686](#686)) ([c685c63](c685c63)) ### 🚨 Tests * update new test for cron standard alignments ([ea56fc1](ea56fc1)) ### ⚙️ Continuous Integrations * **renovate:** configure renovate ([#683](#683)) ([9dbe962](9dbe962))
🎉 This PR is included in version 3.0.0-beta.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [2.4.1](kelektiv/node-cron@v2.4.0...v2.4.1) (2023-08-14) ### 🐛 Bug Fixes * replace loop timeout by max match date ([#686](kelektiv/node-cron#686)) ([c685c63](kelektiv/node-cron@c685c63)) ### ⚙️ Continuous Integrations * **renovate:** configure renovate ([#683](kelektiv/node-cron#683)) ([9dbe962](kelektiv/node-cron@9dbe962))
## [2.4.1](kelektiv/node-cron@v2.4.0...v2.4.1) (2023-08-14) ### 🐛 Bug Fixes * replace loop timeout by max match date ([kelektiv#686](kelektiv#686)) ([c685c63](kelektiv@c685c63)) ### ⚙️ Continuous Integrations * **renovate:** configure renovate ([kelektiv#683](kelektiv#683)) ([9dbe962](kelektiv@9dbe962))
## [2.4.1](kelektiv/node-cron@v2.4.0...v2.4.1) (2023-08-14) ### 🐛 Bug Fixes * replace loop timeout by max match date ([kelektiv#686](kelektiv#686)) ([c685c63](kelektiv@c685c63)) ### ⚙️ Continuous Integrations * **renovate:** configure renovate ([kelektiv#683](kelektiv#683)) ([9dbe962](kelektiv@9dbe962))
Description
This change is inspired by cronie's
crond
.It replaces the 5 seconds timeout in the
_getNextDateFrom
while loop by a maximum date in the future (8 years).Related Issue
Fixes #467.
Closes #499.
Motivation and Context
In environments with limited resources (CPU/memory), it may take more than 5 seconds to find the next date. This throws an error (see #467), while it should only take the time it needs and find the occurence.
How Has This Been Tested?
Using the
systemd-run
command, I've been able to compare the behavior of test runs between themain
branch and this version and to confirm that these changes have all tests passing correctly in a low resources environment.Types of changes
Checklist:
!
after the type/scope in the title (see the Conventional Commits standard).This issue was initially discussed on Discord.