diff --git a/lib/bunyan.js b/lib/bunyan.js index 725d9b9..f305684 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -38,6 +38,12 @@ try { } var EventEmitter = require('events').EventEmitter; +try { + var safeJsonStringify = require('safe-json-stringify'); +} catch (e) { + safeJsonStringify = null; +} + // The 'mv' module is required for rotating-file stream support. try { /* Use `+ ''` to hide this import from browserify. */ @@ -814,7 +820,21 @@ Logger.prototype._emit = function (rec, noemit) { // Stringify the object. Attempt to warn/recover on error. var str; if (noemit || this.haveNonRawStreams) { - str = JSON.stringify(rec, safeCycles()) + '\n'; + try { + str = JSON.stringify(rec, safeCycles()) + '\n'; + } + catch(err) { + if (safeJsonStringify) { + str = safeJsonStringify(rec) + '\n'; + } + else { + str = '(Could not JSON stringify data. See stderr for details.)'; + _warn(format('bunyan: ERROR: Could not JSON stringify a property. ' + + 'Please consider installing safe-json-stringify for more ' + + 'information. Stack trace: %s' + , err.stack)); + } + } } if (noemit) diff --git a/package.json b/package.json index 93bc35d..e120163 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,8 @@ "// comment2": "'mv' required for RotatingFileStream", "optionalDependencies": { "dtrace-provider": "~0.3 >0.3.0", - "mv": "~2" + "mv": "~2", + "safe-json-stringify": "1.0.1" }, "devDependencies": { "nodeunit": "0.9.*", diff --git a/test/safe-json-stringify-1.js b/test/safe-json-stringify-1.js new file mode 100644 index 0000000..04c8edf --- /dev/null +++ b/test/safe-json-stringify-1.js @@ -0,0 +1,11 @@ +var bunyan = require('../lib/bunyan'); + +var log = bunyan.createLogger({ + name: 'safe-json-stringify-1' +}); + +var obj = {}; +obj.__defineGetter__('boom', + function() { throw new Error('__defineGetter__ ouch!'); }); +log.info({obj: obj}, 'using __defineGetter__'); + diff --git a/test/safe-json-stringify-2.js b/test/safe-json-stringify-2.js new file mode 100644 index 0000000..79c1ca7 --- /dev/null +++ b/test/safe-json-stringify-2.js @@ -0,0 +1,11 @@ +process.env.BUNYAN_TEST_NO_SAFE_JSON_STRINGIFY = '1'; +var bunyan = require('../lib/bunyan'); + +var log = bunyan.createLogger({ + name: 'safe-json-stringify-2' +}); + +var obj = {}; +obj.__defineGetter__('boom', + function() { throw new Error('__defineGetter__ ouch!'); }); +log.info({obj: obj}, 'using __defineGetter__'); diff --git a/test/safe-json-stringify-3.js b/test/safe-json-stringify-3.js new file mode 100644 index 0000000..87bf120 --- /dev/null +++ b/test/safe-json-stringify-3.js @@ -0,0 +1,16 @@ +var bunyan = require('../lib/bunyan'); + +var log = bunyan.createLogger({ + name: 'safe-json-stringify-3' +}); + +// And using `Object.defineProperty`. +var obj = {}; +Object.defineProperty(obj, 'boom', { + get: function() { throw new Error('defineProperty ouch!'); }, + enumerable: true // enumerable is false by default +}); +// Twice to test the 'warnKey' usage. +for (var i = 0; i < 2; i++) { + log.info({obj: obj}, 'using defineProperty'); +} diff --git a/test/safe-json-stringify-4.js b/test/safe-json-stringify-4.js new file mode 100644 index 0000000..85f94d4 --- /dev/null +++ b/test/safe-json-stringify-4.js @@ -0,0 +1,17 @@ +process.env.BUNYAN_TEST_NO_SAFE_JSON_STRINGIFY = '1'; +var bunyan = require('../lib/bunyan'); + +var log = bunyan.createLogger({ + name: 'safe-json-stringify-4' +}); + +// And using `Object.defineProperty`. +var obj = {}; +Object.defineProperty(obj, 'boom', { + get: function() { throw new Error('defineProperty ouch!'); }, + enumerable: true // enumerable is false by default +}); +// Twice to test the 'warnKey' usage. +for (var i = 0; i < 2; i++) { + log.info({obj: obj}, 'using defineProperty'); +} diff --git a/test/safe-json-stringify.test.js b/test/safe-json-stringify.test.js new file mode 100644 index 0000000..918f20d --- /dev/null +++ b/test/safe-json-stringify.test.js @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2015 Trent Mick. All rights reserved. + * + * If available, use `safe-json-stringfy` as a fallback stringifier. + * This covers the case where an enumerable property throws an error + * in its getter. + * + * See + */ + +var p = console.warn; +var exec = require('child_process').exec; + +// node-tap API +if (require.cache[__dirname + '/tap4nodeunit.js']) + delete require.cache[__dirname + '/tap4nodeunit.js']; +var tap4nodeunit = require('./tap4nodeunit.js'); +var after = tap4nodeunit.after; +var before = tap4nodeunit.before; +var test = tap4nodeunit.test; + + +test('__defineGetter__ boom', function (t) { + var cmd = process.execPath + ' ' + __dirname + '/safe-json-stringify-1.js'; + exec(cmd, function (err, stdout, stderr) { + t.ifError(err, err); + var rec = JSON.parse(stdout.trim()); + t.equal(rec.obj.boom, '[Throws]'); + t.end(); + }); +}); + +test('__defineGetter__ boom, without safe-json-stringify', function (t) { + var cmd = process.execPath + ' ' + __dirname + '/safe-json-stringify-2.js'; + exec(cmd, function (err, stdout, stderr) { + t.ifError(err, err); + t.ok(stdout.indexOf('Exception in JSON.stringify') !== -1); + t.ok(stderr.indexOf( + 'You can install the "safe-json-stringify" module') !== -1); + t.end(); + }); +}); + +test('defineProperty boom', function (t) { + var cmd = process.execPath + ' ' + __dirname + '/safe-json-stringify-3.js'; + exec(cmd, function (err, stdout, stderr) { + t.ifError(err, err); + var recs = stdout.trim().split(/\n/g); + t.equal(recs.length, 2); + var rec = JSON.parse(recs[0]); + t.equal(rec.obj.boom, '[Throws]'); + t.end(); + }); +}); + +test('defineProperty boom, without safe-json-stringify', function (t) { + var cmd = process.execPath + ' ' + __dirname + '/safe-json-stringify-4.js'; + exec(cmd, function (err, stdout, stderr) { + t.ifError(err, err); + t.ok(stdout.indexOf('Exception in JSON.stringify') !== -1); + t.equal(stdout.match(/Exception in JSON.stringify/g).length, 2); + t.ok(stderr.indexOf( + 'You can install the "safe-json-stringify" module') !== -1); + t.equal(stderr.match( + /You can install the "safe-json-stringify" module/g).length, 1); + t.end(); + }); +});