Make exceptions/promise rejections testable with throw() even if they aren't instances of Error

[sth]

In #1704 support for non-Error exceptions was removed from throws(). Since promise rejections are also tested with throws(), this also removed support for promise rejections that are not instances of Error. While non-Error exceptions might be rare, rejecting promises with non-Error values doesn't seem too uncommon.

throws() is now strictly less useful than before. It can still test cases that were previously tested with throws(x, Error), but for other cases it will always fail. Code that throws non-standard exceptions or uses such promises always fails the assertion.

The fix in #1704 is based on issue #1440 and a motivating example seems to be #1439, where somebody explicitly specified that an Error instance should be thrown and then got confused that a thrown string didn't match. This test have easily been fixed by removing the Error (at least I assume so).

With the new throw() this test still fails with a rather cryptic error message (Function threw exception that is not an error: "...") and now can't be easily fixed. The situation is not really improved, the user needs to implement their own throw() or refactor their library to work with the testing framework. Usually you would want it the other way around, having the tests first to support the refactoring.

I don't think there is really a good reason to hard fail in non-Error values. To solve #1439 a clearer error message would be enough. It should be no problem to explicitly tell the user that the exception/rejection didn't have the specified type.

Additionally from the name one would expect throws() to just check for any thrown exceptions, not to fail when a wrong value is thrown.

[novemberborn]

I agree with you, but the rest of @avajs/core was leaning the other way. We've since cleaned up the assertion though, so perhaps we can reconsider.

Specifically, we could only support const err = t.throws(fn) and t.throws(fn, {is: err}) for non-Error values. E.g. t.throws(fn, {instanceOf: MyNonErrorClass}) wouldn't be allowed.

[sindresorhus]

I'm still against this. While it's possible to reject with a non-error, it's not a good practice. Supporting this would mean a worse experience for the common-case. It would complicate the TS/Flow type definitions, where we would like them to return an Error. And it would be inconsistent as it would not work with t.throws(fn, {instanceOf: MyNonErrorClass}), so we would have to document where it doesn't work.

I'm happy to be convinced otherwise with use-cases and examples.

For reference, this has been in AVA for half a year and you're the first one to complain, so it can't be such a big issue.

[novemberborn]

(I'm closing this issue for housekeeping purposes, but let's keep the conversation going.)

[gwn]

Hello, today is the first day I'm trying AVA and I just wanted to say that I support this issue.

Since promise rejections are also tested with throws(), this also removed support for promise rejections that are not instances of Error. While non-Error exceptions might be rare, rejecting promises with non-Error values doesn't seem too uncommon.

This makes a lot sense. I don't think an AVA user should be tasked to fix all the dependencies that don't throw error instances. That is unrealistic. Sounds like a joke even.

The requirement for the user's and all their dependencies' codes to comply with "best practices" is extremely frustrating. I didn't think that library authors had the luxury to make that kind of assumptions.

I understand that this helps with the internals and implementation; but I don't care about those as a user.

I know I can always always write a custom assertion or depend on a custom assertion library, but it is not the point. This approach bothers me so much that I'm now checking out alternative test libraries at the moment.

Don't get me wrong, I know about open source and that nobody owes me anything, and I appreciate the great work done here. But I just wanted to share my perspective as an experienced hacker playing with AVA the first time.

For reference, this has been in AVA for half a year and you're the first one to complain, so it can't be such a big issue.

This is understandable though. I wouldn't consider "fixing" this for 2 or 3 people as well. But It makes me wonder why don't more people complain about this. Very interesting. This is the first time I'm using this and this was an obstacle for me immediately. I can even say that it ruined my evening :) I wanted to proceed with this library so much.

Anyway.. I wish the best to this project!

[gwn]

Note: I mistakenly posted a draft of my comment the first time, and I've since edited it.

[novemberborn]

I think we can allow this by making it opt-in, if you specify any: true in the expectations. See #2517.

[gwn]

Seems like a fine escape hatch. Thank you for showing interest. I'll stay tuned.