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

rejects.toThrowError #3601

Closed
BetterCallSky opened this issue May 18, 2017 · 53 comments

Comments

@BetterCallSky
Copy link
Contributor

commented May 18, 2017

jest version: 20.0.3

For sync method, it works in this way.

test('example', () => {
  function fn() {
    throw new Error('some error');
  }

  expect(fn).toThrowError('some error');
});

toThrowError doesn't work with promises.

test('example', async () => {
  async function fn() {
    throw new Error('some error');
  }

  await expect(fn()).rejects.toThrowError('some error');
});

I am getting an error:

expect(function).toThrowError(string)
Received value must be a function, but instead "object" was found

This works! The async function throws a function that throws an error.

test('example', async () => {
  async function fn() {
      const errorFunction = () => {
        throw new Error('some error');
      };
      throw errorFunction;
    }

    await expect(fn()).rejects.toThrowError('some error');
  });

Obiously it doesn't make sense. I can see this line in the source code.
https://github.com/facebook/jest/pull/3068/files#diff-5d7e608b44e8080e7491366117a2025fR25
It should be Promise.reject(new Error()) instead of Promise.reject(() => {throw new Error();}

The example from the docs also doesn't work

test('fetchData() rejects to be error', async () => {
  const drinkOctopus = new Promise(() => {
      throw new Error('yuck, octopus flavor');
  });

  await expect(drinkOctopus).rejects.toMatch('octopus');
});
@ghost

This comment has been minimized.

Copy link

commented May 24, 2017

Try awaiting inside the expect block, as in expect(await drinkOctopus).rejects.toMatch('octopus');

@BetterCallSky

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2017

It won't work... an error will be thrown and 'expect' cant catch that.

@ghost

This comment has been minimized.

Copy link

commented May 24, 2017

Found it (I stumbled upon your issue because I had the same problem :))

describe("When a collection is not passed", () => {
      it("Returns an error", async () => {
        await expect(cqrs.MongoEventStore.create()).rejects.toEqual(cqrs.ErrCollectionRequired)
      })
})

Test passes on my machine. I think you can close it now.

@julienw

This comment has been minimized.

Copy link

commented May 24, 2017

What we want is to match a error message (that is, the Error.prototype.message property), just like the documentation says.

Otherwise it's easy to use something like .toEqual(expect.any(Error))

@ghost

This comment has been minimized.

Copy link

commented May 24, 2017

Use toMatch instead of toEqual, then. Unless I missed your point, in which case I apologise.

@julienw

This comment has been minimized.

Copy link

commented May 24, 2017

toMatch is what I want, the point of this bug is that it doesn't work :) See last example in the initial comment.

@ghost

This comment has been minimized.

Copy link

commented May 24, 2017

Apologies, I should not try to help at this late time of the day. I now understood what you mean. It is a good point, but I believe that the example is incorrect, and not the implementation. Let me try to explain what I mean, I hope it makes a tiny bit of sense.
If you look at this (actual) code:

        const collection = {
          createIndex: jest.fn(() => {
            return new Promise((resolve, reject) => { reject(message) })
          })
        }
        await expect(cqrs.MongoEventStore.create(collection)).rejects.toMatch(message)

That works. If you look at the Javascript specification, a Promise does not coerce developers to use Error as a reason for the reject. Yes, that's what some examples specify (as in here), but the specification talks about a generic reason.
In this case, Jest does not make any assumption and tries to match the string against the reason, irrespective of its type.
Does it make any sense at all? I agree that it is not ideal and that some intelligence could be added to extract the message from an Error, but that's too much for tonight :)

My two pennies worth of it.

@julienw

This comment has been minimized.

Copy link

commented May 24, 2017

Mmm I see what you mean. Yet Jest's documentation use the example of a real error with toMatch :) see https://facebook.github.io/jest/docs/expect.html#rejects . Either the example in the doc is wrong, or the implementation is wrong :)

I think this is needlessly difficult to check an error's message with Jest, especially when we want to match.

@BetterCallSky

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2017

@imeruli
reject should be called with an Error object, not a literal string because you don't get the stacktrace.
It's definitely a bad practice.

