Add log option

closes #9

Author: dougwilson

HISTORY.md

@@ -1,3 +1,8 @@
+unreleased
+==========
+
+  * Add `log` option
+
 1.2.3 / 2014-11-21
 ==================
 

README.md

@@ -20,14 +20,37 @@ $ npm install errorhandler
 var errorhandler = require('errorhandler')
 ```
 
-### errorhandler()
+### errorhandler([options])
 
 Create new middleware to handle errors and respond with content negotiation.
 This middleware is only intended to be used in a development environment, as
 the full error stack traces will be sent back to the client when an error
 occurs.
 
-## Example
+#### options.log
+
+Provide a function to be called with the error and a string representation of
+the error. Can be used to write the error to any desired location, or set to
+`false` to only send the error back in the response. Called as
+`log(err, str, req, res)` where `err` is the `Error` object, `str` is a string
+representation of the error, `req` is the request object and `res` is the
+response object (note, this function is invoked _after_ the response has been
+written).
+
+The default value for this option is `true` unless `process.env.NODE_ENV === 'test'`.
+
+Possible values:
+
+  * `true`: Log errors using `console.error(str)`.
+  * `false`: Only send the error back in the response.
+  * A function: pass the error to a function for handling.
+
+## Examples
+
+### Simple example
+
+Basic example of adding this middleware as the error handler only in development
+with `connect` (`express` also can be used in this example).
 
 ```js
 var connect = require('connect')
