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

There is no timeout scheduled with the given id "386". #433

Closed
LavanyaBurlagadda opened this issue Aug 1, 2023 · 10 comments
Closed

There is no timeout scheduled with the given id "386". #433

LavanyaBurlagadda opened this issue Aug 1, 2023 · 10 comments

Comments

@LavanyaBurlagadda
Copy link

We are getting this issue when we are clearing the timeouts in the component unmounting stage.

Let's say that even if the timeout is executed if we try to clear the timeout it shouldn't throw an error.

We are getting sentry issues reported for this issue.

image

@chrisguttandin
Copy link
Owner

There is a subtle difference in how worker-timers handles missing missing ids.

https://github.com/chrisguttandin/worker-timers#error-handling

Is it possible that you call clearTimeout() twice for the same id?

In any case it would be helpful to see the code which registers and clears the timeout.

@theogravity
Copy link

theogravity commented Aug 1, 2023

I seem to be having this problem too.

Here's some sample react code:

import { clearInterval, setInterval } from 'worker-timers';
import React, { useCallback, useEffect, useRef } from 'react';

export const Component = () => {
  const refreshExpiryTimer = useRef<number | null>(null);

  const clearRefreshExpiryTimer = useCallback(() => {
    if (refreshExpiryTimer.current) {
      try {
        // NOTE: this is not window.clearTimeout, this is from the worker-timers package
        // worker-timers version can throw if the id is invalid
        clearInterval(refreshExpiryTimer.current);
      } catch (e) {
        // ignore
      }
    }
  }, [refreshExpiryTimer]);

 useEffect(() => {
    clearRefreshExpiryTimer();
    refreshExpiryTimer.current = setInterval(() => {
        //...
    }, 10000);

    return () => {
      clearRefreshExpiryTimer();
    };
  }, [clearRefreshExpiryTimer]);

  return null
}

It'd be really nice to just have an option to ignore any errors around this or add a function that allows us to check if the id exists or not

@chrisguttandin
Copy link
Owner

I'm not 100% sure how this could be done best in React but I think refreshExpiryTimer needs to be reset to null after calling clearInterval(refreshExpiryTimer.current);. Otherwise it will probably try to call clearInterval() with the same number again.

By the way, it's theoretically possible that the number returned by setInterval() is 0. Therefore (refreshExpiryTimer.current) might resolve to false even though its a number.

@theogravity
Copy link

theogravity commented Aug 2, 2023

I just had that thought now and was going to mention that then saw your comment.

Nulling it out should resolve it. Thanks!

@theogravity
Copy link

theogravity commented Aug 2, 2023

When I tried my code in a skeleton sandbox, it's fine, but when I use it in my own app, I get the issue.

I noticed that when I log out the variable that stores the timer, it starts with a low count then increments for each setup and teardown in the component. This looks like proper behavior.

https://codesandbox.io/s/worker-timer-bug-r7qyyx

In the bug I'm seeing, the error shows ids often in the three or four digits. eg There is no timeout scheduled with the given id "2164".

I do use worker-timers in other scripts of our app, but I don't think we're creating timers that would get to the thousands assuming id increments for each timer created.

I attempted to see if using it in two different components had some impact in the sandbox above, but again, I don't see anything wrong there.

Edit: Apparently we do use this package in lots of places, but I don't think into the four digits worth of setInterval / setTimeout cycles

@theogravity
Copy link

theogravity commented Aug 2, 2023

How is the timerId generated? Looking at https://github.com/chrisguttandin/worker-timers-worker, it doesn't seem obvious to me.

I also noticed the last update on https://github.com/chrisguttandin/worker-timers/blob/master/src/worker/worker.ts was 2 years ago, not sure if this is actively used or not, but was wondering if it could be due to stale code.

Edit:

I see the timerId gets assigned here

https://github.com/chrisguttandin/worker-timers-broker/blob/master/src/module.ts#L113C14-L113C14

@theogravity
Copy link

lol, I figured it out, I forgot to null out in another file where I've also started to use the package

@chrisguttandin
Copy link
Owner

@theogravity Great, I'm glad you found the issue. As a back story, the ids are generated using fast-unique-numbers. I created that utility because I needed this functionality in various places. As long as you don't use more than 1073741824 timers it will only increase the number one. After that it will try to employ more intelligent techniques. :-)

@LavanyaBurlagadda Is your problem of similar nature?

@ahoyahoy
Copy link

Hello,
I am facing the same problem.
The problem is with strict react mode, where render is called twice. (Because I don't have this problem when I switch responseStrictMode to false in next.config.js)
I also tried to catch these errors with try { clearTimeout(..) } catch() {} but it doesn't work.

@chrisguttandin
Copy link
Owner

I finally removed this behavior in v8. worker-timers should no longer throw an error when calling clearInterval() or clearTimeout() with an unknown id.

Please let me know if the problem still persists.

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

No branches or pull requests

4 participants