search

breejs / later

*Maintained fork of Later.* A javascript library for defining recurring schedules and calculating future (or past) occurrences for them. Includes support for using English phrases and Cron schedules. Works in Node and in the browser.
https://breejs.github.io/later/
MIT License
134 stars 13 forks source link

setInterval and setTimeout are limited to 24 days delay #34

Closed mau31415 closed 9 months ago

mau31415 commented 9 months ago

In old node version setTimeout was limited to 32bit timeout which translate to 24 days. On a larger numbers it would overflow. To avoid this issue, "Later" had capped the maximum delay to 32bit delay. However, this limit in node was resolved many years ago. This adjustment in "later" should be removed also.

t = diff < 2147483647 ? setTimeout(fn, diff) : setTimeout(scheduleTimeout, 2147483647);

this if needs to be removed.

titanism commented 9 months ago

@mau31415 can you please submit a PR? Thank you for this finding. 🙏

mau31415 commented 9 months ago

https://github.com/breejs/later/pull/35

titanism commented 9 months ago

This limit is not resolved as far as I can tell.

If you look at the docs at https://nodejs.org/api/timers.html#settimeoutcallback-delay-args it states:

When delay is larger than 2147483647 or less than 1, the delay will be set to 1. Non-integer delays are truncated to an integer.

Screen Shot 2023-11-28 at 3 05 07 PM
titanism commented 9 months ago

I am reverting that PR in the interim until further comments show that this is not the case. Also, a working test case would be great.

mau31415 commented 9 months ago

you are right, it is still the case... I'll try and have a better fix for it.

titanism commented 9 months ago

Okay! No worries, going to close this issue for now. If you have a workaround you want to implement please do. Note that I'm pushing a bunch of updates in the next few minutes to master branch, so you may want to update your fork (or delete it and re-fork it).