fix: route error handler should distinguish between async and regular functions

Author: mvegter

lib/server/routers/index.js

@@ -23,7 +23,7 @@ const subsystemsRoute = require('./subsystems.router');
 const tagsRoute = require('./tags.router');
 const usersRoute = require('./users.router');
 const { appendPath } = require('../utils');
-const { deepmerge } = require('../../utilities');
+const { deepmerge, isPromise } = require('../../utilities');
 
 const routes = [
     attachmentRoute,
@@ -62,11 +62,17 @@ const inheritArgs = (parent, child) => {
 const bindRoute = (http, route, parentPath = '') => {
     const localPath = appendPath(parentPath, route.path, route.pathOption);
     if (route.method && route.controller) {
-        http[route.method](localPath, (request, response, next) => {
+        http[route.method](localPath, async (...args) => {
+            const callable = route.controller;
+            if (isPromise(callable)) {
+                await callable(...args).catch(args[args.length - 1]);
+                return;
+            }
+
             try {
-                route.controller(request, response, next);
+                callable(...args);
             } catch (error) {
-                next(error);
+                args[args.length - 1](error);
             }
         }, route.args);
     }

lib/utilities/index.js

@@ -12,9 +12,11 @@
  */
 
 const deepmerge = require('./deepmerge');
+const isPromise = require('./isPromise');
 const Logger = require('./Logger');
 
 module.exports = {
     deepmerge,
+    isPromise,
     Logger,
 };

lib/utilities/isPromise.js

@@ -0,0 +1,32 @@
+/**
+ * @license
+ * Copyright CERN and copyright holders of ALICE O2. This software is
+ * distributed under the terms of the GNU General Public License v3 (GPL
+ * Version 3), copied verbatim in the file "COPYING".
+ *
+ * See http://alice-o2.web.cern.ch/license for full licensing information.
+ *
+ * In applying this license CERN does not waive the privileges and immunities
+ * granted to it by virtue of its status as an Intergovernmental Organization
+ * or submit itself to any jurisdiction.
+ */
+
+/**
+ * Test whether an object looks like a promise.
+ *
+ * @param {Object} obj The object to test.
+ * @returns {Boolean} Whether the object looks like a promise.
+ */
+const isPromise = (obj) => {
+    if (!obj) {
+        return false;
+    }
+
+    if (typeof obj !== 'object' && typeof obj !== 'function') {
+        return false;
+    }
+
+    return typeof obj.then === 'function' || obj.constructor.name === 'AsyncFunction';
+};
+
+module.exports = isPromise;

test/application/interfaces.test.js

@@ -14,6 +14,7 @@
 const fs = require('fs');
 const path = require('path');
 const chai = require('chai');
+const { isPromise } = require('../../lib/utilities');
 
 const { expect } = chai;
 
@@ -39,7 +40,7 @@ module.exports = () => {
                     const expected = 'The method or operation is not implemented.';
 
                     const callable = instance[method] || clazz[method];
-                    if (callable.constructor.name === 'AsyncFunction') {
+                    if (isPromise(callable)) {
                         it('should return a rejected Promise', () => callable()
                             .then(() => expect.fail())
                             .catch((err) => expect(err).to.equal(expected)));

test/utilities/index.js

@@ -12,7 +12,9 @@
  */
 
 const deepmerge = require('./deepmerge.test');
+const isPromise = require('./isPromise.test');
 
 module.exports = () => {
     describe('deepmerge', deepmerge);
+    describe('isPromise', isPromise);
 };

test/utilities/isPromise.test.js

@@ -0,0 +1,31 @@
+/**
+ * @license
+ * Copyright CERN and copyright holders of ALICE O2. This software is
+ * distributed under the terms of the GNU General Public License v3 (GPL
+ * Version 3), copied verbatim in the file "COPYING".
+ *
+ * See http://alice-o2.web.cern.ch/license for full licensing information.
+ *
+ * In applying this license CERN does not waive the privileges and immunities
+ * granted to it by virtue of its status as an Intergovernmental Organization
+ * or submit itself to any jurisdiction.
+ */
+
+const { isPromise } = require('../../lib/utilities');
+const chai = require('chai');
+
+const { expect } = chai;
+
+module.exports = () => {
+    it('should return true', () => {
+        expect(isPromise(Promise.resolve())).to.be.true;
+        expect(isPromise(Promise.reject())).to.be.true;
+    });
+
+    it('should return false', () => {
+        expect(isPromise()).to.be.false;
+        expect(isPromise(null)).to.be.false;
+        expect(isPromise(undefined)).to.be.false;
+        expect(isPromise('Promise')).to.be.false;
+    });
+};