When using async/await you should write

async foo() {
  throw new Error('some error');
}

not

async foo() {
  throw 'some error';
}
@ghost

This comment has been minimized.

Copy link

commented May 25, 2017

Isentkiewicz, I do no not disagree, however, from a specification standpoint, it is written nowhere.

@BetterCallSky

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2017

See the "Description" section from the link you referred.

The static Promise.reject function returns a Promise that is rejected. For debugging purposes and selective error catching, it is useful to make reason an instanceof Error.

@ecstasy2

This comment has been minimized.

Copy link

commented Jun 21, 2017

+1 on this.

I use this for now:

await expect(Promise.rejects(new Error('Some error')))
  .rejects.toMatchObject({
    message: 'Some error',
  });

This is a temporary workaround if you are willing to match the whole error message. Obviously, jest should support such basic thing.

@vgracia

This comment has been minimized.

Copy link

commented Jun 21, 2017

+1 It would be great if I can use reject with an error object .

@franciscop

This comment has been minimized.

Copy link

commented Jul 14, 2017

What about a toRejectWith() that handles that message? This being a specific situation for rejected errors:

await expect(Promise.rejects(new Error('Some error')))
  .rejects.withMessage('Some error');

Or even this:

await expect(Promise.rejects(new Error('Some error')))
  .rejects.message.toMatch('Some error');
@lachlanhunt

This comment has been minimized.

Copy link

commented Jul 27, 2017

As a workaround for this issue, I came up with this aproach:

async function shouldRejectWithError() {
    throw new Error("An example error");
}

it('should reject with an error', async () => {
    let err;

    try {
        await shouldRejectWithError();
    } catch (e) {
        err = e;
    }

    expect(err.message).toMatch(/example/);
});

If it doesn't throw, err will be undefined and fail to read the message property. Otherwise, if it does throw, it verifies the message matches.

@nbransby

This comment has been minimized.

Copy link

commented Sep 18, 2017

Im confused - why does the documentation (still!) contain an example that doesnt work (https://facebook.github.io/jest/docs/en/expect.html#rejects) and prusumedly never worked?

Also why not solve it by having expect().toThrow() support promises? Then you could write:

expect(Promise.rejects(new Error('Some error'))).toThrow('Some error');

@jomel

This comment has been minimized.

Copy link

commented Sep 28, 2017

+1 same here.
just spent an hour trying to work our why I cant use expect().toThrow() when testing (async) mongoose DB actions & validators (with a not-very-useful jest message "Received value must be a function, but instead "object" was found")

@franciscop

This comment has been minimized.

Copy link

commented Sep 28, 2017

@lachlanhunt that would work, of course, but it is extremely verbose compared to other jest workflows. Probably you also want to check before hand that err is actually defined (there was an error thrown), otherwise it'll give an unhelpful undefined error.

BTW, for anyone reading so far, @ecstasy2 solution stopped working on Jest 21.0: #4532

@SimenB

This comment has been minimized.

Copy link
Collaborator

commented Sep 29, 2017

You can do expect(Promise.reject(new Error('something'))).rejects.toHaveProperty('message', 'something else'); which gives:

image

@acostalima

This comment has been minimized.

Copy link

commented Oct 18, 2017

To test multiple properties, the following is working:

expect(promise).rejects.toHaveProperty('prop1', 'value1');
expect(promise).rejects.toHaveProperty('prop2', 'value2');
@rexxars

This comment has been minimized.

Copy link

commented Oct 19, 2017

Is this considered a bug or just the way Jest is supposed to work?
If the latter is true, the documentation should reflect this. Currently it says you can use:

await expect(drinkOctopus).rejects.toMatch('octopus');

Which doesn't actually work if drinkOctopus is an instance of an Error, which as discussed earlier is considered "best practice". The toMatchObject solution mentioned previously doesn't work as of Jest 21, so you are reduced to things like toHaveProperty, which work, but doesn't support partial matching.

This feels cumbersome, to me. Is there a planned way to handle these rejections?

@SimenB

This comment has been minimized.

Copy link
Collaborator

commented Oct 19, 2017

IMO it's a bug (or missing feature, I suppose) (and as this has never been closed, I'm guessing other collaborators agree with me). Not really sure how to achieve symmetry with other matchers, but .toThrowErrorshould support (rejected) promises. Maybe making it aware that is is in a rejects state so it doesn't complain about not receiving a function somehow?

PR welcome!

@SimenB SimenB added the Help Wanted label Oct 19, 2017

franciscop added a commit to franciscop/server that referenced this issue Oct 25, 2017

@cpojer cpojer closed this in #4884 Nov 14, 2017

cpojer added a commit that referenced this issue Nov 14, 2017

fix .toThrow for promises (#4884)
* fix .toThrow for promises

fixes #3601

Signed-off-by: Łukasz Sentkiewicz <sirmims@gmail.com>

* keep original matcherName

Signed-off-by: Łukasz Sentkiewicz <sirmims@gmail.com>

* fix flow

Signed-off-by: Łukasz Sentkiewicz <sirmims@gmail.com>

* update changelog

Signed-off-by: Łukasz Sentkiewicz <sirmims@gmail.com>
@alem0lars

This comment has been minimized.

Copy link

commented Jan 19, 2018

@skbolton @SimenB The code snippet isn't working for me:

    it("foo", () => {
      return expect(Promise.reject(new Error("octopus"))).rejects.toMatchObject(
        {
          message: expect.stringMatching(/oct/),
        },
      );
    });

In fact it gives me the following error:


    expect(received).toMatchObject(expected)

    Expected value to match object:
      {"message": StringMatching /oct/}
    Received:
      [Error: octopus]
    Difference:
    - Expected
    + Received

      Object {
    -   "message": StringMatching /oct/,
    +   "message": "octopus",
      }

      at Object.<anonymous> (node_modules/expect/build/index.js:179:51)
          at Generator.throw (<anonymous>)
      at step (node_modules/expect/build/index.js:43:727)
      at node_modules/expect/build/index.js:43:926

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 passed, 2 total
Snapshots:   0 total
Time:        3.533s, estimated 4s
Ran all test suites matching /user.test/i.
@skbolton

This comment has been minimized.

Copy link

commented Jan 19, 2018

@alem0lars Sorry to get your hopes up. Maybe there has been a regression. I am running my tests on jest 20.0.4 I thought these tests were on a more current version. For what its worth though it was working then

@SimenB

This comment has been minimized.

Copy link
Collaborator

commented Jan 22, 2018

@alem0lars that seems like a different issue, it doesn't work without rejects either. See https://repl.it/repls/JoyousImpressiveYeti.

Could you open up a new issue about toMatchObject not working to assert on the message of an error?

@alem0lars

This comment has been minimized.

Copy link

commented Jan 22, 2018

@SimenB done :)

@agm1984

This comment has been minimized.

Copy link

commented Feb 5, 2018

Here is my solution:

test('addItem mutation rejects invalid user', async () => {
  try {
    await addItem(root, noArgs, invalidContext)
  } catch (e) {
    expect(e.message).toMatch('You must be authenticated to run this query.')
  }
})

It is essentially what @lachlanhunt posted.

If you simply execute the function under test with the arguments necessary to output the expected error, you can observe it in the catch block and do a simple string check.

@aymericbouzy

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2018

@agm1984 your test will pass if addItem does not throw, unless you use expect.assertions : Documentation

alloy added a commit to artsy/metaphysics that referenced this issue Feb 16, 2018

[test] Fix unhandled promise and the assertion.
* Need to await
* Had to use this way to assert, as suggested here:
  facebook/jest#3601 (comment)
@franciscop-invast

This comment has been minimized.

Copy link

commented May 18, 2018

As explained and based on the work here, I made a small function to help:

// My final utility function, "synchronizer":
const sync = fn => fn.then(res => () => res).catch(err => () => { throw err });

// Example function for testing purposes
const check = async arg => {
  if (arg > 5) throw new Error('Too large!');
  if (arg < 0) throw new Error('Too small!');
  return 'Good to go';
};

