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

Improved trace method #336

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Poseclop
Copy link

@Poseclop Poseclop commented Jun 21, 2024

I feel the trace method should keep it's original intent to log the current stack trace (same as node.js defautl console.trace)

  • Looks like some time ago the default behavior of LoggerService.trace was to call console.trace but this was removed as the stack trace shown was the one of console.trace.
  • This PR aims to add back the stack trace, putting it on a higher level so that only 1 extra line is present in the stack trace (LoggerService.trace)
  • If somehow the stack trace is not available, the method will revert to its previous behavior and only log the message and additional params
  • I did not manage to create a unit test that would succefully check the exact stack trace added in _log method so I'm only making sure the message sent to the method is present in the stack trace. Maybe that can be improved
  • Watchout, this change will obviously drastically change the output in console if users where using a lot of LoggerService.trace calls.

image

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.

1 participant