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

Unhandled rejection warnings when expecting a rejection #173

Closed
laggingreflex opened this issue Oct 30, 2016 · 29 comments
Closed

Unhandled rejection warnings when expecting a rejection #173

laggingreflex opened this issue Oct 30, 2016 · 29 comments

Comments

@laggingreflex
Copy link

I'm not using Bluebird but similarly to this thread I'm getting the same warning errors of unhanded rejection when actually expecting a rejection.

Here's a repo to reproduce: https://github.com/laggingreflex/chai-as-promised-reject-test

It was mentioned chai-as-promised is indeed handling the errors (here) but I think they're not being handled soon enough. Putting a console.log there shows that it reaches that code only after the warning has been displayed:

(node:5580) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: rejected
  promise
(node:5580) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
>>> assertIfNegated about to be called... <<<
    √ should be rejected


  1 passing (48ms)
@domenic
Copy link
Collaborator

domenic commented Oct 30, 2016

This is a bug introduced in Node.js v7.0.0. I can only recommend not using v7 for now. See nodejs/node#8217.

@domenic domenic closed this as completed Oct 30, 2016
@darkobits
Copy link

darkobits commented Oct 31, 2016

I am also experiencing this issue on Node 6.9.1 and:

chai@3.5.0
chai-as-promised@6.0.0
mocha@3.1.0

This spec should fail:

import chai from 'chai';
import chaiAsPromised from 'chai-as-promised';

chai.use(chaiAsPromised);
const expect = chai.expect;

describe('Test', () => {
  it('should fail', () => {
    expect(Promise.resolve(2)).to.eventually.equal(1);
  });
});

Instead, the output is:

  Test
    ✓ should fail
(node:52529) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): AssertionError: expected 2 to equal 1

  4 passing (27ms)

And the process exits with a 0 exit code.

@domenic
Copy link
Collaborator

domenic commented Oct 31, 2016

Ugh, it does seem this kind of major change slipped in to Node.js 6.x as well, although that bug thread is confusing :(.

@laggingreflex
Copy link
Author

@darkobits I think you need to return the expect

  it('should fail', () => {
    return expect(Promise.resolve(2)).to.eventually.equal(1);
  });

Also I think my (OP) problem was that I was using chai v4, I tested it again and the original test lib works well with chai 3.5

@keithamus
Copy link
Member

@laggingreflex do you think this might be a regression in chai v4? If so please please do file a report on the chai issue tracker 😄

@meeber
Copy link
Contributor

meeber commented Oct 31, 2016

Also might wanna check out #157. However, I submitted that a long time ago and there have been a lot of changes since then; it may need further updating.

@adamvoss
Copy link

adamvoss commented Jun 5, 2017

Is this still considered a node bug? I am seeing this too in node 8.0.0.

const chai = require('chai');
chai.use(require("chai-as-promised"));
const assert = chai.assert;

describe('rejection assertion', function(){
  it('should correctly fail the test', function(){
    const p = Promise.resolve("resolved")
    return assert.isRejected(p)
  })
})

The above erroneously passes:

rejection assertion
✓ should correctly fail the test
(node:27794) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): AssertionError: expected promise to be rejected but it was fulfilled with 'resolved'
(node:27794) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

1 passing (37ms)

    "chai": "^4.0.1",
    "chai-as-promised": "^6.0.0",
    "mocha": "^3.4.2"

@meeber
Copy link
Contributor

meeber commented Jun 5, 2017

@adamvoss chai-as-promised doesn't support Chai v4 yet. See #198 and #157.

@zazoomauro
Copy link

zazoomauro commented Jun 8, 2017

I'm having exactly the same issue right there

version:
node: v6.11.0
chai: ^3.5.0
chai-as-promised: ^6.0.0
mocha: ^3.2.0
sinon: ^2.1.0

Source Code:

export default class SomeManager {
  /**
   * @param {SomeOtherManager} someOtherManager
   */
  constructor(someOtherManager) {
    this._someOtherManager = someOtherManager;
  }

  /**
   * @return {Promise}
   */
  execute() {
    return this._someOtherManager.get('id');
  }
}