// Write the expect() in quite a clear way
it('checks the boundaries properly', async () => {
  expect(await sync(check(-1))).toThrow();  // Can also take 'Too small!' or /Too small!/
  expect(await sync(check(0))).not.toThrow();
  expect(await sync(check(5))).not.toThrow();
  expect(await sync(check(10))).toThrow();
});

It has the added benefit compared to rejects that if you forget either the await or the sync, it'll throw errors and not fail silently. Same as if you try to pass a non-promise.

@jcollum

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2018

These big threads like this are such a mess. Is the bug fixed? If it is could someone please provide a final working sample with/without await/async?

I'm mocking node-fetch using a mock in the __mocks__ dir. I have a test that verifies that node-fetch is mocked.

I'm using Promises and this simple bit of code is throwing an error on the new Error

    it('rejects if http call fails', (done) => {
      const msg = 'OMG EXPLOSIONS';
      const error = new Error(msg);
      fetch.mockRejectedValue(error);
      expect(
        getCmsThread({
          msgBody: { collectionGroupId: '1', id: '2', marketplace: 'abc', language: 'en', cmsPath: 'http://test.com' }
        })
      ).rejects.toThrow(msg);
    });

Docs: https://jestjs.io/docs/en/mock-function-api#mockfnmockrejectedvaluevalue

