Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uncaught TypeError: Cannot read property 'currentRetry' of undefined #2315

Closed
ksuszka opened this issue Jun 17, 2016 · 19 comments
Closed

Uncaught TypeError: Cannot read property 'currentRetry' of undefined #2315

ksuszka opened this issue Jun 17, 2016 · 19 comments
Labels
type: bug a defect, confirmed by a maintainer

Comments

@ksuszka
Copy link

ksuszka commented Jun 17, 2016

Tested on Mocha 2.5.3.

When uncaught error is thrown during "before" phase additional "Uncaught TypeError: Cannot read property 'currentRetry' of undefined" error is thrown from mocha.

Simple test case:

'use strict';

describe('test', function () {
    before(function () {
        require('http').createServer().listen(1);
    });

    it('something', function () {
    });
});

output:

  test
    1) "before all" hook

  2) Uncaught error outside test suite

  0 passing (27ms)
  2 failing

  1) test "before all" hook:
     Uncaught Error: listen EACCES 0.0.0.0:1
      at Object.exports._errnoException (util.js:953:11)
      at exports._exceptionWithHostPort (util.js:976:20)
      at Server._listen2 (net.js:1240:19)
      at listen (net.js:1289:10)
      at Server.listen (net.js:1385:5)
      at Context.<anonymous> (test/test.js:5:40)

  2) test Uncaught error outside test suite:
     Uncaught TypeError: Cannot read property 'currentRetry' of undefined
@boneskull
Copy link
Member

haven't confirmed this, but it seems feasible. what reporter are you using?

@boneskull boneskull added the type: bug a defect, confirmed by a maintainer label Jun 17, 2016
@ksuszka
Copy link
Author

ksuszka commented Jun 22, 2016

I've tested it with the default reporter, but I've just checked spec, nyan, json reporters and same issue occurs.

It seems that it happens only if an error is thrown from asynchronous code. So even simpler test case is:

'use strict';

describe('test', function () {
    before(function () {
        setImmediate(function () {
            throw Error('error');
        });
    });

    it('something', function () {
    });
});

Note: I've just checked that the previous test case worked on linux under root account as it seems you can setup http server on port 1 as a root, so if you test under linux use sample with setImmediate or try to open same port twice.

@ScottFreeCode
Copy link
Contributor

Remind me to double-check that this doesn't overlap with #2220.

@boneskull
Copy link
Member

@ksuszka Thanks for the help here.

@boneskull
Copy link
Member

Having a hell of a time getting this to reproduce in a test environment, which sucks.

@ksuszka
Copy link
Author

ksuszka commented Jun 23, 2016

Do you use docker by any chance?

I've created a docker image with this issue. If you have docker then you can recreate the issue by running:

docker run ksuszka/mocha_issue_2315_demo

This is output on my machine:

Unable to find image 'ksuszka/mocha_issue_2315_demo:latest' locally
latest: Pulling from ksuszka/mocha_issue_2315_demo
51f5c6a04d83: Already exists
a3ed95caeb02: Already exists
7004cfc6e122: Already exists
d2e8a01d4f62: Already exists
d86212eebced: Already exists
4d1cea628cb5: Pull complete
10ba257b6e63: Pull complete
f652cf24f303: Pull complete
Digest: sha256:a2602f05b2e37cb11be9cb1e506b28f7bf9e43aaf4be92642fde89886e4c35e4
Status: Downloaded newer image for ksuszka/mocha_issue_2315_demo:latest


  test
    1) "before all" hook

  2) Uncaught error outside test suite

  0 passing (43ms)
  2 failing

  1) test "before all" hook:
     Uncaught Error: error
      at Error (native)
      at Immediate._onImmediate (test.js:6:19)

  2) test Uncaught error outside test suite:
     Uncaught TypeError: Cannot read property 'currentRetry' of undefined
      at _combinedTickCallback (internal/process/next_tick.js:67:7)
      at Immediate._tickCallback [as _onImmediate] (internal/process/next_tick.js:98:9)

The source code for this docker image is here: https://github.com/ksuszka/mocha_issue_2315_demo.

@boneskull
Copy link
Member

@ksuszka Thanks. I can reproduce the issue fine, I just can't reproduce it in a way that's testable; thus, I can't assert it's fixed if I can't assert it's broken.