Spec file:

import chai from 'chai';
import cp from 'chai-as-promised';
import SomeManager from './../../lib/manager/someManager';
import SomeOtherManager from './../../lib/manager/someOtherManager';
import {beforeEach, describe, it} from 'mocha';
import sinon from 'sinon';

describe('SomeManager', () => {
  chai.use(cp);
  let expect = chai.expect;
  let someManager;
  let someOtherManagerMock;

  beforeEach(() => {
    someOtherManagerMock = sinon.createStubInstance(SomeOtherManager);
    someManager = new SomeManager(someOtherManagerMock);
  });

  describe('execute', () => {
    it('should reject promise', () => {
      // Arrange.
      someOtherManagerMock.get.withArgs('id').returns(Promise.reject());

      // Act.
      let actual = someManager.execute();

      // Assert.
      return expect(actual).to.eventually.be.resolved;
    });
  });
});

Output:

SomeManager
    execute
      ✓ should reject promise
(node:11844) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): undefined

With node: 8.1.0 different Output:

    execute
      ✓ should reject promise
(node:12102) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): undefined
(node:12102) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@meeber
Copy link
Contributor

meeber commented Jun 9, 2017

@zazoomauro In your case, the problem is that .resolved isn't an assertion in chai-as-promised. I think you need .fulfilled.

@zazoomauro
Copy link

@meeber Thanks!

@ChrisCurrin
Copy link

I also have the same problem
node: v6.11.0
chai: 3.5.0 -- also tested on ^4.0.1
chai-as-promised: ^7.1.1
mocha: ^3.4.2

@vitalets
Copy link

vitalets commented Aug 9, 2017

Have the same issue.
The solution for node:

process.on('unhandledRejection', () => {});

@cawak
Copy link

cawak commented Sep 18, 2017

Same here:

node: v6.9.1
chai: 3.5.0
chai-as-promised: 7.1.1
mocha: 3.5.3

@vitalets, I think this is not a solution but rather a hack to hide the warning. But it can work in case someone wants to avoid pull request issues :)

Edit: I now think the issue is with the 3rd party library I'm using (aws-sdk-mock module). All tests pass as expected, but this warning makes me wonder if I understand correctly the promise feature.

I noticed that passing an error argument to the callback in the test like this:

AWS.mock('Lambda', 'invoke', function(params, callback){
                callback('Some error');
            });

And the piece of code I test:

