The "cause" property of Error is ignored when matching

[kettanaito]

Describe the bug

Vitest ignores the cause property whenever performing any equality on Error instances.

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-fq9xyu?file=test%2Fbasic.test.ts&initialPath=__vitest__/

System Info

Irrelevant. The issue is in the matcher.

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[sheremet-va]

This is documented behavior: https://vitest.dev/api/expect.html#toequal

A deep equality will not be performed for Error objects. Only the message property of an Error is considered for equality. To customize equality to check properties other than message, use expect.addEqualityTesters. To test if something was thrown, use toThrowError assertion.

[kettanaito]

Hi, @sheremet-va. Got it. Can you give me an example of using .toThrowError() to also assert the cause property, please? I find it to be a common use case, I wouldn't want to add a custom tester/matcher for it.

[kettanaito]

I don't believe .toThrowError() allows me to achieve the goal here. I highly suggest Vitest adds an API to help test more complex errors easier.

Perhaps something like this?

const error = vi.throws(fn)
expect(error).toEqual({ message: '', cause: '' })

[kettanaito]

@sheremet-va, also, any chance to revisit this?

Only the message property of an Error is considered for equality.

I assume message is checked because every error can have a message. The same way, every error can have a cause property.

[sheremet-va]

Technically you can use chai API for this:

expect(() => {
  throw new Error('test', { cause: new Error('cause') });
})
  .throws()
  .property('cause')
  .eql(new Error('cause'))

I am not sure what the best course of action is here, I also don't really like that it only checks the message. Will put it on the board to discuss with the team. If anyone has any ideas here, feel free to suggest here.

Maybe adding toThrowStrictError matcher? 🤔 (Ignoring stack is still probably a good idea 🤔 )

[kettanaito]

I think there are a few possible directions to this:

1. Add cause as a special case alongside message, which equality will always be checked.
2. Add .toThrowStrictError(), although why then differentiate between this and .toThrowError? To keep the previous behavior intact? Is the intention behind .toThrowStrictError() to strictly compare all error properties?
3. Make catching errors easier via Vitest (e.g. vi.throws()). That won't count as an assertion but really a simpler way to do try/catch with, maybe, a built-in error thrown if the given function doesn't throw.

Technically you can use chai API for this:

That's a good call! I will remember this for myself but it won't work for what I'm preparing for my students right now.

Not checking arbitrary properties is okay. But cause is making its way into our code, and it's a defined property with a clear purpose (and also value type). I'd expect this to work:

expect(fn).toThrow(new Error('message', { cause: 123 }))

Need to be careful since cause is a potentially deep property. It can reference an error which has its own cause, which points to an error, which... I can recommend taking only the cause the user explicitly added in the matcher and ignoring any nested cause.

Alternatively, if this worked it'd also be great:

// Return me the error thrown by "fn"
// or throw if "fn" doesn't throw. 
// Returning null is also fine, depends
// if we want to treat this as implicit assertion. 
const error = vi.throws(fn)
expect(error.message).toBe('message')
expect(error.cause).toBe('cause')

[sheremet-va]

I see your point. It would be nice to check cause since it's in the standard now. I believe we should also check the cause's cause and so on if it wasn't validated yet (hello, recursion).

Not sure about vi.throws - we have a few non-testing utilities already, so there is nothing stopping us from adding it, but I would prefer having an explicit assertion.

[kettanaito]

I believe we should also check the cause's cause and so on if it wasn't validated yet

Only if explicitly specified in the assertion 👍

const error = new Error('message', {
  cause: new Error('another', {
    cause: 123
  })
})

expect(error).toEqual(new Error('message')) // OK!
expect(error).toEqual(new Error('message', { cause: new Error('another') })) // OK!
expect(error).toEqual(new Error('message', { cause: new Error('another', { cause: 123 }) })) // OK!
expect(error).toEqual(new Error('message', { cause: new Error('another', { cause: 'bad' }) })) // X

expect(error).toStrictEqual(new Error('message')) // X
expect(error).toStrictEqual(new Error('message', { cause: new Error('another') })) // X
expect(error).toStrictEqual(new Error('message', { cause: new Error('another', { cause: 'bad' }) })) // X
expect(error).toStrictEqual(new Error('message', { cause: new Error('another', { cause: 123 }) })) // OK!

[hi-ogawa]

Something similar is brought up here

• toEqual/toStrictEqual does not check custom Error properties #5244

Comparing only Error.message is probably a historical reason and we should probably revisit this. In the issue, I was mentioning how node:assert's deep equality does #5244 (comment) since they treat Error differently.

It turns out Node doesn't check Error.cause since it's not enumerable. They only treat Error.name and Error.message as a special non-enumerable property to check.
https://stackblitz.com/edit/vitest-dev-vitest-ma1ej3?file=test%2Fbasic.test.ts

import assert from "node:assert";

// ok
assert.deepStrictEqual(
  new Error('x', { cause: 'foo' }),
  new Error('x', { cause: 'bar' })
);

This is not stopping Vitest from checking Error.cause but I thought it's worth mentioning.