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

Odd assertion error message when deep equality failed due to Error.cause #55310

Closed
hi-ogawa opened this issue Oct 8, 2024 · 3 comments · Fixed by #55406
Closed

Odd assertion error message when deep equality failed due to Error.cause #55310

hi-ogawa opened this issue Oct 8, 2024 · 3 comments · Fixed by #55406
Labels
assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs. errors Issues and PRs related to JavaScript errors originated in Node.js core.

Comments

@hi-ogawa
Copy link

hi-ogawa commented Oct 8, 2024

Version

v20.18.0

Platform

Linux myhostname 6.10.10-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 12 Sep 2024 17:21:02 +0000 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

assert.deepStrictEqual(new Error("a", { cause: new Error("x") }), new Error("a", { cause: new Error("y") }))

Uncaught:
AssertionError [ERR_ASSERTION]: Values have same structure but are not reference-equal:

[Error: a]

    at REPL3:1:8
    at ContextifyScript.runInThisContext (node:vm:137:12)
    at REPLServer.defaultEval (node:repl:598:22)
    at bound (node:domain:432:15)
    at REPLServer.runBound [as eval] (node:domain:443:12)
    at REPLServer.onLine (node:repl:927:10)
    at REPLServer.emit (node:events:531:35)
    at REPLServer.emit (node:domain:488:12)
    at [_onLine] [as _onLine] (node:internal/readline/interface:417:12)
    at [_line] [as _line] (node:internal/readline/interface:888:18) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: [Error: a],
  expected: [Error: a],
  operator: 'deepStrictEqual'
}

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

I'm not sure what exactly "reference-equal" means, but it sounds like it's about object identity and that might be misleading/bug.
Also ideally it would be nice if assertion diff can show the diff coming from Error.cause.

What do you see instead?

Assertion error doesn't indicate the error is due to non-matching Error.cause:

Uncaught:
AssertionError [ERR_ASSERTION]: Values have same structure but are not reference-equal:

[Error: a]

Additional information

I saw Node has expanded on Error instance deep equality check #51805 and was testing it around. Vitest is trying to catch up in vitest-dev/vitest#5876 and I thought it would be a good reference to know how Node would handle this.

@RedYetiDev
Copy link
Member

These also aren't reference equal AFAICT. If you deepStrictEqual the errors, they should fail.

@RedYetiDev RedYetiDev added assert Issues and PRs related to the assert subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Oct 8, 2024
@hi-ogawa
Copy link
Author

hi-ogawa commented Oct 8, 2024

Yeah, maybe my description wasn't clear. Assertion failure is expected, but its error message is what I wanted to point out.

Indeed, are not reference-equal is a correct statement, but I felt it's misleading because, when using assert.deepStrictEqual, I wouldn't usually concerned with "reference-equality".

One more example here when dropping cause on one side, it shows the same message. In this case, Values have same structure sounds misleading then.

> assert.deepStrictEqual(new Error("a"), new Error("a", { cause: new Error("y") }))
Uncaught:
AssertionError [ERR_ASSERTION]: Values have same structure but are not reference-equal:

[Error: a]

@BridgeAR
Copy link
Member

BridgeAR commented Oct 12, 2024

@hi-ogawa thank you for the report! This is indeed an issue with comparing errors.

cause is still relatively new and when AssertionError was written in it's current way, errors did not have that property. It's a non-enumerable property and to hide the compared errors stack frames the error is copied in a way that it does not contain the stack trace and cause anymore. Afterwards it's compared regularly. This now also hides the cause and that causes the faulty above error message.

This could be fixed by removing the copying and instead cutting off the stack frames after inspection but before they are compared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs. errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants