From 4cf5c593cd8d709851c4c7cb8f40f142e3bc8493 Mon Sep 17 00:00:00 2001
From: Harrison Ifeanyichukwu <Harrisonifeanyichukwu@gmail.com>
Date: Thu, 8 Nov 2018 13:23:44 +0100
Subject: [PATCH] feat: respond to unhandled promise rejection events

---
 src/modules/Engine.js       | 33 +++++++++++----------------------
 src/modules/Server.js       | 35 +++++++++++++++++++++++++----------
 test/modules/App.spec.js    |  2 +-
 test/modules/Server.spec.js | 25 +++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 33 deletions(-)

diff --git a/src/modules/Engine.js b/src/modules/Engine.js
index 259baad..d3e0a16 100644
--- a/src/modules/Engine.js
+++ b/src/modules/Engine.js
@@ -13,15 +13,13 @@ export default class Engine {
      *@param {http.IncomingMessage} request - the request instance
      *@param {Response} response - the response instance
      *@param {Array} [middlewares] - Array of middlewares
-     *@param {Logger} logger - the logger instance
     */
-    constructor(url, method, request, response, middlewares, logger) {
+    constructor(url, method, request, response, middlewares) {
 
         this.resolved = false;
         this.request = request;
         this.response = response;
         this.middlewares = Util.isArray(middlewares)? middlewares : [];
-        this.logger = logger;
 
         this.url = url.toLowerCase().replace(/[#?].*$/, '').replace(/^\/+/, '').replace(/\/+$/, '');
         this.method = method.toUpperCase();
@@ -67,29 +65,20 @@ export default class Engine {
 
         for (const middleware of this.middlewares) {
             cont = false;
-            try {
-                await middleware(this.request, this.response, next, ...params);
-                //if middleware says continue and response.end is not called, then continue
-                if (cont && !this.response.finished)
-                    continue;
 
-                //else if the response is not ended, end it
-                if (!this.response.finished)
-                    this.response.end();
+            await middleware(this.request, this.response, next, ...params);
+            //if middleware says continue and response.end is not called, then continue
+            if (cont && !this.response.finished)
+                continue;
 
-                return;
-            }
-            catch(ex) {
-                this.logger.fatal(ex, this.response);
-            }
-        }
+            //else if the response is not ended, end it
+            if (!this.response.finished)
+                this.response.end();
 
-        try {
-            await callback(this.request, this.response, ...params);
-        }
-        catch(ex) {
-            this.logger.fatal(ex, this.response);
+            return;
         }
+
+        await callback(this.request, this.response, ...params);
     }
 
     /**
diff --git a/src/modules/Server.js b/src/modules/Server.js
index 67ba393..d8d5a4f 100644
--- a/src/modules/Server.js
+++ b/src/modules/Server.js
@@ -227,6 +227,13 @@ export default class {
         this.logger.fatal(err);
     }
 
+    /**
+     * responds to un handled promise rejection
+    */
+    unhandledPRHandler(reason, p, response) {
+        this.logger.fatal(reason, response);
+    }
+
     /**
      * handle request data event
      *@param {http.IncomingMessage} request - the request object
@@ -235,7 +242,7 @@ export default class {
      *@param {number} bufferDetails.size - the buffer size
      *@param {Array} bufferDetails.buffers - array containing chunks of buffer data
     */
-    onRequestEnd(request, response, bufferDetails) {
+    async onRequestEnd(request, response, bufferDetails) {
 
         //profile the response time
         response.startTime = new Date();
@@ -257,16 +264,24 @@ export default class {
 
         this.parseRequestData(request, url, bufferDetails.buffer);
 
-        this.cordinateRoutes(url, method, request, response).then(status => {
-            if (status)
-                return;
+        const promiseRejectionHandler = Util.generateCallback(
+            this.unhandledPRHandler,
+            this,
+            response
+        );
+        process.on('unhandledRejection', promiseRejectionHandler);
 
-            //send 404 response if router did not resolved
-            let httpErrors = this.config.httpErrors;
-            this.fileServer.serveHttpErrorFile(
-                response, 404, httpErrors.baseDir, httpErrors['404']
-            );
-        });
+        const status = await this.cordinateRoutes(url, method, request, response);
+
+        process.off('unhandledRejection', promiseRejectionHandler);
+        if (status)
+            return;
+
+        //send 404 response if router did not resolved
+        let httpErrors = this.config.httpErrors;
+        this.fileServer.serveHttpErrorFile(
+            response, 404, httpErrors.baseDir, httpErrors['404']
+        );
     }
 
     /**
diff --git a/test/modules/App.spec.js b/test/modules/App.spec.js
index dc29ca7..c785fba 100644
--- a/test/modules/App.spec.js
+++ b/test/modules/App.spec.js
@@ -137,7 +137,7 @@ describe('App', function() {
     });
 
     describe('#listen(port?, callback?)', function() {
-        it(`should start the server at the given port,  calling the callback method once the
+        it(`should start the server at the given port, calling the callback method once the
         server gets started`, function(done) {
             app.listen(null, function() {
                 app.close(function() {
diff --git a/test/modules/Server.spec.js b/test/modules/Server.spec.js
index 13304ed..5099384 100644
--- a/test/modules/Server.spec.js
+++ b/test/modules/Server.spec.js
@@ -385,5 +385,30 @@ describe('Server', function() {
                 });
             });
         });
+
+        it (`should handle all forms of errors, unhandled promises that arises from any of
+            the middleware, or route callbacks`, function(done) {
+
+            server.router.all('/', () => {
+                return new Promise((resolve) => {
+                    resolve(Math.abs(-10));
+                }).then( () => {
+                    throw new Error('chai');
+                });
+            });
+
+            server.listen(null, () => {
+                const spy = sinon.spy(server, 'unhandledPRHandler');
+                request.get('http://localhost:4000/', () => {
+
+                    expect(spy.called).to.be.true;
+                    spy.restore();
+
+                    server.close(() => {
+                        done();
+                    });
+                });
+            });
+        });
     });
 });
\ No newline at end of file