From 65ca3183929aec6f79c4b5aa1d66ee2ecaed93b0 Mon Sep 17 00:00:00 2001
From: Richard Walker <digitalsadhu@gmail.com>
Date: Sun, 5 Jul 2015 19:20:34 +0200
Subject: [PATCH] Create stack-removing errorhandler middleware

Create a wrapper around express' errorhandler that removes the stack
trace from error responses when "options.includeStack" is false.
---
 lib/loopback.js                    |  9 ++---
 package.json                       |  2 +-
 server/middleware/error-handler.js | 16 +++++++++
 test/error-handler.test.js         | 55 ++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 7 deletions(-)
 create mode 100644 server/middleware/error-handler.js
 create mode 100644 test/error-handler.test.js

diff --git a/lib/loopback.js b/lib/loopback.js
index 5fda0c79..fd770e20 100644
--- a/lib/loopback.js
+++ b/lib/loopback.js
@@ -159,6 +159,9 @@ if (loopback.isServer) {
 
   loopback.urlNotFound = loopback['url-not-found'];
   delete loopback['url-not-found'];
+
+  loopback.errorHandler = loopback['error-handler'];
+  delete loopback['error-handler'];
 }
 
 /*
@@ -180,12 +183,6 @@ if (loopback.isServer) {
   loopback.faviconFile = path.resolve(__dirname, '../favicon.ico');
 }
 
-/*!
- * Error handler title
- */
-
-loopback.errorHandler.title = 'Loopback';
-
 /**
  * Add a remote method to a model.
  * @param {Function} fn
diff --git a/package.json b/package.json
index 9d25cce0..b7ab4d54 100644
--- a/package.json
+++ b/package.json
@@ -40,6 +40,7 @@
     "debug": "^2.1.2",
     "depd": "^1.0.0",
     "ejs": "^2.3.1",
+    "errorhandler": "^1.3.4",
     "express": "^4.12.2",
     "inflection": "^1.6.0",
     "loopback-connector-remote": "^1.0.3",
@@ -60,7 +61,6 @@
     "browserify": "^10.0.0",
     "chai": "^2.1.1",
     "cookie-parser": "^1.3.4",
-    "errorhandler": "^1.3.4",
     "es5-shim": "^4.1.0",
     "grunt": "^0.4.5",
     "grunt-browserify": "^3.5.0",
diff --git a/server/middleware/error-handler.js b/server/middleware/error-handler.js
new file mode 100644
index 00000000..c549944b
--- /dev/null
+++ b/server/middleware/error-handler.js
@@ -0,0 +1,16 @@
+var expressErrorHandler = require('errorhandler');
+expressErrorHandler.title = 'Loopback';
+
+module.exports = errorHandler;
+
+function errorHandler(options) {
+  if (!options || options.includeStack !== false) {
+    return expressErrorHandler(options);
+  } else {
+    var baseHandler = expressErrorHandler(options);
+    return function errorHandler(err, req, res, next) {
+      delete err.stack;
+      return baseHandler(err, req, res, next);
+    };
+  }
+}
diff --git a/test/error-handler.test.js b/test/error-handler.test.js
new file mode 100644
index 00000000..d19abf47
--- /dev/null
+++ b/test/error-handler.test.js
@@ -0,0 +1,55 @@
+var loopback = require('../');
+var app;
+var assert = require('assert');
+var request = require('supertest');
+
+describe('loopback.errorHandler(options)', function() {
+
+  it('should return default middleware when options object is not present', function(done) {
+
+    //arrange
+    var app = loopback();
+    app.use(loopback.urlNotFound());
+    app.use(loopback.errorHandler({ log: false }));
+
+    //act/assert
+    request(app)
+      .get('/url-does-not-exist')
+      .end(function(err, res) {
+        assert.ok(res.error.text.match(/<ul id="stacktrace"><li> &nbsp; &nbsp;at raiseUrlNotFoundError/));
+        done();
+      });
+  });
+
+  it('should delete stack when options.includeStack is false', function(done) {
+
+    //arrange
+    var app = loopback();
+    app.use(loopback.urlNotFound());
+    app.use(loopback.errorHandler({ includeStack: false, log: false }));
+
+    //act/assert
+    request(app)
+      .get('/url-does-not-exist')
+      .end(function(err, res) {
+        assert.ok(res.error.text.match(/<ul id="stacktrace"><\/ul>/));
+        done();
+      });
+  });
+
+  it('should pass options on to error handler module', function(done) {
+    //arrange
+    var app = loopback();
+    app.use(loopback.urlNotFound());
+    app.use(loopback.errorHandler({ includeStack: false, log: customLogger }));
+
+    //act
+    request(app).get('/url-does-not-exist').end();
+
+    //assert
+    function customLogger(err, str, req) {
+      assert.ok(err.message === 'Cannot GET /url-does-not-exist');
+      done();
+    }
+  });
+});