@boneskull
Copy link
Member

I'm not sure spinning my wheels on this is the best use of my time. I get the impression that this isn't a showstopper. I'd happily accept a PR to fix it, if it can be tested.

@boneskull boneskull added pr-needs-work status: accepting prs Mocha can use your help with this one! and removed pr-needs-work labels Jun 24, 2016
@gurdiga
Copy link

gurdiga commented Jul 11, 2016

Hey guys! 🙂

Just a quick thought: If this is in the context of async setup code, shouldn’t we then use the done callback to tell Mocha about that? 🤔

I mean something like this:

'use strict';

describe('test', function () {
    before(function (done) {       // <--------------- added the `done` argument
        setImmediate(function () {
            throw Error('error');
            done();
        });
    });

    it('something', function () {
    });
});

In this case it the failure message seems to be more clear:

    ~/tmp/mocha ɀ  mocha 2315.js

  test
    1) "before all" hook

  0 passing (20ms)
  1 failing

  1) test "before all" hook:
     Uncaught Error: error
      at Error (native)
      at Immediate._onImmediate (2315.js:6:19)


    ~/tmp/mocha ɀ  mocha --version
2.5.3
    ~/tmp/mocha ɀ  node --version
v6.2.2
    ~/tmp/mocha ɀ  

Which makes me think that this may be categorized as a slight misuse instead of a bug. What do you guys think? 🤓

@1999
Copy link
Contributor

1999 commented Aug 14, 2016

I had a bit of time to investigate into this issue. The interesting thing here is that this code works as expected (0 passing, 1 failing):

describe('issue-2315: cannot read property currentRetry of undefined', function () {
  before(function () {
    throw new Error;
  });

  it('something', function () {});
});

and this does not (0 passing, 2 failing, uncaught error outside test):

describe('issue-2315: cannot read property currentRetry of undefined', function () {
  before(function () {
    require('http').createServer().listen(1);
  });

  it('something', function () {});
});

So it feels like the exception is fired inside http module. So I dived into net module of node.js which is used for listen function of http.Server. And if smth goes wrong (for example like in @ksuzska example when current user has no rights to listen to port 1), error is emitted but on the next tick but before() was sync and this is the reason of "uncaught error outside test" error. So it looks like the problem here is just about the wrong interpretation of the error. A simple dirty fix for that is inside my PR.

I'm far from understanding mocha design, but it seems like my fix is a really dirty one. The problem is: Runner.prototype.runTests is called, it calls Runner.prototype.hookDown('beforeEach', cb) which is async. Then uncaught exception is fired and caught inside "before all" hook. But beforeEach (hookDown) callback is still called and by that time self.test is deleted (after afterAll hook). So this seems to be the bigger problem: test is still running even despite afterAll was run.

If there are mocha developers here, please correct me if I'm wrong.

@1999
Copy link
Contributor

1999 commented Aug 23, 2016

cc @boneskull

@mikermcneil
Copy link

(for the record, seeing this with mocha@2.4.5 as well)

@ljharb
Copy link

ljharb commented Sep 1, 2016

We've been seeing this on mocha for awhile now on our large codebase, but couldn't narrow it down to specific tests. Any progress here?

@boneskull
Copy link
Member

@ljharb @1999 A PR was sent (#2439) that needs some review.

@boneskull boneskull removed the status: accepting prs Mocha can use your help with this one! label Sep 18, 2016
@ljharb
Copy link

ljharb commented Sep 22, 2016

@boneskull thanks!!! when do you think this might be released?

@boneskull
Copy link
Member

@ljharb If I had to guess, somewhere between 0 and 7 days from now.

1999 pushed a commit to 1999/mocha that referenced this issue Sep 22, 2016
…-files-cache

* upstream/master:
  attempt windows-friendly reproducible case for mochajs#2315
  fix: fix uncaught TypeError if error occurs on next tick, closes mochajs#2315 (mochajs#2439)
  helpful error when necessary suite callback omitted; closes mochajs#1744
  Fix an issue and add relevant tests when describe and xdescribe fail when not supplied with a callback (issue mochajs#1744).
  rename more fixtures; closes mochajs#2383
  Report non-match to STDERR and exit if no files are matched (mochajs#2450)
  Exit process with correct error codes (mochajs#2445)
  fix test files not using .spec suffix fix test fixtures not using .fixture suffix
  always halt execution when async skip() called; closes mochajs#2465 (mochajs#2479)
  avoid test flake in "delay" test; closes mochajs#2469 (mochajs#2470)
  revert accidental change to bin paths
  disregard function names when testing under Node.js 6.5.0; closes mochajs#2467 (mochajs#2477)
  lints more files; add more files to lint check; closes mochajs#2457
  adds *.orig to .gitignore
  Exclude the --inspect flag
  fix check-leaks to catch a leak whose name begins with 'd'
@muneermuhammed
Copy link

I am facing issue when I run my tests with mocha-allure-reporter ,the tests are running successfully with mocha ,but mocha --reporter mocha-allure-reporter returns the following error

notifications-api@1.0.0 test /app
mocha --reporter mocha-allure-reporter

2016-11-15T06:13:26.762Z - info: Notification API up and running on port 4000
2016-11-15T06:13:26.773Z - error: Tue, 15 Nov 2016 06:13:26 GMT uncaughtException
2016-11-15T06:13:26.775Z - error: TypeError: test.currentRetry is not a function
at Runner. (/app/node_modules/mocha-allure-reporter/index.js:29:19)
at emitOne (events.js:77:13)
at Runner.emit (events.js:169:7)
at next (/app/node_modules/mocha/lib/runner.js:517:10)
at Runner.runTests (/app/node_modules/mocha/lib/runner.js:556:3)
at /app/node_modules/mocha/lib/runner.js:637:10
at next (/app/node_modules/mocha/lib/runner.js:283:14)
at Immediate._onImmediate (/app/node_modules/mocha/lib/runner.js:319:5)
at processImmediate as _immediateCallback
npm ERR! Test failed. See above for more details.
Can anyone help to fix this issue?

sgilroy pushed a commit to TwineHealth/mocha that referenced this issue Feb 27, 2019
@afrancht
Copy link

afrancht commented Jan 29, 2020

@boneskull @ksuszka @1999

I'm still running into this issue although it has said to be fixed. I'm running mocha 6.2.2 though. Any help?

      describe('sign up as a user when an account for that user already exists', () => {
            before(async () => {
                await createTestUser();
            });
            it.only('should not let me signup as a user if an account with that email exists.', done => {
                const { email, authKey } = user;

                chai.request(app)
                    .post('/auth/user/signup')
                    .type('application/json')
                    .send({ email, authKey})
                    .end(function(err, res) {
                        expect(res).to.have.status(200);
                        assert.deepEqual(res.body, {
                            message: 'That username already exists.',
                        });

                        done();
                    });
            });

            after(async () => {
                  await deleteTestUserByEmail(user.email) 
            });
        });

Error

1) Auth Routes Tests
       /auth/signup route
         sign up as a user when an account for that user already exists
           "before all" hook:
     Uncaught TypeError: Cannot read property 'email' of null
      at /Users/aaa/git/webapp_ps/tests/routes/authRoutes.test.js:9:3126
      at /Users/aaa/git/webapp_ps/node_modules/mongoose/lib/model.js:4791:16
      at /Users/aaa/git/webapp_ps/node_modules/mongoose/lib/query.js:4389:12
      at model.Query.Query._completeOne (node_modules/mongoose/lib/query.js:2073:12)
      at Immediate.<anonymous> (node_modules/mongoose/lib/query.js:2135:10)
      at Immediate.<anonymous> (node_modules/mquery/lib/utils.js:116:16)
      at processImmediate (internal/timers.js:456:21)

Function being called in before:

const createTestUser = async () => {
    try {

        const newUser = new User();
        userId = newUser._id;
        newUser.email = user.email;
        newUser.hashedPassword = await newUser.hashPassword(
            user.authKey.hashHex);
        newUser.type = user.type;
        newUser.mukSalt = user.mukSalt;
        newUser.authSalt = user.authSalt;
        await newUser.save();
        return userId;
    }catch(e) {
        console.log('>>>>>>>>>>>>Error', e)
    }
};

Assume user exists as I can console log it and the test is running and using it.

Any ideas?

@juergba
Copy link
Member

juergba commented Jan 30, 2020

Where do you set user? Not in the before hook, I guess you leave it undefined?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

10 participants