Replace env option with stacktrace option

Author: dougwilson

HISTORY.md

@@ -1,7 +1,9 @@
 unreleased
 ==========
 
+  * Add `stacktrace` option
   * Add `text/plain` fallback response
+  * Remove `env` option; use `stacktrace` option instead
   * Send complete HTML document
   * Set `X-Content-Type-Options: nosniff` header
 

README.md

@@ -25,17 +25,18 @@ This function is to be invoked as `fn(err)`. If `err` is falsy, the handler will
 write out a 404 response to the `res`. If it is truthy, an error response will
 be written out to the `res`, and `res.statusCode` is set from `err.status`.
 
-#### options.env
-
-By default, the environment is determined by `NODE_ENV` variable, but it can be
-overridden by this option.
-
 #### options.onerror
 
 Provide a function to be called with the `err` when it exists. Can be used for
 writing errors to a central location without excessive function generation. Called
 as `onerror(err, req, res)`.
 
+#### options.stacktrace
+
+Specify if the stack trace of the error should be included in the response. By
+default, this is false, but can be enabled when necessary (like in development).
+It is not recommend to enable this on a production deployment
+
 ## Examples
 
 ### always 404

index.js

@@ -41,12 +41,12 @@ module.exports = finalhandler
 function finalhandler(req, res, options) {
   options = options || {}
 
-  // get environment
-  var env = options.env || process.env.NODE_ENV || 'development'
-
   // get error callback
   var onerror = options.onerror
 
+  // get stack trace option
+  var stacktrace = options.stacktrace || false;
+
   return function (err) {
     var body
     var constructBody
@@ -65,9 +65,9 @@ function finalhandler(req, res, options) {
       }
 
       // production gets a basic error message
-      msg = env === 'production'
-        ? http.STATUS_CODES[res.statusCode]
-        : err.stack || err.toString()
+      msg = stacktrace
+        ? err.stack || String(err)
+        : http.STATUS_CODES[res.statusCode]
     } else {
       res.statusCode = 404
       msg = 'Cannot ' + req.method + ' ' + (req.originalUrl || req.url)

test/test.js

@@ -77,11 +77,15 @@ describe('finalhandler(req, res)', function () {
   })
 
   describe('error response', function () {
-    it('should include error stack', function (done) {
+    it('should not include stack trace', function (done) {
       var server = createServer(new Error('boom!'))
       request(server)
       .get('/foo')
-      .expect(500, /Error: boom!.*at.*:[0-9]+:[0-9]+/, done)
+      .expect(500, /Internal Server Error/, function (err, res) {
+        if (err) return done(err)
+        should(res.text).not.match(/boom!/)
+        done()
+      })
     })
 
     it('should handle HEAD', function (done) {
@@ -99,26 +103,6 @@ describe('finalhandler(req, res)', function () {
       .expect(500, done)
     })
 
-    it('should handle non-error-objects', function (done) {
-      var server = createServer('lame string')
-      request(server)
-      .get('/foo')
-      .expect(500, /lame string/, done)
-    })
-
-    it('should send staus code name when production', function (done) {
-      var err = new Error('boom!')
-      err.status = 501
-      var server = createServer(err, {env: 'production'})
-      request(server)
-      .get('/foo')
-      .expect(501, /Not Implemented/, function (err, res) {
-        if (err) return done(err)
-        should(res.text).not.match(/boom!/)
-        done()
-      })
-    })
-
     describe('when HTML acceptable', function () {
       it('should respond with HTML', function (done) {
         var server = createServer(new Error('boom!'))
@@ -128,14 +112,6 @@ describe('finalhandler(req, res)', function () {
         .expect('Content-Type', 'text/html; charset=utf-8')
         .expect(500, /<html/, done)
       })
-
-      it('should escape error stack', function (done) {
-        var server = createServer(new Error('boom!'))
-        request(server)
-        .get('/foo')
-        .set('Accept', 'text/html')
-        .expect(500, /Error: boom!<br> &nbsp; &nbsp;at/, done)
-      })
     })
 
     describe('when HTML not acceptable', function () {
@@ -145,7 +121,7 @@ describe('finalhandler(req, res)', function () {
         .get('/foo')
         .set('Accept', 'application/x-bogus')
         .expect('Content-Type', 'text/plain; charset=utf-8')
-        .expect(500, /Error: boom!\n    at/, done)
+        .expect(500, 'Internal Server Error\n', done)
       })
     })
 
@@ -222,6 +198,40 @@ describe('finalhandler(req, res)', function () {
       })
     })
   })
+
+  describe('stacktrace', function () {
+    it('should include error stack', function (done) {
+      var server = createServer(new Error('boom!'), {stacktrace: true})
+      request(server)
+      .get('/foo')
+      .expect(500, /Error: boom!.*at.*:[0-9]+:[0-9]+/, done)
+    })
+
+    it('should escape error stack for HTML response', function (done) {
+      var server = createServer(new Error('boom!'), {stacktrace: true})
+      request(server)
+      .get('/foo')
+      .set('Accept', 'text/html')
+      .expect(500, /Error: boom!<br> &nbsp; &nbsp;at/, done)
+    })
+
+    it('should not escape error stack for plain text response', function (done) {
+      var server = createServer(new Error('boom!'), {stacktrace: true})
+      request(server)
+      .get('/foo')
+      .set('Accept', 'application/x-bogus')
+      .expect('Content-Type', 'text/plain; charset=utf-8')
+      .expect(500, /Error: boom!\n    at/, done)
+    })
+
+    it('should handle non-error-objects', function (done) {
+      var server = createServer('lame string', {stacktrace: true})
+      request(server)
+      .get('/foo')
+      .set('Accept', 'text/html')
+      .expect(500, /lame string/, done)
+    })
+  })
 })
 
 function createServer(err, opts) {