return new Promise((resolve, reject) => {
            var lambda = new AWS.Lambda();
            const tokenRetrieverLambdaName = process.env.TR_LAMBDA_NAME;

            lambda.invoke(parameters, (error, response) => {
                if (error) {
                    console.error('Failed to retrieve token for evolve using external lambda.', error);
                    reject("Other error message"); // the reject that creates the duplicates and the warning.
                } else if (response && response.Payload) {
                    reject("For other reason");
                } else { ... }
           }
});

Behaves very strangely when I reject the promise in the code I test. When I reject it with provided error argument ('Some error' of the callback) then it shows the warning but with the 'Some error' message and not with the "Other error message" that I rejected in the tested code (by the way my test catches the "Other error message" message correctly).
When I don't provide the first (error) argument to the callback in the mock, everything works fine even with the rejections (no warnings with the "For other reason" rejection).

If I have time in the weekend, I may look at the code of 'aws-sdk-mock' to confirm the issue (and hopefully to provide a quick fix).

@meeber
Copy link
Contributor

meeber commented Sep 19, 2017

There are multiple issues brought up by people in this thread. Some of the issues were resolved with the release of chai@4 and chai-as-promised@7. Some of the issues were resolved by properly returning the promise in the test so mocha can properly handle the asynchronous test. However, the original problem demonstrated by the OP here isn't resolved, and I don't think it can be resolved by chai or chai-as-promised.

Consider the OP's original example with a console.log added inside the it block:

const chai = require( 'chai' );
const promised = require( 'chai-as-promised' );

chai.use( promised );
chai.should();


const somePromise = new Promise( () => {
  throw new Error( 'rejected' );
} );

describe( 'promise', () => {
  it( 'should be rejected', () => {
    console.log("NOOOOOOOOOOOOO WHYYYYYYYYYYYYYYYY");
    return somePromise.should.be.rejected;
  } );
} );

Output:

mocha index.js

(node:2769) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: rejected
(node:2769) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
promise
NOOOOOOOOOOOOO WHYYYYYYYYYYYYYYYY
(node:2769) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
✓ should be rejected

1 passing (13ms)

An UnhandledPromiseRejectionWarning is generated before the it function is even executed. Moving the Promise declaration into the describe function or inside of a before function results in the same behavior. Only by moving the Promise declaration directly into the it function (or a beforeEach function) does the warning go away.

I don't have time to dig deeper, but this suggests to me that Mocha is introducing at least a one tick delay before it functions are executed.

@pontusnyman
Copy link

pontusnyman commented Nov 20, 2017

return the expect worked for me. And when I think about, it makes sense.

@ScottFreeCode
Copy link

ScottFreeCode commented Nov 28, 2017

Pseudo-cross-posting idea from mochajs/mocha#3009 (comment)

I have something of a workaround for #173 (comment)

const chai = require( 'chai' );
const promised = require( 'chai-as-promised' );

chai.use( promised );
chai.should();


const makeSomePromise = () => new Promise( () => {
  throw new Error( 'rejected' );
} );

describe( 'promise', () => {
  it( 'should be rejected', () => {
    return makeSomePromise().should.eventually.be.rejected; // note the addition of `.eventually`
  } );
} );

One thing to be aware of, although I think this is inherent to promises' eagerness and error handling design and will be relevant no matter whether Mocha is changed to have no tick delays*, is that if you recreate the promise on a per-test basis then every test must handle its rejection (even if it isn't relevant, e.g. if you're testing that it starts off pending), while on the other hand if you were to reuse a rejected promise in multiple tests then the first test must handle the rejection and once it does so there will be no warning if any subsequent test fails to do so even if it needs to (in other words, even if Mocha allowed you to assign a promise once and use it in multiple tests, it'd be almost equivalent to assigning the promise and immediately calling somePromise.catch(()=>{}) [EDITTED TO ADD: without assigning the result of that action, so keeping the original somePromise in its rejected state] to suppress rejection warnings for that promise). In that sense, creating a new promise for every test (whether with beforeEach or with a thunk as in my workaround) would be the more safe/robust approach (if, in certain unusual cases, slightly annoying).

*(NOTE: In this example any asynchronous test or hook can introduce a delay between the variable assignment outside of any test or hook and actually getting to run this test; you would have to move that variable assignment into a before/beforeEach hook that runs just prior to this test in addition to Mocha removing the delays.)

@sfengyuan
Copy link

sfengyuan commented Dec 1, 2017

Return expect won't sovle the problem, the test can not pass still, and I think this is an upstream problem, cuz it happens on assert package built in Node, maybe mocha.

describe('Whay', function () {
  describe('It should pass but can not pass', function (done) {
    whay('test', function (_, obj) {
      console.log(obj)
      // assert.equal(obj.keyword, 'test')
      // done()
    })
  })
})

Here it complains obj is undefined, but it is logged to console successfully.

@hiranya911
Copy link

Seeing this with Node 6.10.2 and Mocha 3.5.3. Has anybody found a reasonable fix/workaround?

My tests cases are of the following form:

it('should throw for unexpected HTTP errors', () => {
     return expect(methodThatTriggersRejection())
          .to.be.rejectedWith(expectedError);
});

@meeber
Copy link
Contributor

meeber commented Dec 13, 2017

@hiranya911 It looks to me like something else might be going on in your example than what is being discussed here. For example, I just tested this standalone example that's based on your example, and it works:

function methodThatTriggersRejection () {
  return Promise.reject(TypeError('testing'));
}

it('should throw for unexpected HTTP errors', () => {
     return expect(methodThatTriggersRejection())
          .to.be.rejectedWith(TypeError, 'testing');
});

Are you sure that methodThatTriggersRejection is returning a rejected Promise object?

