Skip to content
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

Open
JorgeLGonzalez opened this issue Oct 28, 2017 · 13 comments · May be fixed by #7213
Open

Observable.timer unexpected/undocumented for large timeouts #3015

JorgeLGonzalez opened this issue Oct 28, 2017 · 13 comments · May be fixed by #7213

Comments

@JorgeLGonzalez
Copy link

JorgeLGonzalez commented Oct 28, 2017

RxJS version: 5.5.2

Code to reproduce:

Rx.Observable
    .timer(new Date(2017, 10, 28))
    .subscribe(() => {
        console.log('Immediate emit');
    });

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.

@davideast
Copy link

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?

@kwonoj
Copy link
Member

kwonoj commented Oct 30, 2017

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.

@benlesh
Copy link
Member

benlesh commented Oct 31, 2017

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 setInterval with a counter or something. Or add support directly to the AsapScheduler.

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.

@JorgeLGonzalez
Copy link
Author

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!

@GregRos
Copy link

GregRos commented Jan 31, 2018

This is still happening and still problematic. Observable.timer(Infinity) makes sense and is allowed by the signature and the documentation. Why not just compare the interval to MAX_INT, if it's higher, replace it with Observable.never?

@rohiievych
Copy link

Here is a workaround for large timeouts:
timer().pipe(delay(timeout))

@westlakem
Copy link

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.

    const t = timer().pipe(delay(1000 * 60));
    t.subscribe(location.reload());

is constantly refreshing the page

@rohiievych
Copy link

@westlakem, you don't need to pipe a delay for 1 minute, it's used for large timeouts only.
The problem is that you have to pass a callback to make a correct subscription:

timer(1000 * 60).subscribe(() => location.reload());

@thecheeto
Copy link

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 This causes an integer overflow when using delays larger than 2,147,483,647 ms (about 24.8 days). However, its slightly different then the original issue because the timer never emits.

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);
        }
      });
}

@StenCalDabran
Copy link

StenCalDabran commented Oct 8, 2021

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.
Of course this is our error for using the library wrong, but such cases can happen. Please update at least the documentation (although in my opinion @benlesh 's solution produces the actually expected behavior).

@karptonite
Copy link

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.

@aholtkamp
Copy link

aholtkamp commented Mar 9, 2023

Also ran into the problem.

The tsdoc (rxjs 7.8.0) for "operators/delay" even gives this example:

const clicks = fromEvent(document, 'click');
const date = new Date('March 15, 2050 12:00:00'); // in the future
const delayedClicks = clicks.pipe(delay(date)); // click emitted only after that date
delayedClicks.subscribe(x => console.log(x));

So I didn't expect this to not work.

@wyrfel
Copy link

wyrfel commented Nov 12, 2023

For anyone who needs it...here are some workaround replacement implementations based on @benlesh's suggestion above for both timer and delay. They are in TypeScript and should be fully API-compatible with the originals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.