mia-platform / lc39

The Mia-Platform Node.js service launcher
https://www.mia-platform.eu/
Apache License 2.0
23 stars 3 forks source link

Consistency between error logging and logging guidelines #217

Closed peppemanzi closed 1 year ago

peppemanzi commented 2 years ago

Description of the issue

Logging guidelines for Mia-Platform PaaS environment requires errors to be logged using error key. Since the default configuration for error serialization is used for pino.js, errors logged using error key are stringified by lc39's logger as an empty object ({}). Currently, in order to have a correctly logged error, err key must be used, as described in pino.js docs, however this implies that errors are not indexed by the logging system.

A snippet of code for replicating the issue or showing the proposal usage if applicable

main.js:


const launch = require('lc39')

launch('./example.js', {logLevel: 'debug'})

example.js:


module.exports = async function plugin(fastify) {
  fastify.log.error({
    err: new Error('correctly serialized'),
    error: new Error('wrongly serialized')
  })
}

The expected result

Guidelines and logs indexing must be adapted to handle err key and/or values having error key should be serialized if they are instances of JavaScript Error class

davidebianchi commented 2 years ago

So, we add the serializer for the error field in addition to the already existend serialization of error. Correct?

peppemanzi commented 2 years ago

So, we add the serializer for the error field in addition to the already existend serialization of error. Correct? Yes, I think it could be a good solution.

If value provided to error is an instance of js Error, we should serialize it as it is done for the err field, otherwise we should use default serialization. In this way, we don't force modification to existing code when error field was used to log strings or custom objects/arrays, while we would fix the case when {} was logged instead of a meaningful serialization of an Error instance.