Skip to content

deepStrictEqual and notDeepStrictEqual do not compare Error causes nor AggregateError errors arrays #51793

Closed
#51805
Closed
deepStrictEqual and notDeepStrictEqual do not compare Error causes nor AggregateError errors arrays#51793
#51805
Labels
assertIssues and PRs related to the assert subsystem.feature requestIssues requesting new Node.js features.utilIssues and PRs related to the built-in util module.

Description

@Zamralik
Zamralik
opened on Feb 17, 2024

Version

v20.9.0

Platform

Linux DESKTOP-8H17SD6 5.15.133.1-microsoft-standard-WSL2 #1 SMP Thu Oct 5 21:02:42 UTC 2023 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

  • Compare 2 Error one with a cause and one without
  • Compare 2 Error one with different causes
  • Compare 2 AggregateError with different array of errors

These 4 calls should not throw.
Inversely, replacing notDeepStrictEqual by deepStrictEqual should throw for all.

import { notDeepStrictEqual } from "node:assert";

notDeepStrictEqual(
    new Error('Test'),
    new Error('Test', { cause: new Error('Expected Cause') }),
);

notDeepStrictEqual(
    new Error('Test', { cause: new Error('Unexpected Cause') }),
    new Error('Test'),
);

notDeepStrictEqual(
    new Error('Test', { cause: new Error('Unexpected Cause') }),
    new Error('Test', { cause: new Error('Expected Cause') }),
);

notDeepStrictEqual(
    new AggregateError([], 'Aggregate Test'),
    new AggregateError([new Error('Expected Child')], 'Aggregate Test'),
);

notDeepStrictEqual(
    new AggregateError([new Error('Unexpected Child')], 'Aggregate Test'),
    new AggregateError([], 'Aggregate Test'),
);

notDeepStrictEqual(
    new AggregateError([new Error('Unexpected Child')], 'Aggregate Test'),
    new AggregateError([new Error('Expected Child')], 'Aggregate Test'),
);

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

Every time

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

notDeepStrictEqual should throw when the Error's cause / AggregateError's errors match perfectly recursively.
deepStrictEqual should throw when the Error's cause / AggregateError's errors do not match perfectly recursively.

What do you see instead?

notDeepStrictEqual throw when the Error's cause / AggregateError's errors match perfectly recursively.
deepStrictEqual do not throw when the Error's cause / AggregateError's errors do not match perfectly recursively.

Additional information

No response

Activity

  1. kylo5aby commented on Feb 18, 2024

    @kylo5aby
    kylo5aby
    on Feb 18, 2024
    Contributor

    As mentioned in deepstrictequalactual, for Error type, it compares names, messages and enumerable "own" properties.

    cause, message, and name, are both non-enumerable properties. However, as mentioned in ECMA2024, cause triggers some special operations during the creation of an Error instance, so I believe cause serves as another distinguishing attribute among different Error, much like message and name. I'm unsure if others agree with this perspective, and I'm willing to submit a PR if feasible.

  2. Zamralik commented on Feb 18, 2024

    @Zamralik
    Zamralik
    on Feb 18, 2024
    Author

    I use AggregateError and Error's cause to create a tree to provide an explicit trail of what was wrong and where.

    Being able to recursively compare the Error's cause/Aggregate's errors in the tests is more thorough that no info on the reason is missing.

  3. BridgeAR commented on Feb 19, 2024

    @BridgeAR
    BridgeAR
    on Feb 19, 2024
    Member

    I agree with @zhenweijin that we should just add a special handling for it. It should be pretty straight forward to add that check.

  4. BridgeAR
    added
    good first issueIssues that are suitable for first-time contributors.
    assertIssues and PRs related to the assert subsystem.
    feature requestIssues requesting new Node.js features.
    on Feb 19, 2024
  5. github-project-automation
    added this to Node.js feature requestson Feb 19, 2024
  6. github-project-automation
    moved this to Pending Triage in Node.js feature requestson Feb 19, 2024
  7. BridgeAR
    added
    utilIssues and PRs related to the built-in util module.
    on Feb 19, 2024

  8. abidjappie commented on Feb 19, 2024

    @abidjappie
    abidjappie
    on Feb 19, 2024

    I'm interested in looking at this issue if it become available 🙇 (I see it's still in Triage)

  9. Elham-EN commented on Feb 21, 2024

    @Elham-EN
    Elham-EN
    on Feb 21, 2024

    Hi, can you assign me to this issue?

  10. BridgeAR
    removed
    good first issueIssues that are suitable for first-time contributors.
    on Feb 21, 2024

  11. BridgeAR commented on Feb 21, 2024

    @BridgeAR
    BridgeAR
    on Feb 21, 2024
    Member

    There's an open PR, so it's almost done.

  12. nodejs-github-bot
    closed this as completedin #51805on May 12, 2024
  13. nodejs-github-bot
    added a commit that references this issue on May 12, 2024

    assert: add deep equal check for more Error type

    c8a4f70
  14. targos
    added a commit that references this issue on May 13, 2024

    assert: add deep equal check for more Error type

    0f5716c
  15. eliphazbouye
    added a commit that references this issue on Jun 20, 2024

    assert: add deep equal check for more Error type

    a064469
  16. avivkeller
    moved this from Awaiting Triage to Done in Node.js feature requestson Jun 28, 2024
  17. avivkeller
    removed this from Node.js feature requestson Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    assertIssues and PRs related to the assert subsystem.feature requestIssues requesting new Node.js features.utilIssues and PRs related to the built-in util module.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions