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

deepEqual(new Error(), new Error()) behavior changed in node 8 #213

Closed
dominykas opened this issue Sep 26, 2017 · 2 comments
Closed

deepEqual(new Error(), new Error()) behavior changed in node 8 #213

dominykas opened this issue Sep 26, 2017 · 2 comments
Assignees
Labels
bug Bug or defect non issue Issue is not a problem or requires changes

Comments

@dominykas
Copy link

dominykas commented Sep 26, 2017

Ran into this at some point recently as part of asserting with code - it's easy enough to avoid, but could still be useful to clarify which behavior is correct:

✔ ~/devel/projects/tmp 
15:08 $ nvm use 6
Now using node v6.11.2 (npm v5.4.2)
✔ ~/devel/projects/tmp 
15:08 $ node
> require('hoek').deepEqual(new Error(), new Error())
true

vs

✔ ~/devel/projects/tmp 
15:08 $ nvm use 8
Now using node v8.5.0 (npm v5.4.2)
✔ ~/devel/projects/tmp 
15:09 $ node
> require('hoek').deepEqual(new Error(), new Error())
false

It seems that in node 8 property descriptor for error.stack no longer has a get function, but rather has a value - which forces a different path - and the stack is different for the two errors, so possibly the previous behavior was incorrect?

@hueniverse
Copy link
Contributor

I think the new behavior is correct... these two errors are not the same since they are thrown from different places. If there is a valid use case for comparing error properties other than stack we could add an option to ignore some properties. But I think the old behavior was wrong.

@hueniverse hueniverse added the non issue Issue is not a problem or requires changes label Nov 3, 2017
@hueniverse hueniverse self-assigned this Nov 3, 2017
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect non issue Issue is not a problem or requires changes
Projects
None yet
Development

No branches or pull requests

2 participants