`assert.throws()` with async argument

[JosephusPaye]

Hi 👋,

Is assert.throws() currently supposed to work with async functions?

For example, this fails:

assert.throws(async () => {
    await asyncFnThatThrows();
});

While this passes:

assert.throws(() => {
    syncFnThatThrows();
});

What is the recommended way to test that an async function throws?

Thought of something like this, but there's no .pass() or .fail():

try {
   await asyncFnThatThrows();
   test.fail('did not throw');
} catch (err) {
   test.pass();
}

Edit: currently using this hack:

async function throwsAsync(
    asyncFn,
    failMessage = 'did not throw',
    passMesage = 'did throw',
) {
    try {
        await asyncFn();
        assert.equal(false, true, failMessage);
    } catch (err) {
        assert.equal(true, true, passMesage);
    }
}

[lukeed]

We could change it so that promises are supported. But it might be odd that assert.throws becomes the only one that can be awaited.

In the meantime this is what I do:

try {
  await asyncFnThatThrows();
  assert.unreachable('should have thrown');
} catch (err) {
  assert.ok('threw error');
  assert.instance(err, Error);
  // ...
}

[JosephusPaye]

Since it's the only assertion that takes a function to execute (along with .not.throws()), perhaps it makes sense to have it take a promise and be await-able.

Your suggested workaround is good for now though, and I've adopted it.

[lukeed]

Hey,

I've played with this throughout the week. TBH I'm not liking this in practice. In order for assert.throws and assert.not.throws to be reliable (and consistent), the entire assertion method has to be async, which means that every assert.(not.)throws usage has to be awaited, even if the supplied function is synchronous. And so it feels very very silly to have to use async/await anytime you want to check if an error is thrown.

It would also be unique in that no other assert.throws checks (including native) handles async functions.

The try/catch + assert.unreachable pattern looks, and feels, much nicer IMO. Closing in favor of that. Hopefully you've come to like that approach too :)

Thanks~!

[JosephusPaye]

Thanks for the consideration. Your findings make sense, and I haven't seen any issues in practice using the try/catch + assert.unreachable pattern myself.

[hgl]

I personally find the boilerplate a tad heavy for try/catch, what do you think of this API design:

await assert.rejected.instance(asyncFnThatThrows(), Error)

[aral]

@lukeed Unless I’m missing something, shouldn’t your example read:

try {
  await asyncFnThatThrows();
  assert.unreachable('should have thrown');
} catch (err) {
  if (err instanceof assert.Assertion) {
    throw err;
  }
  assert.instance(err, Error);
  // ...
}

i.e, insert of asserting ok, we should rethrow in the catch block if the error is the assertion failure from the try block. (This is what uvu does internally and also how I got it to work so I was just wondering if I’m missing something and, if not, to document it for anyone else reading the issue as it confused me initially.)

PS. Thank you for uvu and polka, sade, etc. I’m starting to get the feeling you’re writing the new small web server I’m working on for me for the most part ;)

[lukeed]

Nope, no need to re-throw. My snippet is exactly what I do in about 50 different projects.

I mean, if all you're doing is asserting that err is an Error, then ya you should re-throw, but you should always actually be asserting the details of your error (stack, message, code) to make sure it's what you wanted.

PS no problem :)

[aral]

@lukeed I’m confused: so without the assert in the try block, if asyncFnThatThrows() doesn’t throw due to a regression, the assert.ok('threw error') test will pass, no? (And we want it to fail because asyncFnThatThrows() didn’t throw an error, we did with the assert.unreachable().

It’s possible that I am missing something and/or that I need to step away from the screen and come back to this later :)

[lukeed]

@aral I'm saying that if you only have this:

try {
  await asyncFnThatThrows();
  assert.unreachable('should have thrown');
} catch (err) {
  assert.instance(err, Error);
}

then yes, this could lead to misleading results since the Assertion that uvu/assert throws extends Error, thus satisfying the assert.instance check.

I'm saying that this snippet shouldn't stand on its own in most cases. Instead, the recommendation is to always assert against properties within the err that gets thrown. So something more like this:

try {
  await asyncFnThatThrows();
  assert.unreachable('should have thrown');
} catch (err) {
  assert.instance(err, Error);
  assert.match(err.message, 'something specific');
  assert.is(err.code, 'ERROR123');
}

In this case, if/when assert.unreachable does throw, it doesnt match the other conditions. And you still get the benefit of ensuring that asyncFnThatThrows function needs to throw.

---

I see your PR, thanks! Will review shortly.