Skip to content

Commit

Permalink
Don't swallow Error message/stack when using formatter
Browse files Browse the repository at this point in the history
Fixes winstonjs#1178

When we use a formatter and pass an `Error` object in as `meta`,
we end up removing the Error's message and stack information when
`decycle`ing it. `clone` checks if the passed-in object is an `Error`
and does not `decycle` it in that case, but when we use a formatter,
this check does not happen because we `clone` the whole `options` object
instead of just cloning `options.meta`.

Simply removing the `&& !(options.meta instanceof Error)` from this line
fixes the problem, by forcing it to `clone` `meta` the first time, when
the `instanceof Error` check will catch it and copy `message` and `stack`
into a new, non-Error object. So when that's buried in `options.meta`,
re-`clone`ing it won't hurt.

Reading the code it seems to not have any bad side-effects. In the places
that use meta, double-`clone`ing is done anyway for other non-`Error`
objects, and the if conditions (`typeof meta !== 'object' && meta != null`)
would be false either way for `Error` objects or non-`Error` objects.
  • Loading branch information
Evan Battaglia committed Jan 25, 2018
1 parent ffe883e commit bb9e1d4
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion lib/winston/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ exports.log = function (options) {
: exports.timestamp,
timestamp = options.timestamp ? timestampFn() : null,
showLevel = options.showLevel === undefined ? true : options.showLevel,
meta = options.meta !== null && options.meta !== undefined && !(options.meta instanceof Error)
meta = options.meta !== null && options.meta !== undefined
? exports.clone(options.meta)
: options.meta || null,
output;
Expand Down
4 changes: 2 additions & 2 deletions test/custom-formatter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function assertFileFormatter (basename, options) {
// We must wait until transport file has emitted the 'flush'
// event to be sure the file has been created and written
transport.once('flush', this.callback.bind(this, null, filename));
transport.log('info', 'What does the fox say?', null, function () {});
transport.log('info', 'What does the fox say?', new Error("foo"), function () {});
},
"should log with the appropriate format": function (_, filename) {
var data = fs.readFileSync(filename, 'utf8');
Expand Down Expand Up @@ -58,7 +58,7 @@ vows.describe('winston/transport/formatter').addBatch({
formatter: {}
}),
"and function value with custom format": assertFileFormatter('customFormatter', {
pattern: /^\d{13,} INFO What does the fox say\?/,
pattern: /^\d{13,} INFO What does the fox say\?.*foo/,
json: false,
timestamp: function() {
return Date.now();
Expand Down

0 comments on commit bb9e1d4

Please sign in to comment.