@@ -41,6 +64,33 @@ if (process.env.NODE_ENV === 'development') {
 }
 ```
 
+### Custom output location
+
+Sometimes you may want to output the errors to a different location than STDERR
+during development, like a system notification, for example.
+
+```js
+var connect = require('connect')
+var errorhandler = require('errorhandler')
+var notifier = require('node-notifier')
+
+var app = connect()
+
+if (process.env.NODE_ENV === 'development') {
+  // only use in development
+  app.use(errorhandler({log: errorNotification}))
+}
+
+function errorNotification(err, str, req) {
+  var title = 'Error in ' + req.method + ' ' + req.url
+
+  notifier.notify({
+    title: title,
+    message: str
+  })
+}
+```
+
 ## License
 
 [MIT](LICENSE)

index.js

@@ -3,11 +3,13 @@
  * Copyright(c) 2010 Sencha Inc.
  * Copyright(c) 2011 TJ Holowaychuk
  * Copyright(c) 2014 Jonathan Ong
+ * Copyright(c) 2014 Douglas Christopher Wilson
  * MIT Licensed
  */
 
 /**
  * Module dependencies.
+ * @private
  */
 
 var accepts = require('accepts')
@@ -17,11 +19,17 @@ var util = require('util')
 
 /**
  * Module variables.
+ * @private
  */
 
 var inspect = util.inspect
 var toString = Object.prototype.toString
 
+/* istanbul ignore next */
+var defer = typeof setImmediate === 'function'
+  ? setImmediate
+  : function(fn){ process.nextTick(fn.bind.apply(fn, arguments)) }
+
 /**
  * Error handler:
  *
@@ -47,10 +55,29 @@ var toString = Object.prototype.toString
  * @api public
  */
 
-exports = module.exports = function errorHandler(){
+exports = module.exports = function errorHandler(options) {
   // get environment
   var env = process.env.NODE_ENV || 'development'
 
+  // get options
+  var opts = options || {}
+
+  // get log option
+  var log = opts.log === undefined
+    ? env !== 'test'
+    : opts.log
+
+  if (typeof log !== 'function' && typeof log !== 'boolean') {
+    throw new TypeError('option log must be function or boolean')
+  }
+
+  // default logging using console.error
+  if (log === true) {
+    log = function logerror(err, str) {
+      console.error(str)
+    }
+  }
+
   return function errorHandler(err, req, res, next){
     // respect err.status
     if (err.status) {
@@ -62,9 +89,10 @@ exports = module.exports = function errorHandler(){
       res.statusCode = 500
     }
 
-    // write error to console
-    if (env !== 'test') {
-      console.error(stringify(err))
+    // log the error
+    var str = stringify(err)
+    if (log) {
+      defer(log, err, str, req, res)
     }
 
     // cannot actually respond
@@ -93,7 +121,7 @@ exports = module.exports = function errorHandler(){
               .replace('{stack}', stack)
               .replace('{title}', escapeHtml(exports.title))
               .replace('{statusCode}', res.statusCode)
-              .replace(/\{error\}/g, escapeHtml(stringify(err)).replace(/  /g, ' &nbsp;').replace(/\n/g, '<br>'));
+              .replace(/\{error\}/g, escapeHtml(str).replace(/  /g, ' &nbsp;').replace(/\n/g, '<br>'))
             res.setHeader('Content-Type', 'text/html; charset=utf-8');
             res.end(html);
         });
@@ -108,7 +136,7 @@ exports = module.exports = function errorHandler(){
     // plain text
     } else {
       res.setHeader('Content-Type', 'text/plain');
-      res.end(stringify(err));
+      res.end(str)
     }
   };
 };

test/test.js

@@ -211,8 +211,161 @@ describe('errorHandler()', function () {
   })
 })
 
-function createServer(error) {
-  var _errorHandler = errorHandler()
+describe('errorHandler(options)', function () {
+  describe('log', function () {
+    it('should reject a string', function () {
+      assert.throws(errorHandler.bind(null, {log: 'yes, please'}), /option log must be/)
+    })
+
+    describe('when "undefined"', function () {
+      var _consoleerror
+
+      before(function () {
+        _consoleerror = console.error
+      })
+      afterEach(function () {
+        console.error = _consoleerror
+      })
+
+      describe('when NODE_ENV == test', function () {
+        alterEnvironment('NODE_ENV', 'test')
+
+        it('should produce no output', function (done) {
+          var error = new Error('boom!')
+          var server = createServer(error)
+
+          console.error = function () {
+            var log = util.format.apply(null, arguments)
+
+            if (log !== error.stack.toString()) {
+              return _consoleerror.apply(this, arguments)
+            }
+
+            done(new Error('console.error written to'))
+          }
+
+          request(server)
+          .get('/')
+          .set('Accept', 'text/plain')
+          .expect(500, error.stack.toString(), done)
+        })
+      })
+
+      describe('when NODE_ENV != test', function () {
+        alterEnvironment('NODE_ENV', '')
+
+        it('should write to console', function (done) {
+          var cb = after(2, done)
+          var error = new Error('boom!')
+          var server = createServer(error)
+
+          console.error = function () {
+            var log = util.format.apply(null, arguments)
+
+            if (log !== error.stack.toString()) {
+              return _consoleerror.apply(this, arguments)
+            }
+
+            cb()
+          }
+
+          request(server)
+          .get('/')
+          .set('Accept', 'text/plain')
+          .expect(500, error.stack.toString(), cb)
+        })
+      })
+    })
+
+    describe('when "true"', function () {
+      var _consoleerror
+
+      before(function () {
+        _consoleerror = console.error
+      })
+      afterEach(function () {
+        console.error = _consoleerror
+      })
+
+      it('should write to console', function (done) {
+        var cb = after(2, done)
+        var error = new Error('boom!')
+        var server = createServer(error, {log: true})
+
+        console.error = function () {
+          var log = util.format.apply(null, arguments)
+
+          if (log !== error.stack.toString()) {
+            return _consoleerror.apply(this, arguments)
+          }
+
+          cb()
+        }
+
+        request(server)
+        .get('/')
+        .set('Accept', 'text/plain')
+        .expect(500, error.stack.toString(), cb)
+      })
+    })
+
+    describe('when "false"', function () {
+      var _consoleerror
+
+      alterEnvironment('NODE_ENV', '')
+      before(function () {
+        _consoleerror = console.error
+      })
+      afterEach(function () {
+        console.error = _consoleerror
+      })
+
+      it('should not write to console', function (done) {
+        var error = new Error('boom!')
+        var server = createServer(error, {log: false})
+
+        console.error = function () {
+          var log = util.format.apply(null, arguments)
+
+          if (log !== error.stack.toString()) {
+            return _consoleerror.apply(this, arguments)
+          }
+
+          done(new Error('console.error written to'))
+        }
+
+        request(server)
+        .get('/')
+        .set('Accept', 'text/plain')
+        .expect(500, error.stack.toString(), done)
+      })
+    })
+
+    describe('when a function', function () {
+      it('should call function', function (done) {
+        var cb = after(2, done)
+        var error = new Error('boom!')
+        var server = createServer(error, {log: log})
+
+        function log(err, str, req, res) {
+          assert.equal(err, error)
+          assert.equal(str, error.stack.toString())
+          assert.equal(req.url, '/')
+          assert.equal(res.statusCode, 500)
+          cb()
+        }
+
+        request(server)
+        .get('/')
+        .set('Accept', 'text/plain')
+        .expect(500, error.stack.toString(), cb)
+      })
+    })
+  })
+})
+
+function createServer(error, options) {
+  var _errorHandler = errorHandler(options)
 
   return http.createServer(function (req, res) {
     _errorHandler(error, req, res, function (err) {
@@ -221,3 +374,15 @@ function createServer(error) {
     })
   })
 }
+
+function alterEnvironment(key, value) {
+  var prev
+
+  before(function () {
+    prev = process.env[key]
+    process.env[key] = value
+  })
+  after(function () {
+    process.env[key] = prev
+  })
+}