-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Observable.timer unexpected/undocumented for large timeouts #3015
Comments
This is indeed a bug/unhandled strange edge case. I have a repro here: https://stackblitz.com/edit/angular-jlstzg?file=app%2Fapp.component.ts @benlesh What your thoughts on an argument check? |
This one's bit interesting edge case indeed. I think it's worth to note it in doc, but not sure about runtime check - as it falls into 32bit signed integer range (https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout) it implicitly follows maximum number range we could schedule. |
We could test to see if it's more than 2147483647, and if so, schedule an interval... basically something like this: if (ms > 2147483647) {
const MAX = 2147483647;
const n = Math.floor(ms / MAX)
const r = ms % MAX;
return concat(timer(MAX, MAX).pipe(take(n)), timer(r)).pipe(takeLast());
} else {
// usual timer path here.
} I mean we could make it work.. I'd probably do something more efficient than whats up there. A However it seems like it would be really weird for someone to want to do this, and they could just as easily do what I did above. |
FWIW: I agree it is an unusual edge case and the doc update may suffice, but an argument range check would benefit the naive consumer (e.g. me) who is primarily using the Date argument, rather than the number. In any case, thanks for considering and let me know if I can help! |
This is still happening and still problematic. |
Here is a workaround for large timeouts: |
I don't understand why this is an issue.. I'm doing timer(1000*60) for a 1 minute delay and it's still refreshing immediately. also @Azotgen s solution didn't work for me.
is constantly refreshing the page |
@westlakem, you don't need to pipe a delay for 1 minute, it's used for large timeouts only.
|
I also think that this edge case should be called out in the docs (or a link to the Mozilla docs page mentioned above) for naive users. Just ran into this issue in a production application and it wasn't very clear what the issue was. The use case was, we wanted to show a global banner message until a specified date and then hide it. The abbreviated code is below. It was working fine and showing the banner until August 5th 2020 and then the banner was hiding correctly until August 30th and then it started showing again. This was 25 days after the hide date which makes sense why it stopped working due to the setTimeout limitation private readonly showBannerUntilDate = moment.utc('2020-08-05T00:00:00.000Z');
public showBanner = new BehaviorSubject<boolean>(true);
constructor() {
timer(this.showBannerUntilDate.toDate())
.pipe(take(1))
.subscribe(() => {
let now = moment.utc();
if (now.isSameOrAfter(this.showBannerUntilDate)) {
this.showBanner.next(false);
}
});
} |
As another naive consumer using only the Date argument, exactly this behavior had quite an impact. We used it to make an update each quarter and schedule it again for the next quarter. As the next quarter at the time was less then 24 days away, everything worked fine. Unfortunately the code has been used (over an internal library) in multiple Microservices. Now in October with the new quarter being almost 90 days away, all those Microservices executed the function once each millisecond (NodeJS), leading to an immense amount of produced logging on GCP (~70.000 gibibyte), which produced a cost of several thousand US dollar in the week before it was discovered. |
We just got hit with this bug. We had a list of videos scheduled to go live for an event at specific Dates. It worked fine until someone scheduled a video for over a month in the future, and suddenly it thought that the far future video was the current video. It would be good if this limitation were called out in the docs. |
Also ran into the problem. The tsdoc (rxjs 7.8.0) for "operators/delay" even gives this example:
So I didn't expect this to not work. |
RxJS version: 5.5.2
Code to reproduce:
Expected behavior:
Should emit about a month from today (Oct 27, 2017).
Actual behavior:
Emits immediately.
Additional information:
I gather the timers has a max value of 2147483647 (2^31-1), and it makes sense to have such a limitation, but I would suggest an argument check and/or a warning or note in the documentation. An error seems justified as I cannot imagine someone passing in a value greater than that an expecting a 1 ms timeout.
The text was updated successfully, but these errors were encountered: