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

Unhandled error during JSON.parse #1744

Closed
OlenaHryshko opened this issue Aug 1, 2023 · 5 comments · Fixed by #1750
Closed

Unhandled error during JSON.parse #1744

OlenaHryshko opened this issue Aug 1, 2023 · 5 comments · Fixed by #1750
Assignees
Labels

Comments

@OlenaHryshko
Copy link

OlenaHryshko commented Aug 1, 2023

Description

Using newrelic to capture application logs + pino as a logger agent.
While logging using pino, it is possible to pass mergingObject to add more context to the log. In case key(s) of the mergingObject contains " symbol, it is not getting shielded. So, logged object is not a JSON string, but newrelic lib tries to parse it to a JSON object.

NewRelic throws an error below:

SyntaxError:   Unexpected token s in JSON at position 219
at JSON.parse (<anonymous>)
at formatLogLine   (/app/node_modules/newrelic/lib/instrumentation/pino/pino.js:128:31)
at /app/node_modules/newrelic/lib/aggregators/log-aggregator.js:46:16
at Array.map (<anonymous>)
at LogAggregator._toPayloadSync   (/app/node_modules/newrelic/lib/aggregators/log-aggregator.js:44:32)
at LogAggregator.send   (/app/node_modules/newrelic/lib/aggregators/base-aggregator.js:105:32)
at /app/node_modules/newrelic/lib/agent.js:396:18
at new Promise (<anonymous>)
at Agent.forceHarvestAll   (/app/node_modules/newrelic/lib/agent.js:392:29)
at Timeout.afterTimeout [as _onTimeout]   (/app/node_modules/newrelic/lib/agent.js:514:11)

Expected Behavior

Handle JSON.parse error.
In case it is not possible to parse JSON object from log string, save the whole log string as a message.

Troubleshooting or NR Diag results

As per comment in the code, newrelic calls pino asJson method. It returns invalid JSON string, which is getting passed into newrelic reformatLogLine method.
image
image

Steps to Reproduce

  1. Setup newrelic and enable logging.
  2. Setup pino.
  3. Get instance of pino logger and call logger.info({'some"prop': {'details': true}}).
    Code snippet:
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}}
After this newrelic will pick up the log, try to do JSON.parse and throw the error mentioned above.

Your Environment

  • Node version: 16.16.0
  • pino package version: 7.11.0
  • newrelic package version: 9.14.1 (checked latest one 10.6.0 - same code algorithm)

Additional context

@workato-integration
Copy link

@bizob2828
Copy link
Member

@OlenaHryshko thanks for your report. We can provide some defensive code but this also appears to be a bug with pino because it cannot properly stringify an object that has a " in a key. Since pino manually builds serialization/de-serialization. If you do this in Node.js it just works

> JSON.stringify({ 'test"me': 'value'})
'{"test\\"me":"value"}'
> const a = JSON.stringify({ 'test"me': 'value'})
undefined
> JSON.parse(a)
{ 'test"me': 'value' }
>

@OlenaHryshko
Copy link
Author

@bizob2828 thanks for your response.
Created an issue in pino lib. We'll see if it's possible to fix it there.
As soon as this serializer is custom, I'd suggest adding some defensive code in any case. You never know what edge cases might occur.

@bizob2828
Copy link
Member

@OlenaHryshko that has been released in 10.6.2

@OlenaHryshko
Copy link
Author

@bizob2828 thank you for the notification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
2 participants