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 error on "t.throws()" test of trivial async function #1371

Closed
sdd opened this issue Apr 27, 2017 · 9 comments
Closed

Unhandled rejection error on "t.throws()" test of trivial async function #1371

sdd opened this issue Apr 27, 2017 · 9 comments

Comments

@sdd
Copy link

sdd commented Apr 27, 2017

Description

Unhandled rejection error on test of async function that throws

Test Source

import test from 'ava';

const f = async () => {
    throw new Error();
};

test(async t => {

    const error = await t.throws(async () => {
        return await f();
    });

    t.is(error.message, '');
});

Error Message & Stack Trace

1 failed [08:28:15]
  1 rejection

  test › [anonymous]
  /Users/scotty/workspace/fetch-nice/test/test.js:9

   8:
   9:     const error = await t.throws(async () => {
   10:         return await f();

  Test.<anonymous> (test/test.js:9:27)
  step (test/test.js:9:191)
  Test.<anonymous> (test/test.js:9:99)
  Test.__dirname [as fn] (test/test.js:7:1)

  Unhandled Rejection
  Error
    test/test.js:4:11
    Generator.next (<anonymous>)
    step (test/test.js:9:191)
    test/test.js:9:437
    test/test.js:9:99
    f (test/test.js:3:7)
    test/test.js:10:22
    Generator.next (<anonymous>)
    step (test/test.js:9:191)
    test/test.js:9:437
    test/test.js:9:99
    _tryBlock (node_modules/core-assert/index.js:311:5)
    _throws (node_modules/core-assert/index.js:330:12)
    Function.assert.throws (node_modules/core-assert/index.js:360:3)
    Test.<anonymous> (test/test.js:9:27)
    Generator.next (<anonymous>)
    step (test/test.js:9:191)
    test/test.js:9:437
    Test.<anonymous> (test/test.js:9:99)
    Test.__dirname [as fn] (test/test.js:7:1)

Config

{
  "ava": {
  "require": [
      "babel-register",
      "babel-polyfill"
    ]
  }
}

Command-Line Arguments

ava -w

Environment

Node.js v7.6.0
darwin 15.6.0
ava 0.19.1
npm 4.1.2

@sindresorhus
Copy link
Member

Simpler test case:

import test from 'ava';

test(async t => {
	await t.throws(async () => {
		throw new Error();
	});
});

@sindresorhus sindresorhus added the bug current functionality does not work as desired label Apr 27, 2017
@sdd sdd changed the title async throwing function unhandled rejection Unhandled rejection error on "t.throws()" test of trivial async function Apr 27, 2017
@novemberborn
Copy link
Member

This is correct behavior: the function doesn't throw, so the assertion fails. AVA ignores the return value, so the promise rejection ends up being unhandled and is reported separately.

That said I think we should extend t.throws() and t.notThrows() so that if the function returns a promise without throwing, the assertion is applied to that promise instead.

@sdd
Copy link
Author

sdd commented Apr 27, 2017

Is this not effectively the same as the following example in the t.throws() docs here?

const promise = Promise.reject(new TypeError('🦄'));

test('rejects', async t => {
	const error = await t.throws(promise);
	t.is(error.message, '🦄');
});

@sindresorhus
Copy link
Member

@sdd No it's the same as:

import test from 'ava';

const promise = () => Promise.reject(new TypeError('🦄'));

test('rejects', async t => {
	const error = await t.throws(promise);
	t.is(error.message, '🦄');
});

Note the arrow function.

@sindresorhus
Copy link
Member

That said I think we should extend t.throws() and t.notThrows() so that if the function returns a promise without throwing, the assertion is applied to that promise instead.

👍

@sindresorhus sindresorhus added enhancement new functionality and removed bug current functionality does not work as desired labels Apr 27, 2017
@novemberborn novemberborn added this to the 1.0 milestone Oct 25, 2017
@justinhelmer
Copy link

A valid use-case for this design pattern in case you are on the fence:

const someTruthy = true;

const someAsyncThing = await () => {
  if (someTruthy) {
    throw new TypeError('🦄');
  }

  await asyncStuff();
  await moreAsyncStuff();
}

// currently fails with unhandled rejection
test('rejects', async t => {
  await t.throws(someAsyncThing, '🦄');
});

IMO the above design pattern is much nicer than:

// passes
test('rejects', async t => {
  await someAsyncThing()
    .catch(err => {
      t.is(err.message, '🦄');
    });
});

Also, it took me a while to discover this is actually functioning as designed, since it seems very similar to the use-case documented here: https://github.com/avajs/ava/#throwsfunctionpromise-error-message

@novemberborn
Copy link
Member

A valid use-case for this design pattern in case you are on the fence:

We're not on the fence, see the issue labels 😉 Help most wanted!

@hildjj
Copy link

hildjj commented Jan 22, 2018

Agree that this would be a lovely feature. As a note to my future self (since I keep making this mistake), a work-around is to use an IIFE to turn the promise-returning function into a promise:

test(async t => {
	await t.throws((async () => {
		throw new Error();
	})());
});

oantoro added a commit to oantoro/ava that referenced this issue Jan 23, 2018
oantoro added a commit to oantoro/ava that referenced this issue Jan 24, 2018
oantoro added a commit to oantoro/ava that referenced this issue Jan 24, 2018
oantoro added a commit to oantoro/ava that referenced this issue Jan 24, 2018
oantoro added a commit to oantoro/ava that referenced this issue Jan 26, 2018
oantoro added a commit to oantoro/ava that referenced this issue Jan 27, 2018
oantoro added a commit to oantoro/ava that referenced this issue Jan 28, 2018
oantoro added a commit to oantoro/ava that referenced this issue Jan 28, 2018
@novemberborn
Copy link
Member

This was fixed in #1650.

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

No branches or pull requests

5 participants