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

Consider making common error properties non enumerable #29

Closed
Weakky opened this issue May 6, 2020 · 1 comment · Fixed by #30
Closed

Consider making common error properties non enumerable #29

Weakky opened this issue May 6, 2020 · 1 comment · Fixed by #30

Comments

@Weakky
Copy link
Contributor

Weakky commented May 6, 2020

Description

Native JS errors don't have its properties enumerable. What this means is that the name, stack and message properties do not appear when console.logging errors.

Notice the name and message property appearing below.

image

Proposal

To get deserialized errors closer to the native ones, when deserializing the error, we could use Object.defineProperty instead of a direct assignment to make the properties not enumerable. eg: here https://github.com/sindresorhus/serialize-error/blob/master/index.js#L54

for (const property of commonProperties) {
	if (typeof from[property] === 'string') {
        Object.defineProperty(to, property, {
          value: from[property]
        })
	}
}

I'm happy to make a PR if you agree with the proposal!

Thanks for your awesome work across the Node community ❤️

@sindresorhus
Copy link
Owner

Yes, definitely makes sense. The current behavior was just an oversight on my part. A PR would be great :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants