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

asJson() method doesn't return a valid JSON if mergingObject key contains a double quote #1767

Closed
OlenaHryshko opened this issue Aug 3, 2023 · 6 comments · Fixed by #1779
Labels

Comments

@OlenaHryshko
Copy link

OlenaHryshko commented Aug 3, 2023

In case key(s) of the mergingObject contains " symbol, it is not getting shielded. So, logged object is not a valid JSON string.

See code example:

const pino = require('pino');
const logger = pino();

logger.info({'some"prop': {'details': true}});

This will result in the log below:
{"level":30,"time":1690897379299,"pid":41644,"hostname":"your-hostname","some"prop":{"details":true}}

I'm using newrelic to collect logs and it relies on asJson() method. Due to an invalid JSON is returned, JSON.parse throws an error.
Please see details here newrelic/node-newrelic#1744.

Please confirm whether this is an expected log format or it should be fixed.
From methods name asJson() I'd expect it to return a valid JSON string.

@mcollina
Copy link
Member

mcollina commented Aug 4, 2023

Looks like a bug that needs fixing! Good spot!

@OlenaHryshko
Copy link
Author

Hi @mcollina
Thanks for the confirmation. What are further actions on this issue? Will it be fixed by pino team?

@jsumners
Copy link
Member

jsumners commented Aug 7, 2023

Given that none of the maintainers have ever encountered this issue, it's unlikely to be on our priority list. We would be eager to review any PR that solves it, though.

@tzviki
Copy link
Contributor

tzviki commented Aug 8, 2023

I create this pull request.
I fully understand if this should be changed as a feature, or option with override stringifier for keys. Happy to discuss further!

@tzviki
Copy link
Contributor

tzviki commented Aug 30, 2023

I added the issue now (updated my comment on pr) since I realized this issue can probably be closed. Sorry didn't initially include it

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants