Add a test-only transform to catch infinite loops

[gaearon]

I took @amasad's code from https://repl.it/site/blog/infinite-loops and decided to try it out.
It's pretty neat.

This makes the tests throw when we're inside a loop that has more than M iterations and ran for more than N milliseconds. Currently, both needs to happen although we can of course adjust the conditions (just guarding iterations should be enough for us)

It's only enabled for tests.

Failure looks like this:

In the past, Jest would just hang.

Open questions:

• Is this useful? Do we want this at all? Does showing the loop callsite help?
• We need to make sure this always fails the test even if the error ends up being caught.
• If one test case fails with this we probably want to fail others immediately (if they're also stuck in a loop). Otherwise we have to wait for each individual one to fail even though they likely stress the same path. On the other hand only the first one is the actual problem. Need to think about how to present this. Maybe two different messages ("Potential infinite loop" vs "The test failed because another test has a potential infinite loop. Search for it in the output." or something.) Or include the bad stack in all messages. Not really. With our current iteration limits they fail pretty fast.
• How many iterations and/or time are we okay with? — Determined this by running tests. Set different limits for source files and test filess.
• License/attribution (I added @amasad to the authors at the top, pretty sure he won't mind it being MIT, but I'll make sure before merging).

Posting this for @acdlite to play with since he's spent the most time with infinite loops and probably knows what's helpful and what isn't.

[amasad]

Yes, it's MIT. I have a test suite for it. I can put it up in a repo if you think that's useful.

[gaearon]

If you could send a PR against my PR that would be great 😄
Alternatively just copy paste it here and I'll adapt it.

[acdlite]

Ahhhhhhh this is so great. +1000000

[acdlite]

Me, but non-ironically:

[gaearon]

Approved? :-)

[acdlite]

Sorry for the delay. Had to find the right GIF.

[amasad]

I see you removed the time heuristic. One thing to keep an eye out for is that this may generate false positives if you have loops with large iterations that are pretty fast -- which is not uncommon.

Alas couldn't find the time to send a PR. Here are the tests if you'd like to adapt them. (Some of our old tests are written in Mocha 🙈):

/* eslint-env node, mocha */
const transform = require('../');
const assert = require('assert');

const init =
  'var _loopStart = Date.now(),_loopIt = 0;' +
  'setTimeout(function () {_loopStart = Infinity;});';
const ifError =
  'if (++_loopIt > 5000 && Date.now() - _loopStart > 150) ' +
  'throw new RangeError("Potential infinite loop. ' +
  'You can disable this from settings.");';

describe('jsLoopBreaker', () => {
  it('transform breaks while loops', () => {
    const code = `
    while (true) {
      doIt();
    }
    `;

    const expected = `${init}
while (true) {${ifError}
  doIt();
}`;

    const { code: actual } = transform(code, { jsLoopBreaker: true });
    assert.equal(actual, expected);

    assertEval(actual);
  });

  it('transform breaks while loops with no body', () => {
    const code = `
    while (true) doIt();
    `;

    const expected = `${init}
while (true) {${ifError}doIt();}`;

    const { code: actual } = transform(code, { jsLoopBreaker: true });
    assert.equal(actual, expected);

    assertEval(actual);
  });

  it('transform inside functions on same line', () => {
    const code = `
    function x() {while (true)doIt()}
    `;

    const expected = `
function x() {${init}while (true) {${ifError}doIt();}}`;

    const { code: actual } = transform(code, { jsLoopBreaker: true });
    assert.equal(actual, expected);

    assertEval(actual + '\nx();');
  });

  it('transform breaks for loops', () => {
    const code = `
    for (var i = 0; i < 100; i++) {
      doIt();
    }
    `;

    const expected = `${init}
for (var i = 0; i < 100; i++) {${ifError}
  doIt();
}`;

    const { code: actual } = transform(code, { jsLoopBreaker: true });
    assert.equal(actual, expected);
  });

  it('transform breaks do loops', () => {
    const code = `
    do {
      doIt();
    } while (true);
    `;

    const expected = `${init}
do {${ifError}
  doIt();
} while (true);`;

    const { code: actual } = transform(code, { jsLoopBreaker: true });
    assert.equal(actual, expected);
  });
});

function assertEval(code) {
  /* eslint no-unused-vars: off */
  function doIt() {}
  /* eslint no-eval: off */
  const before = Date.now();
  try {
    eval(code);
  } catch (e) {
    assert.equal(e.name, 'RangeError');
    assert(e.message.match(/inf/));
    assert(Date.now() - before < 1000, `failed to break: ${code}`);
  }
}

[clemmy]

Awesome! I wonder if it's worthwhile catching infinite recursion as well. 🙂

[amasad]

worthwhile catching infinite recursion as well

For better or worse JavaScript catches that by blowing up the stack 😅
See https://repl.it/@amasad/RecursionRecursion

[gaearon]

One thing to keep an eye out for is that this may generate false positives if you have loops with large iterations that are pretty fast

Yeah. In our case this isn't expected to happen because the number of iterations should always roughly correspond to the number of components, and we only have a few stress tests that render over a thousand. So having them fail early might help highlight other unintentional long loops which is good.