karma-mocha hangs when there's a exception in afterEach hook

[mclin]

Prerequisites

• Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
• Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
• 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
• Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

If an exception/assert error occurs in a hook, karma-mocha stops immediately due to an unhandled exception.

Steps to Reproduce

Example repo: https://github.com/mclin/karma-mocha-bug

Create a test that throws in afterEach:

test.test.js

describe('test.test', function() {
  afterEach(function() {
    throw new Error('error');
  });

  it('should pass', function() {
  });
});

Run this test in with karma and karma-mocha

karma.conf.js

module.exports = function(config) {
  config.set({
    frameworks: ['mocha'],
    files: [
      'test.test.js',
    ],
    browsers: ['Chrome']
  });
};

Run it with karma start and see the tests stall and the browser times out.

Expected behavior:
Test should complete the tests with an error

Mocha 7.0.1

23 04 2020 15:58:01.800:INFO [karma-server]: Karma v5.0.2 server started at http://0.0.0.0:9876/
23 04 2020 15:58:01.803:INFO [launcher]: Launching browsers Chrome with concurrency unlimited
23 04 2020 15:58:01.809:INFO [launcher]: Starting browser Chrome
23 04 2020 15:58:03.763:INFO [Chrome 81.0.4044.122 (Mac OS 10.14.6)]: Connected on socket 29Qi9Yrf-M0n-eQmAAAA with id 74115074
Chrome 81.0.4044.122 (Mac OS 10.14.6) test.test "after each" hook for "should pass" FAILED
	Error: error
	    at Context.<anonymous> (test.test.js:3:11)
Chrome 81.0.4044.122 (Mac OS 10.14.6): Executed 2 of 1 (1 FAILED) (0.003 secs / 0 secs)
TOTAL: 1 FAILED, 1 SUCCESS

Actual behavior: [What actually happens]

It hangs and the browser disconnects.
Mocha 7.1.1

23 04 2020 15:34:39.525:INFO [karma-server]: Karma v5.0.2 server started at http://0.0.0.0:9876/
23 04 2020 15:34:39.529:INFO [launcher]: Launching browsers Chrome with concurrency unlimited
23 04 2020 15:34:39.534:INFO [launcher]: Starting browser Chrome
23 04 2020 15:34:40.673:INFO [Chrome 81.0.4044.122 (Mac OS 10.14.6)]: Connected on socket 2Lwvatd4wKVZh7ndAAAA with id 76880926
Chrome 81.0.4044.122 (Mac OS 10.14.6): Executed 1 of 1 SUCCESS (0 secs / 0.001 secs)
Chrome 81.0.4044.122 (Mac OS 10.14.6) ERROR
  Disconnected, because no message in 30000 ms.
Chrome 81.0.4044.122 (Mac OS 10.14.6): Executed 1 of 1 DISCONNECTED (30.011 secs / 0.001 secs)

Reproduces how often: 100%

Versions

mocha 7.1.1
karma-mocha 2.0.0
karma 5.0.2

MacOS 10.14.6
Chrome 81.0.4044.113

Additional Information

This issue was introduced in mocha 7.1.0 here:
a995e33

Unhandled exception occurs here

karma-mocha is passing a hook object as the param with a 'test end' event.
https://github.com/karma-runner/karma-mocha/blob/master/src/adapter.js#L132

The hook doesn't have a retriedTest method, so it throws "Uncaught TypeError: test.retriedTest is not a function"

Either karma-mocha shouldn't be passing a hook into "test end" or there needs to be a duck/type check for calling retriedTest()

[juergba]

@mclin thank you for this issue and your detailed analysis.

Either karma-mocha shouldn't be passing a hook into "test end" or there needs to be a duck/type check for calling retriedTest()

I will have a look. Emitting a test end for a hook (karma-mocha) is wrong. But I guess I will add a check in our code.

[juergba]

cc @andrewminer

[juergba]

@mclin could check please wether it works with following change?

• old line: if (test.retriedTest() && test.parent) {
• new line: if (test.type === 'test' && test.retriedTest() && test.parent) {

Edit: From my side it works. The change has to be done in the bundled file mocha/mocha.js, and not in mocha/lib/runner.js.

[mclin]

Works on my end! Sorry I didn't see that other issue.