clone module causes NodeJS to crash with some objects

[paglias]

We're using Loggly and this module at our company and we started to have issues with NodeJS crashing, after some investigation we were able to determine that they were caused by the clone module this library is using when passed some arguments it's not able to clone, in our case it was errors from the quite popular superagent module https://github.com/visionmedia/superagent

More info:

• pvorb/clone#106
• nodejs/node#32568

Reproduction

// setup winston to use loggly and send json

import superagent from 'superagent';

// any url returning a non-successfull http error works 
superagent.post('https://habitica.com/api/v4/user').catch(err => {
  logger.error(err); // this will crash node
});

Given the type of crash and the fact that the maintainer of the clone module doesn't seem interested in fixing it, I would suggest to change it to a different implementation because right now it's making it quite difficult for us to use this module knowing that any error that includes a stream or some other properties will result in the nodejs process to crash

thanks!

[devotox]

This happens way too often.

It seems the master branch of clone has a fix from this PR but no release cut yet

[paglias]

I'm wondering if cloning is even needed in this case, it's basically cloning an object already modified by winston and changing 3 properties, it might be easier to simply restore the properties to the origininal state after the message is sent

I'm not sure who's managing this repo (trying with @alestrunda @michal-bures) but this issue is pretty serious and preventing us from using Loggly right now so it would be nice to know if there's an intention to fix it or not, thanks :)

[devotox]

Make a PR and you could test it in your system by using your GitHub version

[paglias]

@devotox I will if it's not fixed in the coming days but given that this repository is under loggly's organization, that it's a product we're paying for and that the bug is pretty serious (IMO) and makes the use of this module quite dangerous in production I wanted to first see if there was any interest in fixing this directly from Loggly

[alestrunda]

Thanks for pointing that out, this library hasn't been updated for some time, we are currently reviewing what changes should be made.

[alestrunda]

fixed in 644466d

[paglias]

@alestrunda thanks for fixing this! We'll re-enable Loggly at the end of the week, I'll let you know if there's any problem but it shouldn't