dbhowell / pino-cloudwatch

AWS CloudWatch Logs transport for pino
MIT License
38 stars 13 forks source link

If one message is invalid, the entire stream blows up #12

Closed hchor closed 4 years ago

hchor commented 5 years ago

Problem: If message is blank on any log entry, this library simply stops logging.

I ran the following command to stream fastify pino output to CloudWatch:

npm run dev | pino-cloudwatch ...

Everything seemed fine - the log group was created in CloudWatch logs, etc. I was getting a message because ts-node-dev outputs an initial message that has a blank line, but I thought nothing of it. I couldn't, however, figure out why my CloudWatch group was not receiving a response.

So I figured out it's because if a single message was an empty string, I get a message like:

InvalidParameterException: 2 validation errors detected: Value '' at 'logEvents.1.member.message' failed to satisfy constraint: Member must have length greater than or equal to 1; Value '' at 'logEvents.4.member.message' failed to satisfy constraint: Member must have length greater than or equal to 1

Then I added this block of code to CloudWatchStream.prototype.putLogEvents to filter out empty messages:

options = { ...options, logEvents: options.logEvents.filter(option => option.message) };

and it starts logging.

I'm sure that displaying an error is fine; but completely stopping logging due to 1 invalid message is, imo, not robust and was incredibly difficult to debug. I wasted about 6 hours. I would suggest displaying an error but not completely stopping the logging.

dbhowell commented 5 years ago

Thanks for lodging this issue!

And I agree, it's not ideal to completely stop the logging due to it. Feel free to submit a PR if you feel up to it.

dbhowell commented 4 years ago

This should now be fixed in 0.4.1