Replace env option with stacktrace option

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
 
 0.4.0 / 2015-06-14

README.md

@@ -29,17 +29,18 @@ be written out to the `res`, and `res.statusCode` is set from `err.status`.
 
 The final handler will also unpipe anything from `req` when it is invoked.
 
-#### 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

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

test/test.js

@@ -109,11 +109,12 @@ 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(bodyShouldNotContain('boom!'))
+      .expect(500, /Internal Server Error/, done)
     })
 
     it('should handle HEAD', function (done) {
@@ -131,23 +132,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(bodyShouldNotContain('boom!'))
-      .expect(501, /Not Implemented/, done)
-    })
-
     describe('when there is a request body', function () {
       it('should not hang/error when unread', function (done) {
         var buf = new Buffer(1024 * 16)
@@ -204,14 +188,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 () {
@@ -221,7 +197,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)
       })
     })
 
@@ -317,6 +293,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 bodyShouldNotContain(str) {