Add error context if timers are infinite looped #4626

DeFuex · 2017-10-07T18:23:49Z

Summary
related to Issue #2893. If runAllTimers() is called and timers are caught in an infinite loop the error message should contain more information like pending timers, each showing a depending stacktrace.

facebook-github-bot · 2017-10-07T18:23:58Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot · 2017-10-07T18:31:34Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

SimenB · 2017-10-07T19:10:46Z

packages/jest-util/src/fake_timers.js

-import type {ProjectConfig} from 'types/Config';
-import type {Global} from 'types/Global';
-import type {ModuleMocker} from 'jest-mock';
+import type {ProjectConfig } from 'types/Config';


you should run yarn lint --fix to remove all of these whitespace changes

I also doubt you want the yarn.lock changes

hi, @SimenB , i get a lot of Unable to resolve path to module issues when yarn lint --fix ...and i also had to add "eslint-config-fbjs": "^2.0.0" to make yarn lint work. do i do something wrong here?

remove your changes to yarn.lock, and run yarn clean-all && yarn && yarn lint --fix. Make sure you are on yarn@1.1.0 as well.

That should fix it 🙂

hehe, i'll try, thanks! just switched from atom to vs code (there was some unexpected indent behaviour which i resolved now)

$ eslint . --cache --ext js,md Cannot read config file: /Users/timoobereder/Documents/Projects/react/jest/packages/eslint-config-fb-strict/index.js Error: Cannot find module 'eslint-config-fbjs' Referenced from: /Users/timoobereder/Documents/Projects/react/jest/.eslintrc.js Error: Cannot read config file: /Users/timoobereder/Documents/Projects/react/jest/packages/eslint-config-fb-strict/index.js Error: Cannot find module 'eslint-config-fbjs' Referenced from: /Users/timoobereder/Documents/Projects/react/jest/.eslintrc.js at Function.Module._resolveFilename (module.js:470:15) at Function.Module._load (module.js:418:25) at Module.require (module.js:498:17) at require (internal/module.js:20:19) at Object.<anonymous> (/Users/timoobereder/Documents/Projects/react/jest/packages/eslint-config-fb-strict/index.js:10:20) at Module._compile (module.js:571:32) at Object.Module._extensions..js (module.js:580:10) at Module.load (module.js:488:32) at tryModuleLoad (module.js:447:12) at Function.Module._load (module.js:439:3) /Users/timoobereder/.yarn/bin/yarn: line 31: 92774 Segmentation fault: 11 node "$basedir/yarn.js" "$@"```

Segfault, interesting...

i'm sure i have the latest yarn version through homebrew

@DeFuex I pushed to your branch. Still a couple of lint errors, but those are actual issues, not style related.

So just pull, and do yarn and you should be good

SimenB · 2017-10-07T19:12:40Z

I realize it's WIP, but some tests would be great 😃

SimenB · 2017-10-08T09:43:07Z

packages/jest-util/src/fake_timers.js

-          "Assuming we've hit an infinite recursion and bailing out...",
+          "Assuming we've hit an infinite recursion and bailing out." +
+          'The following timers are still pending:' +
+          Object.keys(timers)


timers is not defined, and the syntax below is weird.

i know, im not sure if im doing this the right way, but i want to get the timers and then want to get only pending timers and output the stacktrace information for each specifically with map const timers = Object.assign({}, this._timers);

SimenB · 2017-10-08T09:43:15Z

packages/jest-util/src/fake_timers.js

+          Object.keys(timers)
+            .sort((left, right) => timers[left].expiry - timers[right].expiry)
+            .map(function(key, index) {
+              '\n\nsetTimeout - pending timer time ' +


missing return here?

i don't think a return is missing since Object,keys(object).map should work like an iterate.

if you just want to just iterate, you should use .forEach.

But if you don't return here, the string is never shown to the user as it's lost inside the map. So you want map and the return.

Something like this (which still fails, but it's a start):

diff --git i/packages/jest-util/src/fake_timers.js w/packages/jest-util/src/fake_timers.js index a1870352..1e0b09e7 100644 --- i/packages/jest-util/src/fake_timers.js +++ w/packages/jest-util/src/fake_timers.js @@ -234,16 +234,16 @@ export default class FakeTimers { Object.keys(timers) .sort((left, right) => timers[left].expiry - timers[right].expiry) .map(function(key, index) { - '\n\nsetTimeout - pending timer time ' + + return '\n\nsetTimeout - pending timer time ' + timers[key].expiry - - this._now; + this._now + '\n' + formatStackTrace(new Error().stack, this._global.setTimeout, { noStackTrace: true, }) + '\n\nsetImmediate - pending timer time ' + timers[key].expiry - - this._now; + this._now + '\n' + formatStackTrace(new Error().stack, this._global.setImmediate, { noStackTrace: true,

ah yeah, does make sense. i thought using object.keys would be nicer to iterate through an object since i also can use sort to get the pending timers that are left.

DeFuex · 2017-10-08T09:59:45Z

still getting a segmentation fault 😢

SimenB · 2017-10-08T10:02:54Z

What version of node are you running?

Also, have you checked out jest inside of react, or is react just the name of your directory?

This is the path in your stack strace: /Users/timoobereder/Documents/Projects/react/jest/. If jest is inside of react, that can explain some of the issues, make sure to check out jest in its own directory

DeFuex · 2017-10-08T10:06:26Z

i'm running node version v7.4.0 and react is just the name of my directory

SimenB · 2017-10-08T10:08:25Z

Node 7 is not supported, you should upgrade to node 8.

That said, it shouldn't be an issue... Not sure I can help with this 😞

DeFuex · 2017-10-08T10:52:04Z

linked the right node version (v8.6.0) and latest npm. getting following error now after yarn clean-all && yarn && yarn lint --fix -> Cannot read config file: /Users/timoobereder/Documents/Projects/react/jest/packages/eslint-config-fb-strict/index.js Error: Cannot find module 'eslint-config-fbjs' Referenced from: /Users/timoobereder/Documents/Projects/react/jest/.eslintrc.js Error: Cannot read config file: /Users/timoobereder/Documents/Projects/react/jest/packages/eslint-config-fb-strict/index.js Error: Cannot find module 'eslint-config-fbjs' Referenced from: /Users/timoobereder/Documents/Projects/react/jest/.eslintrc.js at Function.Module._resolveFilename (module.js:527:15) at Function.Module._load (module.js:476:23) at Module.require (module.js:568:17) at require (internal/module.js:11:18) at Object.<anonymous> (/Users/timoobereder/Documents/Projects/react/jest/packages/eslint-config-fb-strict/index.js:10:20) at Module._compile (module.js:624:30) at Object.Module._extensions..js (module.js:635:10) at Module.load (module.js:545:32) at tryModuleLoad (module.js:508:12) at Function.Module._load (module.js:500:3) error Command failed with exit code 1.

SimenB · 2017-10-08T11:02:07Z

To confirm, you are on yarn 1.1.0 as well?

DeFuex · 2017-10-08T11:15:50Z

i feel a little stupid, as it seems i got node, npm and yarn messed up with a normal global install and the homebrew variant. at least now i know why it wasn't working. got it now and gonna go on working on the pr, thanks @SimenB 😄

DeFuex · 2017-10-08T16:33:57Z

Current state: needs adjustment in 1 test case and proper testing

SimenB · 2017-10-08T20:54:59Z

packages/jest-util/src/fake_timers.js

-          "Assuming we've hit an infinite recursion and bailing out...",
+          "Assuming we've hit an infinite recursion and bailing out." +
+          'The following timers are still pending:' +
+          String(


this is unnecessary, you should do a reduce on the array returned by map instead

Or just .join

thx, i will do that today

SimenB · 2017-10-08T20:55:44Z

packages/jest-util/src/fake_timers.js

@@ -225,11 +225,37 @@ export default class FakeTimers {
    }

    if (i === this._maxLoops) {
+      const timers = Object.assign({}, this._timers);


you don't need to do this if you only do Object.keys on it (just don't mutate the object)

Change define consts to avoid undefined

cpojer · 2018-01-06T10:45:10Z

It seems like this PR alters snapshots on Travis?

SimenB · 2018-01-06T11:32:04Z

This will be void if and when we merge #5171 as well (at which time this context info would be great to have in Lolex, I'd think. /cc @benjamingr).

cpojer · 2018-01-06T13:44:20Z

Got it, will close this PR in favor of the lolex replacement then.

github-actions · 2021-05-12T23:07:40Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

Add context functionality for infinite looped timers

c59bd2d

facebook-github-bot added the CLA Signed ✔️ label Oct 7, 2017

DeFuex added 2 commits October 7, 2017 20:36

Fix small issue

a829147

Fix error in map

00189e8

SimenB reviewed Oct 7, 2017

View reviewed changes

SimenB added 2 commits October 8, 2017 11:39

Revert yarn.lock changes

0ece0e9

Fix styling errors

0c4b2c8

SimenB reviewed Oct 8, 2017

View reviewed changes

DeFuex added 2 commits October 8, 2017 17:53

Add timers and return of iteration

ec17d52

Add explicit casts for flow type check

80cba31

SimenB reviewed Oct 8, 2017

View reviewed changes

DeFuex added 2 commits October 12, 2017 19:04

Change object iteration

2754435

Change define consts to avoid undefined

Merge branch 'master' into feature/context-on-infinite-timer-recursion

eae4426

DeFuex changed the title ~~[WIP] Add error context if timers are infinite looped~~ Add error context if timers are infinite looped Oct 12, 2017

cpojer closed this Jan 6, 2018

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add error context if timers are infinite looped #4626

Add error context if timers are infinite looped #4626

DeFuex commented Oct 7, 2017

facebook-github-bot commented Oct 7, 2017

facebook-github-bot commented Oct 7, 2017

SimenB Oct 7, 2017

SimenB Oct 7, 2017

DeFuex Oct 8, 2017

SimenB Oct 8, 2017

DeFuex Oct 8, 2017

DeFuex Oct 8, 2017

SimenB Oct 8, 2017

DeFuex Oct 8, 2017

SimenB Oct 8, 2017

SimenB commented Oct 7, 2017

SimenB Oct 8, 2017 •

edited

DeFuex Oct 8, 2017

SimenB Oct 8, 2017

DeFuex Oct 8, 2017

SimenB Oct 8, 2017

DeFuex Oct 8, 2017

DeFuex commented Oct 8, 2017

SimenB commented Oct 8, 2017 •

edited

DeFuex commented Oct 8, 2017

SimenB commented Oct 8, 2017

DeFuex commented Oct 8, 2017

SimenB commented Oct 8, 2017

DeFuex commented Oct 8, 2017

DeFuex commented Oct 8, 2017

SimenB Oct 8, 2017

SimenB Oct 12, 2017

DeFuex Oct 12, 2017

SimenB Oct 8, 2017

cpojer commented Jan 6, 2018

SimenB commented Jan 6, 2018

cpojer commented Jan 6, 2018

github-actions bot commented May 12, 2021

Add error context if timers are infinite looped #4626

Add error context if timers are infinite looped #4626

Conversation

DeFuex commented Oct 7, 2017

facebook-github-bot commented Oct 7, 2017

facebook-github-bot commented Oct 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB commented Oct 7, 2017

SimenB Oct 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DeFuex commented Oct 8, 2017

SimenB commented Oct 8, 2017 • edited

DeFuex commented Oct 8, 2017

SimenB commented Oct 8, 2017

DeFuex commented Oct 8, 2017

SimenB commented Oct 8, 2017

DeFuex commented Oct 8, 2017

DeFuex commented Oct 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer commented Jan 6, 2018

SimenB commented Jan 6, 2018

cpojer commented Jan 6, 2018

github-actions bot commented May 12, 2021

SimenB Oct 8, 2017 •

edited

SimenB commented Oct 8, 2017 •

edited