assert: handle class constructors in throws()

[cjihrig]

Currently, if a class constructor is passed as the expected value to throws(), and a match is not received, the code falls through to the validation function case, which uses fn.prototype.call(). Class constructors must be called with new, so this throws a TypeError. This commit adds a try...catch, which handles the thrown error.

I'm proposing this as an alternative to #4166. The drawback I see with #4166 is that it only handles classes which extend Error. In general, it might be a good thing to wrap user defined validation functions in a try...catch anyway. Closes #3188.

[Trott]

Does this mean that assert.throws() will now ignore all errors thrown by the validation function? (And are there semver implications to such a change? Wouldn't we want things like ReferenceError to bubble up to the user?)

[bnoordhuis]

I agree, this seems dangerous. You could rethrow unless e instanceof TypeError && /Class constructors cannot be invoked without 'new'/.test(e.message) but that changes the throw site and can still swallow genuine TypeErrors from the expected callback.

[cjihrig]

There are definitely semver implications. I don't have strong feelings about this PR. To be honest, I'd rather close it than special case and rethrow errors.

[Trott]

For what it's worth, and likely not telling anyone anything they don't already know, the "proper" fix is probably something like:

• Unlock API
• Change function signature to add an options object and the user can send a validation option and/or a constructor option for an object that the thrown error must match
• Relock the API

That could be done in a semver-minor way if we leave the legacy behavior intact, bug and all.

I'm not necessarily recommending that route--the API was locked for a reason--but just saying: If we want to fix it "right", the solution probably looks something like that...

[mscdex]

CI is all green except for some flaky, unrelated Windows tests.