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

[BUG] setTimeout fires incorrectly when using chrome debug #4470

Closed
Thomas101 opened this issue Dec 1, 2015 · 47 comments
Closed

[BUG] setTimeout fires incorrectly when using chrome debug #4470

Thomas101 opened this issue Dec 1, 2015 · 47 comments
Labels
JavaScript Resolution: Locked This issue was locked by the bot.

Comments

@Thomas101
Copy link
Contributor

Hey,

I've found an issue that means when you're running on an iOS device and debugging in chrome setTimeouts can fire in the wrong order. If you place the following code block into a fresh app and run it you should get the error...

  componentDidMount:function() {
    this.runTest()
  },

  test:function(cb) {
    var aFired = false
    var bFired = false
    var aExpected = new Date().getTime()+130
    var bExpected = new Date().getTime()+500
    console.log('Schedule A to fire @' + aExpected)
    console.log('Schedule B to fire @' + bExpected)

    var a = setTimeout(() => {
      var now = new Date().getTime()
      console.log('A fired', now, now - aExpected)
      aFired = true
      if (bFired) { console.log('ERROR: b fired early'); return;
      }
      if (aFired && bFired) { cb() }
    }, 130)
    var b = setTimeout(() => {
      var now = new Date().getTime()
      console.log('B fired', now, now - bExpected)
      bFired = true
      if (!aFired) { console.log('ERROR: A not fired'); return;
      }
      if (aFired && bFired) { cb() }
    }, 500)
  },
  runTest:function() {
    this.test(() => {
      setTimeout(() => {
        this.runTest()
      }, 500)
    })
  },

On the console you'll get something like...

index.ios.js:25 Schedule A to fire @1448977966848
index.ios.js:26 Schedule B to fire @1448977967218
index.ios.js:30 A fired 1448977966754 -94
index.ios.js:38 B fired 1448977966755 -463
index.ios.js:25 Schedule A to fire @1448977966902
index.ios.js:26 Schedule B to fire @1448977967272
index.ios.js:30 A fired 1448977966806 -96
index.ios.js:38 B fired 1448977966806 -466
index.ios.js:25 Schedule A to fire @1448977966951
index.ios.js:26 Schedule B to fire @1448977967321
index.ios.js:30 A fired 1448977966835 -116
index.ios.js:38 B fired 1448977966835 -486


index.ios.js:25 Schedule A to fire @1448977966983
index.ios.js:26 Schedule B to fire @1448977967353
index.ios.js:38 B fired 1448977966871 -482
index.ios.js:40 ERROR: A not fired
index.ios.js:30 A fired 1448977966871 -112
index.ios.js:32 ERROR: b fired early

... with the first 3 firing correctly and the last one incorrectly. I believe this bug is also what is causing issue #1693 as one of the setTimeout functions sometimes fires early in touchableHandleResponderGrant

@facebook-github-bot
Copy link
Contributor

Hey Thomas101, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If this is a feature request or a bug that you would like to be fixed by the team, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • If you don't know how to do something or not sure whether some behavior is expected or a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • We welcome clear issues and PRs that are ready for in-depth discussion; thank you for your contributions!

@christopherdro
Copy link
Contributor

@Thomas101 Try taking a look at https://facebook.github.io/react-native/docs/timers.html#timermixin at the last paragraph:
We strongly discourage using the global setTimeout(...) and recommend instead that you use this.setTimeout(...)

@Thomas101
Copy link
Contributor Author

Thanks @christopherdro, we've been using that wherever possible but some of the built in react functionality uses plain old settimeout which is where the bug raised its head initially

@ide
Copy link
Contributor

ide commented Dec 1, 2015

@Thomas101 Thanks for your detailed investigation and write-up. RN should implement timers correctly. In fact if I'm reading your code and logs correctly, it looks like the timeout arg is ignored as events A and B run at almost the same time. cc @sahrens

@Thomas101
Copy link
Contributor Author

Those were the logs that came out, you should get similar results if you run it yourself. This is purely speculation as I haven't looked at the implementation, but as it only happenens when running the chrome debugger and using the device, did wonder if it could be a time drift issue between my Mac and ipad?

@satya164
Copy link
Contributor

satya164 commented Dec 3, 2015

@christopherdro Also not possible to use it when using the ES6 class syntax. Need to manually cleanup ofc.

@sahrens
Copy link
Contributor

sahrens commented Dec 5, 2015