@hiranya911
Copy link

hiranya911 commented Dec 13, 2017

It has a promise chain whose catch block throws an exception. I assumed this gets translated into a rejected promise, especially since the assertions in my tests pass:

return Promise.resolve()
  .then(() => {

  })
  .catch((error) => {
     throw new MyException("error message"); // This is what my assertions are checking for
  });

Edit:

I just tried returning the exception wrapped in promise rejection as follows.

return Promise.reject(new MyException("error message"));

The tests still passed, and I still got the warnings.

(node:156176) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 151): [object Object]
      ✓ should throw for unexpected HTTP errors
(node:156176) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 151)

@meeber
Copy link
Contributor

meeber commented Dec 14, 2017

@hiranya911 Does the example I provided work without warnings if you copy/paste it into your current project? If not, does it still display warnings if you copy/paste it into an empty project? I suspect you'll need to produce a standalone, reproducible example for us to be able to assist further.

Something is causing an extra tick of the event loop to occur between your promise being rejected and it being "handled" by chai-as-promised. However, based on the examples you've provided so far, it doesn't look to be the same issue as discussed in this thread. The issue being discussed in this thread is that when a promise is rejected outside of the it block from which the promise is being asserted upon, an extra tick of the event loop occurs due to Mocha internals. In your example, the promise is being rejected within the same it block and thus wouldn't be subjected to the Mocha-induced extra tick.

@hiranya911
Copy link

@meeber I think I've narrowed it down a bit. I'm using sinon to stub out certain operations, and simply having the following (by itself with no other code) in the test triggers warnings:

let stub = sinon.stub(HttpRequestHandler.prototype, 'sendRequest')
        .returns(Promise.reject(expectedResult));

@hiranya911
Copy link

@meeber I think I'm running into the issue described here... http://clarkdave.net/2016/09/node-v6-6-and-asynchronously-handled-promise-rejections/

Solution would be to use sinon-as-promised package.

@brndn4
Copy link

brndn4 commented Mar 22, 2018

@hiranya911 Thanks! that worked for me. Another alternative is to use callsFake() instead of returns() to return the rejected promise from the stub. Also, it looks like the latest sinon version includes the rejects/returns functions.

@mlakmal
Copy link

mlakmal commented Oct 10, 2018

i encountered same issue but was able to resolve this by using async/await like below.

await expect(medProfile.allergiesSection.getTagName()).to.eventually.equal('div');

originally i had it like below. which threw rejection when element not found. but cucumber test passed without catching that error.

Then(/^user should see allergies and health conditions section$/, async () => { expect(medProfile.allergiesSection.getTagName()).to.eventually.equal('div'); });

@vitalets
Copy link

With async/await you don't need chai-as-promised at all:

expect(await medProfile.allergiesSection.getTagName()).to.equal('div');

@unicornist
Copy link

unicornist commented Feb 20, 2020

I think the major issue here is the bad documentation of chai-as-promised that confuses everybody.

All of these pass without an unhandled promise rejection warning and without any await or returning anything.

expect(Promise.reject).to.be.rejected
expect(Promise.reject).to.be.rejectedWith(YourError)
expect(Promise.reject).to.eventually.rejectedWith(Error).with.property('name', 'YourError')
expect(Promise.reject).to.be.rejectedWith(Error).eventually.with.property('name', 'YourError')

but this shows warnings even with eventually

expect(Promise.reject).to.eventually.throw(Error)

@iongion
Copy link

iongion commented Oct 16, 2020

Still having this issue, behaving randomly, this always works

      it('handles message error', (done) => {
        const error = new Error('Solver internal error');
        producerRPC.sendPayload(consumerHost, {} as any).catch((err) => {
          expect(err).to.be.equal(error);
        });
      });

While this does not

    describe('sendPayload.robust', () => {
      it('handles message error', async () => {
        const error = new Error('Solver internal error');
        return producerRPC.sendPayload(consumerHost, {} as any).should.eventually.be.rejectedWith(error);
      });
    });

Sometimes I see (node:114025) UnhandledPromiseRejectionWarning: Error: , sometimes I don't

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests