err serializer should not copy every key

[stephenh]

On every project I've used pino on (2 :-) ), we end up using our own err serializer that just does:

function errSerializer(err: Error): unknown {
  return {
    type: err.constructor.name,
    message: err.message,
    stack: err.stack,
  };
}

Because the for key in err behavior of the current serializer invariably ends up walking an Error that points to a connection pool or an HTTP request or an ORM metadata field or a Buffer that was not intended to put JSON.stringified and we get huge/nonsensical/corrupt output in our logs.

(The latest instance actually caused our app to crash b/c of an infinite loop in the fast-safe-stringify library: https://github.com/davidmarkclements/fast-safe-stringify/issues/46#issuecomment-633172599)

So, admittedly based on my N=2, I think the err serializer should be changed to either:

1. This above verison that just does type, message, stack, or
2. Make the for key in err smarter and have it only copy over values that are one of:
   • a) primitives,
   • b) have a toJSON property, or
   • c) a is-plain-object (using that npm package) and then apply this same criteria to each value in that object (either as deep as it needs to go or for a fixed number of levels)

I realize both of these are breaking changes, but basically I think err serializer (and really everything in pino that leads to objects getting JSON.stringify'd) should not stringify classes unless it explicitly has a toJSON method on it because its not known whether the class really intended to have its potentially-large/circular 'private' data put on the wire.

(Basically only primitives, object literals, i.e. is-plain-object, and classes with toJSON, are fine to make this assumption.)

As an alternative given the breaking change, maybe having some sort of flag like doNotStringifyClasses (bad name, but you get the idea) on the top-level pino config that switched to different err and formatter behavior and recommend that in the docs as a better default going forward.