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 #2447

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

lexctk
Copy link
Contributor

@lexctk lexctk commented Apr 9, 2024

This fixes #2446 by adding optional cause property

@chriskuech
Copy link

@DABH this PR seems simple and would help a lot--could you please take a look?

@DABH
Copy link
Contributor

DABH commented Apr 29, 2024

Can we add a test or modify a test to maintain coverage? 🙏

@lexctk
Copy link
Contributor Author

lexctk commented May 30, 2024

Can we add a test or modify a test to maintain coverage? 🙏

took me way longer than expected to get back to this. I added a test case, please let me know if ok, I'll make any necessary changes

@p-herbert
Copy link

Any updates for this feature?

@chriskuech
Copy link

@p-herbert I was able to circumvent the issue with a custom formatter

import winston from 'winston'

export const serialize = (error: Error, i = 1): string =>
  `${i}. ${error.stack}\n${'cause' in error ? serialize(error.cause as Error, i + 1) : ''}`

const formatError = winston.format((info) => {
  if (info instanceof Error) {
    return {
      ...info,
      message: serialize(info),
    }
  }

  return info
})

export const logger = winston.createLogger({
  levels: winston.config.npm.levels,
  format: winston.format.combine(winston.format.timestamp(), formatError()),
  transports: [
    new winston.transports.Console({
      format: winston.format.combine(
        winston.format.colorize(),
        winston.format.simple(),
      ),
    }),
  ],
})

@lexctk lexctk requested a review from chriskuech July 30, 2024 16:39
@HenriqueSilverio
Copy link

This would be very useful.

Copy link
Contributor

@DABH DABH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this was fixed in logform but not in winston, I believe it's valid to go ahead and merge this. It feels like in general (unless there's some unlucky property name) this should not be a breaking change, and in any case, would be in the spirit of how we're trying to handle Errors, but addressing this new property added in es2022.

Thanks for your contribution!

@DABH DABH merged commit 201b6f1 into winstonjs:master Nov 1, 2024
4 checks passed
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.

[Bug]: Error.cause not logged correctly
5 participants