[3.0.0-rc3] Logging object with circular references breaking
Closed this issue ยท 10 comments
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
Tricky! What would you expect it to log for a circular object?
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.
Maybe the way that util does it have [circular]
added or maybe [circular ${parentFieldName}]
Maybe use of https://github.com/moll/json-stringify-safe or https://github.com/davidmarkclements/fast-safe-stringify is an appropriate solution?
... 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?
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.
@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
@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.
@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()
);
FYI circular JSON is supported by default thanks to winstonjs/logform#35