set allowUncaught on hooks (#3669); closes #2302

* set allowUncaught on hooks * add test: hooks swallow uncaught exceptions * PR feedback
mochajs · Mar 6, 2019 · a0c03e3 · a0c03e3
1 parent df1937b
commit a0c03e3
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 2 deletions.
diff --git a/lib/runner.js b/lib/runner.js
@@ -371,6 +371,8 @@ Runner.prototype.hook = function(name, fn) {
       hook.ctx.currentTest = self.test;
     }
 
+    hook.allowUncaught = self.allowUncaught;
+
     self.emit(constants.EVENT_HOOK_BEGIN, hook);
 
     if (!hook.listeners('error').length) {

diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js
@@ -9,6 +9,7 @@ var Test = Mocha.Test;
 var Runnable = Mocha.Runnable;
 var Hook = Mocha.Hook;
 var noop = Mocha.utils.noop;
+var EVENT_HOOK_BEGIN = Runner.constants.EVENT_HOOK_BEGIN;
 var EVENT_TEST_FAIL = Runner.constants.EVENT_TEST_FAIL;
 var EVENT_TEST_RETRY = Runner.constants.EVENT_TEST_RETRY;
 var EVENT_RUN_END = Runner.constants.EVENT_RUN_END;
@@ -443,7 +444,7 @@ describe('Runner', function() {
   });
 
   describe('allowUncaught', function() {
-    it('should allow unhandled errors to propagate through', function(done) {
+    it('should allow unhandled errors to propagate through', function() {
       var newRunner = new Runner(suite);
       newRunner.allowUncaught = true;
       newRunner.test = new Test('failing test', function() {
@@ -453,7 +454,75 @@ describe('Runner', function() {
         newRunner.runTest();
       }
       expect(fail, 'to throw', 'allow unhandled errors');
-      done();
+    });
+
+    it('should not allow unhandled errors in sync hooks to propagate through', function(done) {
+      suite.beforeEach(function() {
+        throw new Error();
+      });
+      var runner = new Runner(suite);
+      runner.allowUncaught = false;
+
+      // We are monkey patching here with runner.once and a hook.run wrapper to effectively
+      // capture thrown errors within the event loop phase where Runner.immediately executes
+      runner.once(EVENT_HOOK_BEGIN, function(hook) {
+        var _run = hook.run;
+        hook.run = function(fn) {
+          function throwError() {
+            _run.call(hook, fn);
+          }
+          expect(throwError, 'not to throw');
+          done();
+        };
+      });
+
+      runner.hook('beforeEach', noop);
+    });
+
+    it('should allow unhandled errors in sync hooks to propagate through', function(done) {
+      suite.beforeEach(function() {
+        throw new Error('allow unhandled errors in sync hooks');
+      });
+      var runner = new Runner(suite);
+      runner.allowUncaught = true;
+
+      runner.once(EVENT_HOOK_BEGIN, function(hook) {
+        var _run = hook.run;
+        hook.run = function(fn) {
+          function throwError() {
+            _run.call(hook, fn);
+          }
+          var expected = 'allow unhandled errors in sync hooks';
+          expect(throwError, 'to throw', expected);
+          done();
+        };
+      });
+
+      runner.hook('beforeEach', noop);
+    });
+
+    it('async - should allow unhandled errors in hooks to propagate through', function(done) {
+      // the `done` argument, although unused, it triggers the async path
+      // see this.async in the Runnable constructor
+      suite.beforeEach(function(done) {
+        throw new Error('allow unhandled errors in async hooks');
+      });
+      var runner = new Runner(suite);
+      runner.allowUncaught = true;
+
+      runner.once(EVENT_HOOK_BEGIN, function(hook) {
+        var _run = hook.run;
+        hook.run = function(fn) {
+          function throwError() {
+            _run.call(hook, fn);
+          }
+          var expected = 'allow unhandled errors in async hooks';
+          expect(throwError, 'to throw', expected);
+          done();
+        };
+      });
+
+      runner.hook('beforeEach', noop);
     });
   });