TomFrost / Bristol

Insanely configurable logging for Node.js
MIT License
113 stars 19 forks source link

fixes logging of array values (issue #10) #13

Open kgraves opened 9 years ago

kgraves commented 9 years ago

Also adds unit test.

Thanks for this node module @TomFrost. :+1:

jeffijoe commented 8 years ago

@TomFrost are you going to merge this any time soon? :)

wangshan commented 8 years ago

I think I'm hit by the same issue, it's unable to log array of RegExp objects, will this patch fix it?

kgraves commented 8 years ago

No, I don't think this will fix your issue. The problem is how JSON.stringify converts a regex to a string

$ node
> JSON.stringify(/asdf/)
'{}'

You could write a patch to handle a regex case. However, it would only work where it is a top-level object in what you are logging. For nested objects, Bristol uses JSON.stringify. Your best bet might be to use something like regex-stringify before you use Bristol. Otherwise you'll have to implement a solution where you walk the entire logged object, and find each regex.

TomFrost commented 8 years ago

I've been playing with this one locally. While I like what this is doing, It feels a bit offputting that the array values are logged with their numeric keys, which in the context of a flatted log message, are fairly meaningless (and can get downright confusing if you log more than one array, and Bristol uses its method of finding a non-duplicate key).

My next thought was to treat elements of an array as though they were all passed as individual values to the logging function. It worked a treat, but you lose all cohesion/relationship between those values in that process. Sometimes that's fine, but that's a bad default.

So what I've been chewing on is giving it some generic key like "array" (which can be changed by passing it in an object instead) and letting the formatters flatten that however they desire. It's actually a slightly easier integration that way too.

As for the values themselves @wangshan and @kgraves, the next release of Bristol will be 2.0, so I'm looking at refactoring all of the stringifying-logic (json or otherwise) into a shared lib to reduce the largely-repeated code in the formatters. I can certainly do a recursive scan of objects in there and properly call .toString on RegExps before JSON.stringify() gets called. Let me know if that would scratch your itch.

Thoughts are welcome!