use built-in express error handler

nicola · nicola · commit 3942aeea0b31 · 2015-08-08T14:16:21.000+02:00
diff --git a/lib/handlers/error.js b/lib/handlers/error.js
@@ -5,25 +5,24 @@ var fs = require('fs');
 
 function errorPageHandler(err, req, res, next) {
     var ldp = req.app.locals.ldp;
-    if (!ldp.noErrorPages) {
-        var errorPage = ldp.errorPages +
-                err.status.toString() + '.html';
-        fs.readFile(errorPage, 'utf8', function(readErr, text) {
-            if (readErr) {
-                defaultErrorHandler(err, res);
-            } else {
-                res.status(err.status);
-                res.send(text);
-            }
-        });
-    } else {
-        defaultErrorHandler(err, res);
+
+    // If noErrorPages is set,
+    // then use built-in express default error handler
+    if (ldp.noErrorPages) {
+        return next(err);
     }
-}
 
-function defaultErrorHandler(err, res) {
-    res.status(err.status);
-    res.send(err.message);
+    // Check if error page exists
+    var errorPage = ldp.errorPages + err.status.toString() + '.html';
+    fs.readFile(errorPage, 'utf8', function(readErr, text) {
+        if (readErr) {
+            return next();
+        }
+
+        res.status(err.status);
+        res.header('Content-Type', 'text/html');
+        res.send(text);
+    });
 }
 
 exports.handler = errorPageHandler;
diff --git a/test/errors.js b/test/errors.js
@@ -17,51 +17,47 @@ var ldnode = require('../index');
 var ACL = require('../lib/acl').ACL;
 var ns = require('../lib/vocab/ns.js').ns;
 
-describe('Error page tests', function() {
-    var errorAddress = 'http://localhost:3457/test/';
+describe('Error pages', function() {
 
-    var errorLdp = ldnode.createServer({
+    // LDP with error pages
+    var errorLdp = ldnode({
         root: __dirname + '/resources',
         errorPages: __dirname + '/resources/errorPages'
     });
-    errorLdp.listen(3457);
+    var errorServer = supertest(errorLdp);
 
-    var errorServer = supertest(errorAddress);
-
-    // Instance of server with error pages flag set to false
-    var noErrorAddress = 'http://localhost:3458/test/';
-
-    var noErrorLdp = ldnode.createServer({
+    // LDP with no error pages
+    var noErrorLdp = ldnode({
         root: __dirname + '/resources',
         noErrorPages: true
     });
-    noErrorLdp.listen(3458);
-
-    var noErrorServer = supertest(errorAddress);
-
+    var noErrorServer = supertest(noErrorLdp);
+
+    function defaultErrorPage(filepath, expected) {
+        var handler = function (res) {
+            var errorFile = read(filepath);
+            if (res.text === errorFile && !expected){
+                console.log("Not default text");
+            }
+        };
+        return handler;
+    }
+
+    describe('noErrorPages', function () {
+        var file404 = 'errorPages/404.html';
+        it('Should return 404 express default page', function(done) {
+            noErrorServer.get('/non-existent-file.html')
+                .expect(defaultErrorPage(file404, false))
+                .expect(404, done);
+        });
+    });
 
-    describe('Error page test', function() {
+    describe('errorPages set', function() {
         var file404 = 'errorPages/404.html';
-        function defaultErrorPage(filepath, expected) {
-            var handler = function (res) {
-                var errorFile = read(filepath);
-                if (res.text === errorFile && !expected){
-                    console.log("Not default text");
-                }
-            };
-            return handler;
-        }
-        it('Should return 404 custom page if flag set to true',
-           function(done) {
-               errorServer.get('non-existent-file.html')
-                   .expect(defaultErrorPage(file404, true))
-                   .expect(404, done);
-           });
-        it('Should return 404 default page if flag set to false',
-           function(done) {
-               noErrorServer.get('non-existent-file.html')
-                   .expect(defaultErrorPage(file404, false))
-                   .expect(404, done);
-           });
+        it('Should return 404 custom page if exists', function(done) {
+            errorServer.get('/non-existent-file.html')
+                .expect(defaultErrorPage(file404, true))
+                .expect(404, done);
+        });
     });
 });
