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

Feature - optionally include Error.cause property #226

Merged
merged 2 commits into from
Nov 24, 2022
Merged

Feature - optionally include Error.cause property #226

merged 2 commits into from
Nov 24, 2022

Conversation

davidnbooth
Copy link

@davidnbooth davidnbooth commented Nov 11, 2022

This would extend the options (currently {stack: Boolean}) to include a "cause" option. Passing cause: true would make the emitted info include the Error.cause property and allow the fullest error logging for users who are using error causes.

I wasn't sure if the conditionals should be if (cause) or if (cause && einfo.cause). I'd be very happy with either.

For background, Error.cause is an ES2022 addition, added to node in 16.9.0.

Extends options to allow emitted info to include the ES2022 Error.cause
@wbt
Copy link
Contributor

wbt commented Nov 15, 2022

Looks pretty good to me! Will leave it open for a bit in case others have comments.
Also, do you want to add tests to prevent future regressions?

@davidnbooth
Copy link
Author

Thanks for the feedback! I added some tests. I wasn't 100% sure what I was doing but I think they are right.

@davidnbooth
Copy link
Author

Is there anything else or is it good to go?

@wbt wbt merged commit fdc37d1 into winstonjs:master Nov 24, 2022
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 this pull request may close these issues.

3 participants