Winston fails with "TypeError: Converting circular structure to JSON"

[mherger]

Please tell us about your environment:

• winston version? 3.3.3 (and before)
  • winston@2
  • winston@3
• node -v outputs: v14.17.1
• Operating System?: Amazon Linux 8ae1a37af318 4.14.186-110.268.amzn1.x86_64 SMP Tue Jul 14 02:57:34 UTC 2020 x86_64 GNU/Linux
• Language? any

What is the problem?

App crashes with TypeError: Converting circular structure to JSON when logging through Winston.

What do you expect to happen instead?

Winston should safely convert any object to be logged.

Other information

> node --stack-trace-limit=50 cluster.js

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Socket'
    |     property '_writableState' -> object with constructor 'WritableState'
    |     property 'afterWriteTickInfo' -> object with constructor 'Object'
    --- property 'stream' closes the circle
    at JSON.stringify (<anonymous>)
    at Http._request (/usr/src/app/node_modules/winston/lib/winston/transports/http.js:200:30)
    at Http.log (/usr/src/app/node_modules/winston/lib/winston/transports/http.js:52:10)
    at Http._write (/usr/src/app/node_modules/winston-transport/index.js:82:19)
    at doWrite (/usr/src/app/node_modules/winston-transport/node_modules/readable-stream/lib/_stream_writable.js:428:64)
    at writeOrBuffer (/usr/src/app/node_modules/winston-transport/node_modules/readable-stream/lib/_stream_writable.js:417:5)
    at Http.Writable.write (/usr/src/app/node_modules/winston-transport/node_modules/readable-stream/lib/_stream_writable.js:334:11)
    at DerivedLogger.ondata (/usr/src/app/node_modules/readable-stream/lib/_stream_readable.js:681:20)
    at DerivedLogger.emit (events.js:375:28)
    at DerivedLogger.emit (domain.js:470:12)
    at addChunk (/usr/src/app/node_modules/readable-stream/lib/_stream_readable.js:298:12)
    at readableAddChunk (/usr/src/app/node_modules/readable-stream/lib/_stream_readable.js:280:11)
    at DerivedLogger.Readable.push (/usr/src/app/node_modules/readable-stream/lib/_stream_readable.js:241:10)
    at DerivedLogger.Transform.push (/usr/src/app/node_modules/readable-stream/lib/_stream_transform.js:139:32)
    at DerivedLogger._transform (/usr/src/app/node_modules/winston/lib/winston/logger.js:305:12)
    at DerivedLogger.Transform._read (/usr/src/app/node_modules/readable-stream/lib/_stream_transform.js:177:10)
    at DerivedLogger.Transform._write (/usr/src/app/node_modules/readable-stream/lib/_stream_transform.js:164:83)
    at doWrite (/usr/src/app/node_modules/readable-stream/lib/_stream_writable.js:409:139)
    at writeOrBuffer (/usr/src/app/node_modules/readable-stream/lib/_stream_writable.js:398:5)
    at DerivedLogger.Writable.write (/usr/src/app/node_modules/readable-stream/lib/_stream_writable.js:307:11)
    at DerivedLogger.log (/usr/src/app/node_modules/winston/lib/winston/logger.js:244:14)
    at DerivedLogger.<computed> [as error] (/usr/src/app/node_modules/winston/lib/winston/create-logger.js:95:19)
    at /usr/src/app/lib/util.js:241:12
    at Layer.handle_error (/usr/src/app/node_modules/express/lib/router/layer.js:71:5)
    at trim_prefix (/usr/src/app/node_modules/express/lib/router/index.js:315:13)
    at /usr/src/app/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/usr/src/app/node_modules/express/lib/router/index.js:335:12)
    at Domain.next (/usr/src/app/node_modules/express/lib/router/index.js:275:10)
    at Domain.emit (events.js:375:28)
    at Domain.emit (domain.js:470:12)
    at Request.<anonymous> (/usr/src/app/node_modules/aws-sdk/lib/request.js:29:25)
    at Request.<anonymous> (/usr/src/app/node_modules/aws-sdk/lib/request.js:690:12)
    at Request.callListeners (/usr/src/app/node_modules/aws-sdk/lib/sequential_executor.js:116:18)
    at Request.emit (/usr/src/app/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (/usr/src/app/node_modules/aws-sdk/lib/request.js:688:14)
    at Request.transition (/usr/src/app/node_modules/aws-sdk/lib/request.js:22:10)
    at AcceptorStateMachine.runTo (/usr/src/app/node_modules/aws-sdk/lib/state_machine.js:14:12)
    at /usr/src/app/node_modules/aws-sdk/lib/state_machine.js:26:10
    at Request.<anonymous> (/usr/src/app/node_modules/aws-sdk/lib/request.js:38:9)
    at Request.<anonymous> (/usr/src/app/node_modules/aws-sdk/lib/request.js:690:12)
    at Request.callListeners (/usr/src/app/node_modules/aws-sdk/lib/sequential_executor.js:116:18)
    at Request.emit (/usr/src/app/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (/usr/src/app/node_modules/aws-sdk/lib/request.js:688:14)
    at Request.transition (/usr/src/app/node_modules/aws-sdk/lib/request.js:22:10)
    at AcceptorStateMachine.runTo (/usr/src/app/node_modules/aws-sdk/lib/state_machine.js:14:12)
    at /usr/src/app/node_modules/aws-sdk/lib/state_machine.js:26:10
    at Request.<anonymous> (/usr/src/app/node_modules/aws-sdk/lib/request.js:38:9)
    at Request.<anonymous> (/usr/src/app/node_modules/aws-sdk/lib/request.js:690:12)
    at Request.callListeners (/usr/src/app/node_modules/aws-sdk/lib/sequential_executor.js:116:18)
    at Request.emit (/usr/src/app/node_modules/aws-sdk/lib/sequential_executor.js:78:10)

I know this has been discussed before. And it seems that at some point in v2 development JSON.stringify() was replaced with fast-safe-stringify (see winstonjs/logform#35) to address this issue. But I no longer find any reference to this module, but JSON.stringify() seems to be used again in v3?

I yet have to find a reproducible way to trigger this failure. It only happens "once in a few million" requests. But it's still something we'd like to get rid of.

[mherger]

Or maybe it's only the HTTP transport in

winston/lib/winston/transports/http.js

Line 200 in 2625f60

req.end(Buffer.from(JSON.stringify(options), 'utf8'));

which was not updated?

[talesmgodois]

Same thing happening to me

[wbt]

Looks like @mherger has found at least one source of the issue and an example of a prior commit which solved the same issue elsewhere. A PR applying that to this instance and any other (especially non-test) instances of JSON.stringify would be welcome. A PR with tests to help avoid regression would be too.