Handle async errors (#4016)

* wip: start handling async errors

Ref #2059

* Implement basic cancelation

* Improve cancelation

* lint and flow happy, more tests

* snapshot updates

* happy tests

* add tests for before hooks

* log errors if they escape us

* use p-cancelable

* improve based on CR

integration_tests/__tests__/jasmine_async.test.js

@@ -19,14 +19,16 @@ describe('async jasmine', () => {
     ]);
     const json = result.json;
 
-    expect(json.numTotalTests).toBe(2);
+    expect(json.numTotalTests).toBe(4);
     expect(json.numPassedTests).toBe(1);
-    expect(json.numFailedTests).toBe(1);
+    expect(json.numFailedTests).toBe(3);
     expect(json.numPendingTests).toBe(0);
 
     const {message} = json.testResults[0];
     expect(message).toMatch('with failing timeout');
     expect(message).toMatch('Async callback was not invoked within timeout');
+    expect(message).toMatch('done - with error thrown');
+    expect(message).toMatch('done - with error called back');
   });
 
   it('works with beforeEach', () => {
@@ -35,11 +37,14 @@ describe('async jasmine', () => {
     ]);
     const json = result.json;
 
-    expect(json.numTotalTests).toBe(1);
+    expect(json.numTotalTests).toBe(3);
     expect(json.numPassedTests).toBe(1);
-    expect(json.numFailedTests).toBe(0);
+    expect(json.numFailedTests).toBe(2);
     expect(json.numPendingTests).toBe(0);
-    expect(json.testResults[0].message).toBe('');
+
+    const {message} = json.testResults[0];
+    expect(message).toMatch('done - with error thrown');
+    expect(message).toMatch('done - with error called back');
   });
 
   it('works with afterAll', () => {
@@ -104,12 +109,17 @@ describe('async jasmine', () => {
     const json = result.json;
     const message = json.testResults[0].message;
 
-    expect(json.numTotalTests).toBe(9);
-    expect(json.numPassedTests).toBe(5);
-    expect(json.numFailedTests).toBe(4);
+    expect(json.numTotalTests).toBe(16);
+    expect(json.numPassedTests).toBe(6);
+    expect(json.numFailedTests).toBe(9);
 
     expect(message).toMatch('fails if promise is rejected');
     expect(message).toMatch('works with done.fail');
+    expect(message).toMatch('works with done(error)');
+    expect(message).toMatch('fails if failed expectation with done');
+    expect(message).toMatch('fails if failed expectation with done - async');
+    expect(message).toMatch('fails with thrown error with done - sync');
+    expect(message).toMatch('fails with thrown error with done - async');
     expect(message).toMatch('fails a sync test');
     expect(message).toMatch('fails if a custom timeout is exceeded');
   });

integration_tests/jasmine_async/__tests__/promise_before_all.test.js

@@ -34,4 +34,19 @@ describe('promise beforeAll', () => {
 
     it('fails', () => {});
   });
+
+  describe('done - with error thrown', () => {
+    beforeAll(done => {
+      throw new Error('fail');
+      done(); // eslint-disable-line
+    });
+    it('fails', () => {});
+  });
+
+  describe('done - with error called back', () => {
+    beforeAll(done => {
+      done(new Error('fail'));
+    });
+    it('fails', () => {});
+  });
 });

integration_tests/jasmine_async/__tests__/promise_before_each.test.js

@@ -21,4 +21,20 @@ describe('promise beforeEach', () => {
   it('runs tests after beforeEach asynchronously completes', () => {
     expect(this.flag).toBe(1);
   });
+
+  // failing tests
+  describe('done - with error thrown', () => {
+    beforeEach(done => {
+      throw new Error('fail');
+      done(); // eslint-disable-line
+    });
+    it('fails', () => {});
+  });
+
+  describe('done - with error called back', () => {
+    beforeEach(done => {
+      done(new Error('fail'));
+    });
+    it('fails', () => {});
+  });
 });

integration_tests/jasmine_async/__tests__/promise_it.test.js

@@ -26,6 +26,10 @@ describe('promise it', () => {
     done();
   });
 
