[3.0.0-rc3] Logging object with circular references breaking

[8lueberry]

v3.0.0-rc3

When logging data with circular data, it does not log and subsequent logs are not logged neither

      const customData = {};
      customData.myself = customData; // circular data

      const logger = winston.createLogger({ transports: [new winston.transports.Console()] });
      logger.info('hello'); // shows on console

      logger.error({
        message: 'testing error',
        myCustomData: customData,
      }); // doesn't log

      logger.info('hello'); // doesn't log

[DABH]

Tricky! What would you expect it to log for a circular object?

[8lueberry]

Maybe the same behaviour as 2.4 or at least not breaking the logging pipe.

The problem is that some of the error thrown by some lib contains circular properties.

[Nokel81]

Maybe the way that util does it have [circular] added or maybe [circular ${parentFieldName}]

[crussell52]

Maybe use of https://github.com/moll/json-stringify-safe or https://github.com/davidmarkclements/fast-safe-stringify is an appropriate solution?

[crussell52]

... and subsequent logs are not logged neither

This may be arguable the more important point -- I ran into a case where a formatter threw an exception and all subsequent log attempts quietly failed. An edge-case failure in logging a message should not halt all logging 😟

Is that expected behavior?

[DABH]

Agreed -- seems like the first thing to do is to figure out how to make things more robust so any bad log call doesn't break any future logs. (This should probably be true for all transports, so some change in winston-transport or winston...) PRs/ideas welcome, otherwise we will find time eventually to figure this out.

[crussell52]

@DABH I have been unable to find which layer is swallowing the errors and breaking the logging pipeline. I'm happy to dig in and try to come up with a viable solution for that, but I'll need an assist on finding the root cause.

Do you want to track the "error robustness" in a separate issue? I feel that solving circular references is also important to solve and should be addressed separately.

Edit: Opened #1261 to cover robustness

[crussell52]

@Danlevan It looks like the root cause of the failure to serialize is actually in the logform project -- specifically: winstonjs/logform/blob/master/json.js#L13

You probably want to open an issue there for this -- once it has been solved in the lib, the dependencies will just need to be updated here.

[indexzero]

@crussell52 @Nokel81 @Danlevan removing built-in, default always on support for logging of circular object references was a conscious performance decision for the default winston behavior in winston@3.

This problem is 100% solvable as a custom format – here's a quick & dirty one. If you'd like to contribute one to logform (like circular-json or perhaps a { circular: true } option that users must opt-in to) that would be neat.

const cycle = require('cycle');
const { format } = require('winston');
const { combine, json } = format;

// Create a format to decycle the object
const decycleFormat = format((info, opts) => cycle.decycle(info))

// Combine the decycleFormat with the built-in json format.
const circularJsonFormat = combine(
  decycle(),
  json()
);

[indexzero]

FYI circular JSON is supported by default thanks to winstonjs/logform#35