Do Not Concatenate Error Message When An Error Stack is Present

[mgreenw]

Fixes: #1660

Normally, the functionality described to concatenate the message properties of additional meta objects makes sense. However, when the meta object is an Error, this does not make sense: a custom printf that prints the stack will double-print the Error.message as it is by default included in the Error.stack.

Example:

error: Exception raised when calling methodConnection closed.                                                        
Error: Connection closed.                                                                                                
    at WebSocket.closeListener (C:\Users\me\Desktop\project\src\index.js:96:16)              
    at WebSocket.emit (events.js:194:15)                                                                                 
    at WebSocket.emitClose (C:\Users\me\Desktop\project\node_modules\ws\lib\websocket.js:191:10)                  
    at Socket.socketOnClose (C:\Users\me\Desktop\project\node_modules\ws\lib\websocket.js:850:15)                 
    at Socket.emit (events.js:189:13)                                                                                    
    at TCP._handle.close (net.js:597:12)

Therefore, if a stack property exists in the meta object, do not append the message to the initial message.

Related:
#1338

[codyzu]

Any progress on this? I don't like the concatenated message for errors. This would be great!

[codyzu]

In the meantime, I wrote a formatter that can be used as a workaround: #1660 (comment)

[mgreenw]

@codyzu I made a very similar custom formatter as a temporary solution!

In terms of progress...I'm not a maintainer so I'm not sure what to do until this is reviewed. I know the tests are failing - I'm looking into it now.

@DABH - what are the next steps on this?

[mgreenw]

@codyzu I fixed the failing tests, so all that should be left is getting approval for this change.

[mgreenw]

@DABH Would you be able to look at this when you get a chance? Thanks!

[sondremare]

Has this been fixed in another commit? Or why is this not merged?

[yaro90]

Any reason not to include this? Thanks

[DABH]

Hi there! Sorry, with essentially no funding for winston it's been hard to keep up with PRs beyond maintenance.

Can someone provide an example of current behavior and behavior with the proposed change?

I'm worried that this is a breaking change and would require a major version bump. Maybe that's what we should do, but would like to hear from folks on that. Or maybe I'm misunderstanding and this isn't a breaking change. I recall that a lot of folks had issues with the way Errors were treated with winston@3, but it's been so long since that release that I imagine most of the community has adapted, so I don't want to break all their stuff again in 3.x with a better solution (but if this is important enough we could start looking at 4.0, it would be helpful if we had some other changes to make to justify such a release).

Thank you for your contribution and for perhaps years of patience on this issue... 🥲

[DABH]

(Of course, this branch would be have to be brought up to date as well or a new/equivalent PR be opened...)