Skip to content

fix(logger): handle illegal null/undefined as extra args #3614

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

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

dreamorosi
Copy link
Contributor

Summary

Changes

Please provide a summary of what's being changed

This PR adds an extra runtime check to values passed as extra arguments to a Logger logging method (i.e. logger.info('message', extra);). With the fix added to this PR we are now skipping these values when they are null or undefined rather than trying to process them, which addresses the bug described in the linked issue.

For the sake of completeness, TypeScript already forbids passing these types to the logger methods, so this case could happen only if:

  • customers are using JavaScript with no type hints in their IDE
  • customers are passing a value of type any
  • customers are explicitly turning off type checking for the file / line (i.e. // @ts-expect-error or // @ts-ignore)
  • customers are manually type casting the value to make TS happy

Nevertheless, given the above it's still worth adding a runtime check in this case - especially because this case was handled gracefully in previous versions of Logger.

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: fixes #3613


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi self-assigned this Feb 18, 2025
@dreamorosi dreamorosi requested a review from a team February 18, 2025 10:13
@dreamorosi dreamorosi requested a review from a team as a code owner February 18, 2025 10:13
@boring-cyborg boring-cyborg bot added logger This item relates to the Logger Utility tests PRs that add or change tests labels Feb 18, 2025
@pull-request-size pull-request-size bot added the size/M PR between 30-99 LOC label Feb 18, 2025
@dreamorosi dreamorosi changed the title fix(logger): handle illegal null/undefined as extra args fix(logger): handle illegal null/undefined as extra args Feb 18, 2025
@github-actions github-actions bot added the bug Something isn't working label Feb 18, 2025
@pull-request-size pull-request-size bot added size/S PR between 10-29 LOC and removed size/M PR between 30-99 LOC labels Feb 18, 2025
@dreamorosi dreamorosi requested a review from am29d February 18, 2025 10:25
@am29d am29d merged commit 6f99073 into main Feb 18, 2025
44 checks passed
@am29d am29d deleted the fix/logger_handle_null branch February 18, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logger This item relates to the Logger Utility size/S PR between 10-29 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: regression when passing null as extra arg in logging methods
2 participants