Not sure who owns timers these days - maybe @tadeuzagallo or @nicklockwood ? Also cc @dmmiller because we want to be consistent on iOS and android.

@aleclarson
Copy link
Contributor

I've experienced this while "Debug in Chrome" is active. setTimeout fires very early.

Has it been confirmed that TimerMixin avoids this issue?

I thought TimerMixin only helps with cleanup in componentWillUnmount?

@Thomas101
Copy link
Contributor Author

Hi @aleclarson using the TimerMixin still has the same problem as this uses setTimeout behind the scenes.

@aleclarson
Copy link
Contributor

Who works on the debugger-proxy packager endpoint? Is there a hack related to setTimeout that involves accounting for socket latency? I can't imagine how else setTimeout could be acting so strangely.

@PaulBGD
Copy link

PaulBGD commented Jan 9, 2016

Go vote for this issue so that the developers can see that it's important: https://productpains.com/post/react-native/settimeout-fires-incorrectly-when-using-chrome-debug/

@sahrens
Copy link
Contributor

sahrens commented Jan 9, 2016

I don't know much about our chrome integration or why it would cause a difference in timer behavior - @frantic or @tadeuzagallo?

@aleclarson
Copy link
Contributor

Would it be worth implementing a native shim for setTimeout and clearTimeout?

@satya164
Copy link
Contributor

Any updates on this?

@satya164
Copy link
Contributor

So, I did a some small test on this,

var start = performance.now(); console.log(performance.now(), start); // 751855.6900000001 492872.2
var start = performance.now(); setTimeout(() => console.log(performance.now() - start), 1); // 345195.46

This is super weird. The value of performance.now() is vastly different than when it's assigned to a variable. SImilar results for Date.now().

No wonder setTimeout executes instantly even if you give a timeout of 10 seconds.

cc @vjeux @dmmiller @nicklockwood

@vjeux
Copy link
Contributor

vjeux commented Jan 25, 2016

[meta-discussion] At this point, there is a good repro case (thanks @Thomas101, that's really awesome) and no clear owner (as everyone is cc-ing other people). The way forward is to get someone to dig into the internals and figure out what's going on. It doesn't have to be a Facebook employee, thankfully the entire part of this stack is open source and anyone can try and figure out what's going on.

I suggest that if you are affected by this issue and have some time to spare, try to dig in and figure out what's going on.

@marcosinigaglia
Copy link

I think the release 0.25-rc solve the problem.

@Abhay-Joshi-Git
Copy link

Abhay-Joshi-Git commented Jun 29, 2016

I am using 0.28.0. It looks like logging in chrome with this.setInterval is not working still. I faced the issue for Android.

@martinarroyo
Copy link

Same as @Abhay-Joshi-Git here as well (using plain setTimeout in a function outside a React component). For a timeout of 4000 ms it fires immediately. (RN 0.28.0 running on Android device). When Chrome Debugging is disabled it works as expected.

@nihgwu
Copy link
Contributor

nihgwu commented Jun 29, 2016

I'm using RN0.29RC0 and facing the same problem, onLongPress fires immediately while Debug JS Remotely enabled

@marcshilling
Copy link

Happening to me with RN 28....setTimeout are firing immediately.

@greatbsky
Copy link

RN0.29 fires immediately while Debug JS Remotely enabled

@matt-oakes
Copy link
Contributor

Will this cease to be a problem once this issue is implemented? #8641

@nambrot
Copy link

nambrot commented Jul 30, 2016

fwiw, on Android, when in Debug with Chrome, setTimeout does not fire at all

@lacker
Copy link
Contributor

lacker commented Oct 28, 2016

It looks like this is fixed as mentioned in #9436 . It kind of seems like there's a bunch of different issues being discussed here so if similar things continue to occur perhaps it would be easier to open a new issue. Anyway I am going to close this one.

@lacker lacker closed this as completed Oct 28, 2016
@tomasfrancisco
Copy link

I'm not certain that the issue that is reported on this thread is resolved.
I'm getting the same behavior as described by many of you.
One way to get things done and testable was to set the timers to a time less than 2 seconds, but when the app gets bigger it will be unbearable to test it.

RN0.42.0

@jesseditson
Copy link

Also experiencing this, not sure why this bug is closed. Shall we file a new one? Seems like that's what's being suggested by @lacker, is that just to keep noise down? Is there any reason to believe a prior patch would have fixed this? (It's unclear to me if this was ever fixed.)

@lucasbento
Copy link
Contributor

The same thing happened here with RN 0.44.0 running on Android.

@Odinvt
Copy link

Odinvt commented May 20, 2017

For the people who are still facing this issue ! which is the case for us on RN 0.44.0 on Android :

We found indeed that the problem was related to the device time not being exactly as the time on the chrome remote debugger machine. This is due to the React Native javascript setTimeout function calling the native Timing.createTimer method with a Date.now() parameter which is not the same as the native side's time when in remote js debug mode (where the javascript execution environment is the chrome browser or whatever debugger you're using) as you can see in JSTimers.js#L93.

So, the solution for us was 1 of the following :

  • Either set the time of the device to be exactly as the remote debugging machine's time using ADB
  • Or compensate for the time difference between the native side and the remote js side

We chose the second solution as the first one wasn't suited for our needs so we created a package called React Native Timer Native (I know, i'm not so good when it comes to naming things ^^') which is a fork of React Native Timer.

What it does --as a stupid hack-- is simply call a native method (when in dev mode) with the current javascript timestamp, then the native method gets its current timestamp using System.currentTimeMillis(), calculates the difference and returns it back to javascript, then the js package just adds that to whatever duration you passed to timer.setTimeout .

The device time must be in the future compared to the remote debugging machine time otherwise it doesn't work properly

It does get the timeouts executed with a decent precision (still need to substract the time that the bridge takes to call the native method so any inputs are welcome ;) ).