My test fails:

  ● API › getArticle › rejects if http call fails

    OMG EXPLOSIONS

       62 |     it('rejects if http call fails', (done) => {
       63 |       const msg = 'OMG EXPLOSIONS';
     > 64 |       const error = new Error(msg);

It seems that the instant that the new Error is called my test fails.

Am I using it wrong? Sure seems like I'm following the docs. I'll post a full sample if someone can figure out what I'm doing wrong.

"jest": "~23.3.0",

@skbolton

This comment has been minimized.

Copy link

commented Jul 9, 2018

@jcollum try returning in front of your expect

it('rejects if http call fails', () => {
  const error = 'OMG EXPLOSIONS'
  fetch.mockRejectedValue(new Error(msg))

  return expect(
    getArticle({
      msgBody: { 
        collectionGroupId: '1',
        id: '2', 
        marketplace: 'abc',
        language: 'en', 
        cmsPath: 'http://test.com' 
    }
  })
).rejects.toThrow(msg)
})

If you don't return the expect when doing promises it doesn't resolve/ reject it for you

@SimenB

This comment has been minimized.

Copy link
Collaborator

commented Jul 9, 2018

The example in the docs tells you to await or return: https://jestjs.io/docs/en/expect#rejects

@jcollum

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2018

Actually the issue was that the code under test didn't have a .catch -- added that in and it behaved normally.

Complete test:

    it('rejects gracefully if http call fails', (done) => {
      const msg = 'OMG EXPLOSIONS'; 
      fetch.mock.calls.length.should.equal(0);
      fetch.mockRejectedValueOnce(new Error(msg));

      getArticle({
        msgBody: { collectionGroupId: '1', id: '2', marketplace: 'abc', language: 'en', cmsPath: 'http://test.com' }
      })
        .then(() => {
          done(new Error('Then step should not be hit'));
        })
        .catch((err) => {
          fetch.mock.calls.length.should.equal(1);
          err.message.should.equal(msg);
          done();
        });
    });

It would read cleaner with .rejects.toThrow(msg) and without the done. But it works.

@SimenB

This comment has been minimized.

Copy link
Collaborator

commented Jul 10, 2018

As long as you return or await the promise, you don't need done. This has been stated multiple times in this issue.

it('rejects gracefully if http call fails', async () => {
  const msg = 'OMG EXPLOSIONS';
  expect(fetch).toHaveBeenCalledTimes(0);
  fetch.mockRejectedValueOnce(new Error(msg));

  await expect(() =>
    getArticle({
      msgBody: {
        collectionGroupId: '1',
        id: '2',
        marketplace: 'abc',
        language: 'en',
        cmsPath: 'http://test.com'
      }
    })
  ).rejects.toThrow(msg);

  expect(fetch).toHaveBeenCalledTimes(1);
});
@horttanainen

This comment has been minimized.

Copy link

commented Jul 20, 2018

@SimenB You are certainly correct about not needing done if await or return is used, but unfortunately your example is broken.

In Jest 23.0.1:

expect(received).rejects.toThrow()

    received value must be a Promise.
    Received:
      function: [Function anonymous]

On the other hand, this works:

await getArticle({
      msgBody: {
        collectionGroupId: '1',
        id: '2',
        marketplace: 'abc',
        language: 'en',
        cmsPath: 'http://test.com'
      }
    })
    .then(anything => throw new Error("Should not come here."))
    .catch(e => expect(e).toEqual(new Error("OMG EXPLOSIONS")))
@skbolton

This comment has been minimized.

Copy link

commented Jul 21, 2018

@jcollum the catch is needed because you are configuring a test dependency to return a rejected promise which causes your function under test to reject (1). You don't need the done and you can do rejects.toThrow() you just have to return the expectation and assert on it (2). Also controlling what something returns (1) and asserting it was called is probably unnecessary (deleted in my example).

it('rejects gracefully if http call fails', () => {
  const msg = 'OMG EXPLOSIONS';
  fetch.mockRejectedValueOnce(new Error(msg));  // (1)

  const promise = getArticle({
    msgBody: {
      collectionGroupId: '1',
      id: '2',
      marketplace: 'abc', 
      language: 'en', 
      cmsPath: 'http://test.com' 
    }
  });

  return expect(promise).rejects.toThrow(msg); // (2)
});

For the async awaiters around these parts you can achieve the same thing you just don't want the async function of the test to return a rejected promise or else jest will fail the test (1). We want to catch that it would fail and do an assertion that way the test still passes (2). When catching you are handling the error and a non reject promise is returned

it('rejects gracefully if http call fails', async () => {  // (1)
  const msg = 'OMG EXPLOSIONS';
  fetch.mockRejectedValueOnce(new Error(msg));

  await getArticle({
    msgBody: {
      collectionGroupId: '1',
      id: '2',
      marketplace: 'abc', 
      language: 'en', 
      cmsPath: 'http://test.com' 
    }
  })
  .catch(err => expect(err).toEqual(new Error(msg)); // (2)

  // since we caught and handled the error the async function will resolve happily and not
  // fail our test
});
@jcollum

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2018

Thanks for the input.

@philraj

This comment has been minimized.

Copy link

commented Aug 8, 2018

For anyone having trouble parsing the content of this thread, the following is how you do it:

await expect(repo.get(999)).rejects.toThrow(NotFoundException);
@mpareja

This comment has been minimized.

Copy link

commented Sep 20, 2018

Here's how I'm dealing with this in the event that I want to assert on properties of the error:

const promise = systemUnderTest()

await expect(promise).rejects.toEqual(new Error('the expected message'))
await expect(promise).rejects.toMatchObject({
  some: 'expected property value'
})
@johnjensenish

This comment has been minimized.

Copy link

commented Oct 31, 2018

For anyone looking for a simple, working example of the problem posed in the original post:

test('example', async () => {
  async function fn() {
    throw new Error('some error')
  }

  await expect(fn()).rejects.toThrow('some error')
})
@robertpenner

This comment has been minimized.

Copy link

commented Dec 20, 2018

This documentation example is still misleading (as mentioned above in 2017):

await expect(fetchData()).rejects.toMatch('error');

If fetchData() throws an Error, the toMatch fails because it expects a string:

expect(string)[.not].toMatch(expected)
    
string value must be a string.
Received:
  object: [Error: Failed to get message]

The example only works if the function throws a string instead of an Error, which is not a great assumption for the documentation to make.

What works is to replace toMatch with toThrow as mentioned above:

await expect(fn()).rejects.toThrow('some error')
@superhawk610

This comment has been minimized.

Copy link

commented Jun 11, 2019

@robertpenner your final example only tests whether an Error was thrown, but will match regardless of whether that error's message was some error or not:

const fn = () => Promise.reject('expected message')

await expect(fn()).rejects.toThrow('expected message') // matches
await expect(fn()).rejects.toThrow('wrong message') // matches, though it shouldn't

I believe the .toHaveProperty or .toMatchObject approaches are the best for this situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.