+  it('works with async done', done => {
+    setTimeout(done, 1);
+  });
+
   it('is bound to context object', () => {
     return new Promise(resolve => {
       if (this.someContextValue !== 'value') {
@@ -46,6 +50,42 @@ describe('promise it', () => {
     done.fail(new Error('done.fail was called'));
   });
 
+  it('works with done(error)', done => {
+    done(new Error('done was called with error'));
+  });
+
+  it('fails if failed expectation with done', done => {
+    expect(true).toEqual(false);
+    done();
+  });
+
+  it('fails if failed expectation with done - async', done => {
+    setTimeout(() => {
+      expect(true).toEqual(false);
+      done();
+    }, 1);
+  });
+
+  it('fails with thrown error with done - sync', done => {
+    throw new Error('sync fail');
+    done(); // eslint-disable-line
+  });
+
+  it('fails with thrown error with done - async', done => {
+    setTimeout(() => {
+      throw new Error('async fail');
+      done(); // eslint-disable-line
+    }, 1);
+  });
+
+  // I wish it was possible to catch this but I do not see a way.
+  // Currently both jest and mocha will pass this test.
+  it.skip('fails with thrown error - async', () => {
+    setTimeout(() => {
+      throw new Error('async fail - no done');
+    }, 1);
+  });
+
   it('fails a sync test', () => {
     expect('sync').toBe('failed');
   });

packages/jest-jasmine2/package.json

@@ -14,7 +14,8 @@
     "jest-diff": "^20.0.3",
     "jest-matcher-utils": "^20.0.3",
     "jest-message-util": "^20.0.3",
-    "jest-snapshot": "^20.0.3"
+    "jest-snapshot": "^20.0.3",
+    "p-cancelable": "^0.3.0"
   },
   "devDependencies": {
     "jest-runtime": "^20.0.4"

packages/jest-jasmine2/src/__tests__/queue_runner.test.js

@@ -130,4 +130,30 @@ describe('queueRunner', () => {
 
     expect(options.fail).toHaveBeenCalledWith('miserably', 'failed');
   });
+
+  it('calls `fail` when done(error) is invoked', async () => {
+    const error = new Error('I am an error');
+    const fail = jest.fn();
+    const fnOne = jest.fn(next => next(error));
+    const fnTwo = jest.fn(next => next());
+    const options = {
+      clearTimeout,
+      fail,
+      onException: () => {},
+      queueableFns: [
+        {
+          fn: fnOne,
+        },
+        {
+          fn: fnTwo,
+        },
+      ],
+      setTimeout,
+    };
+    await queueRunner(options);
+    expect(fnOne).toHaveBeenCalled();
+    expect(fail).toHaveBeenCalledWith(error);
+    // Even if `fail` is called, the queue keeps running.
+    expect(fnTwo).toHaveBeenCalled();
+  });
 });

packages/jest-jasmine2/src/jasmine/Env.js

@@ -161,7 +161,7 @@ module.exports = function(j$) {
       return seed;
     };
 
-    async function queueRunnerFactory(options) {
+    function queueRunnerFactory(options) {
       options.clearTimeout = realClearTimeout;
       options.fail = self.fail;
       options.setTimeout = realSetTimeout;
@@ -187,9 +187,31 @@ module.exports = function(j$) {
         }
       }
 
-      reporter.jasmineStarted({
-        totalSpecsDefined,
-      });
+      const uncaught = err => {
+        if (currentSpec) {
+          currentSpec.onException(err);
+          currentSpec.cancel();
+        } else {
+          console.error('Unhandled error');
+          console.error(err.stack);
+        }
+      };
+
+      // Need to ensure we are the only ones handling these exceptions.
+      const oldListenersException = process
+        .listeners('uncaughtException')
+        .slice();
+      const oldListenersRejection = process
+        .listeners('unhandledRejection')
+        .slice();
+
+      process.removeAllListeners('uncaughtException');
+      process.removeAllListeners('unhandledRejection');
+
+      process.on('uncaughtException', uncaught);
+      process.on('unhandledRejection', uncaught);
+
+      reporter.jasmineStarted({totalSpecsDefined});
 
       currentlyExecutingSuites.push(topSuite);
 
@@ -215,6 +237,18 @@ module.exports = function(j$) {
       reporter.jasmineDone({
         failedExpectations: topSuite.result.failedExpectations,
       });
+
+      process.removeListener('uncaughtException', uncaught);
+      process.removeListener('unhandledRejection', uncaught);
+
+      // restore previous exception handlers
+      oldListenersException.forEach(listener => {
+        process.on('uncaughtException', listener);
+      });
+
+      oldListenersRejection.forEach(listener => {
+        process.on('unhandledRejection', listener);
+      });
     };
 
     this.addReporter = function(reporterToAdd) {

packages/jest-jasmine2/src/jasmine/Spec.js

@@ -96,13 +96,15 @@ Spec.prototype.execute = function(onComplete, enabled) {
   const fns = this.beforeAndAfterFns();
   const allFns = fns.befores.concat(this.queueableFn).concat(fns.afters);
 
-  this.queueRunnerFactory({
+  this.currentRun = this.queueRunnerFactory({
     queueableFns: allFns,
     onException() {
       self.onException.apply(self, arguments);
     },
     userContext: this.userContext(),
-  }).then(() => complete(true));
+  });
+
+  this.currentRun.then(() => complete(true));
 
   function complete(enabledAgain) {
     self.result.status = self.status(enabledAgain);
@@ -114,6 +116,12 @@ Spec.prototype.execute = function(onComplete, enabled) {
   }
 };
 
+Spec.prototype.cancel = function cancel() {
+  if (this.currentRun) {
+    this.currentRun.cancel();
+  }
+};
+
 Spec.prototype.onException = function onException(error) {
   if (Spec.isPendingSpecException(error)) {
     this.pend(extractCustomPendingMessage(error));

packages/jest-jasmine2/src/jasmine_async.js

@@ -38,7 +38,7 @@ function promisifyLifeCycleFunction(originalFn, env) {
       const returnValue = fn.call({});
 
       if (isPromise(returnValue)) {
-        returnValue.then(done, done.fail);
+        returnValue.then(done.bind(null, null), done.fail);
       } else {
         done();
       }
@@ -68,7 +68,7 @@ function promisifyIt(originalFn, env) {
       const returnValue = fn.call({});
 
       if (isPromise(returnValue)) {
-        returnValue.then(done, done.fail);
+        returnValue.then(done.bind(null, null), done.fail);
       } else if (returnValue === undefined) {
         done();
       } else {

packages/jest-jasmine2/src/queue_runner.js

@@ -8,6 +8,7 @@
  * @flow
  */
 
+import PCancelable from 'p-cancelable';
 import pTimeout from './p_timeout';
 
 type Options = {
@@ -24,10 +25,20 @@ type QueueableFn = {
   timeout?: () => number,
 };
 
-async function queueRunner(options: Options) {
+function queueRunner(options: Options) {
+  const token = new PCancelable((onCancel, resolve) => {
+    onCancel(resolve);
+  });
+
   const mapper = ({fn, timeout}) => {
-    const promise = new Promise(resolve => {
-      const next = () => resolve();
+    let promise = new Promise(resolve => {
+      const next = function(err) {
+        if (err) {
+          options.fail.apply(null, arguments);
+        }
+        resolve();
+      };
+
       next.fail = function() {
         options.fail.apply(null, arguments);
         resolve();
@@ -39,6 +50,9 @@ async function queueRunner(options: Options) {
         resolve();
       }
     });
+
+    promise = Promise.race([promise, token]);
+
     if (!timeout) {
       return promise;
     }
@@ -57,10 +71,16 @@ async function queueRunner(options: Options) {
     );
   };
 
-  return options.queueableFns.reduce(
+  const result = options.queueableFns.reduce(
     (promise, fn) => promise.then(() => mapper(fn)),
     Promise.resolve(),
   );
+
+  return {
+    cancel: token.cancel.bind(token),
+    catch: result.catch.bind(result),
+    then: result.then.bind(result),
+  };
 }
 
 module.exports = queueRunner;

yarn.lock

@@ -4679,6 +4679,10 @@ osenv@^0.1.4:
     os-homedir "^1.0.0"
     os-tmpdir "^1.0.0"
 
+p-cancelable@^0.3.0:
+  version "0.3.0"
+  resolved "https://registry.yarnpkg.com/p-cancelable/-/p-cancelable-0.3.0.tgz#b9e123800bcebb7ac13a479be195b507b98d30fa"
+
 p-finally@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/p-finally/-/p-finally-1.0.0.tgz#3fbcfb15b899a44123b34b6dcc18b724336a2cae"