@hansfpc
Copy link

hansfpc commented Jul 11, 2017

Same issue here (Using RN 0.45). I have to disable 'Debug JS remotely' :(

@MrToph
Copy link

MrToph commented Jul 22, 2017

Still an issue in 0.46.3, can we reopen this?

@caltv
Copy link

caltv commented Jul 27, 2017

I'm also seeing this with the iOS simulator, and my machine has the same time as the Simulator...disabling 'Debug JS remotely' fixes it

@nicotroia
Copy link

For me, the setTimeout will wait until a re-render happens. So you have to touch the screen or scroll the page in order to fire the handler (assuming the time has passed). Disabling remote debugging makes it work as normal

@alpfilho
Copy link

alpfilho commented Aug 9, 2017

still an issue in 0.46.4

@dave-irvine
Copy link

Still an issue on 0.47.x as far as I can tell

spittet added a commit to spittet/futurospective-mobile that referenced this issue Aug 28, 2017
…y works

when using a device (tested with iOS) as there's an apparent timeout issue when
using Chrome Debug (see facebook/react-native#4470).
I haven't yet tested it on Android though.
@emusgrave
Copy link

@Odinvt Did you consider integrating your library into a PR for the core RN library? I'm curious if the team would accept it as a solution for this issue.

@Odinvt
Copy link

Odinvt commented Sep 5, 2017

@emusgrave I thought about it and I would love to, but I don't think that the PR will be merged because first of all, it's an ugly hack and second of all, it's not a modification of react native's source code, it's only a modification of another package ( React Native Timer ) which would force devs to use the package's overridden methods instead of the classic JS setTimeout and setInterval methods...

The source code is there, we've been using it for 4 months in our dev env for our projects and it's proven its worth. I encourage anyone who has the time to make those changes in RN's source code and submit a PR to do so, I think that the idea is there but the code still needs some refactoring and tests should be written. I hope you're all having fun creating stuff with RN as I know I do <3

@kesha-antonov
Copy link
Contributor

RN 0.50 Same issue

@ahmb84
Copy link

ahmb84 commented Nov 13, 2017

Same issue from RN 0.50.3

@loveky
Copy link

loveky commented Dec 13, 2017

any update on this? just spent about 1 hour and see this issue... 😞

@jacklovepdf
Copy link

close the debugger model

@Vishal-vash
Copy link

I had similar issue using onLongPress with debugger open and it fires the event as onPress , So onLongPress also don't work accurately in debugger mode similar to setTimeout/setInterval

@dhirendrarathod2000
Copy link

RN 0.53
Not working

@fatfatson
Copy link

sync your emulator and host' time by:

adb shell su root date $(get-date -format MMddHHmmyyyy.ss)

@Sharcoux
Copy link

get-date -> unknown command.

I had better luck with:

adb shell su root "date `date +%m%d%H%M%Y.%S`"

@facebook facebook locked as resolved and limited conversation to collaborators Jul 20, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
JavaScript Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests