fix: Controller wraps handler with try/catch (#909)

By having the controller perform try/catch around the
handler function allows us to add extra safety to all
our controllers and safeguards that we will always catch
exceptions thrown by a controller method.

CONTRIBUTING.md

@@ -46,3 +46,15 @@ When you're done making changes and you'd like to propose them for review by ope
 Congratulations! The whole Unleash community thanks you. :sparkles:
 
 Once your PR is merged, you will be proudly listed as a contributor in the [contributor chart](https://github.com/unleash/Unleash/graphs/contributors).
+
+## Nice to know
+
+### Controllers
+
+In order to handle HTTP requests we have an abstraction called [Controller](https://github.com/Unleash/unleash/blob/master/src/lib/routes/controller.ts). If you want to introduce a new route handler for a specific path (and sub pats) you should implement a controller class which extends the base Controller. An example to follow is the [routes/admin-api/feature.ts](https://github.com/Unleash/unleash/blob/master/src/lib/routes/admin-api/feature.ts) implementation. 
+
+The controller takes care of the following:
+- try/catch RequestHandler method
+- error handling with proper response code if they fail
+- `await` the RequestHandler method if it returns a promise (so you don't have to)
+- access control so that you can just list the required permission for a RequestHandler and the base Controller will make sure the user have these permissions. 

src/lib/addons/addon-schema.ts

@@ -1,5 +1,5 @@
 import joi from 'joi';
-import { nameType } from '../routes/admin-api/util';
+import { nameType } from '../routes/util';
 import { tagTypeSchema } from '../services/tag-type-schema';
 
 export const addonDefinitionSchema = joi.object().keys({

src/lib/middleware/no-authentication.test.ts

@@ -1,14 +1,14 @@
+import { IAuthRequest } from 'lib/routes/unleash-types';
 import supertest from 'supertest';
 import express from 'express';
 import noAuthentication from './no-authentication';
-import { IUserRequest } from '../routes/admin-api/user';
 
 test('should add dummy user object to all requests', () => {
     expect.assertions(1);
 
     const app = express();
     noAuthentication('', app);
-    app.get('/api/admin/test', (req: IUserRequest<any, any, any, any>, res) => {
+    app.get('/api/admin/test', (req: IAuthRequest<any, any, any, any>, res) => {
         const user = { ...req.user };
 
         return res.status(200).json(user).end();

src/lib/routes/admin-api/addon.ts

@@ -6,7 +6,6 @@ import { Logger } from '../../logger';
 import AddonService from '../../services/addon-service';
 
 import extractUser from '../../extract-user';
-import { handleErrors } from './util';
 import {
     CREATE_ADDON,
     UPDATE_ADDON,
@@ -34,13 +33,9 @@ class AddonController extends Controller {
     }
 
     async getAddons(req: Request, res: Response): Promise<void> {
-        try {
         const addons = await this.addonService.getAddons();
         const providers = this.addonService.getProviderDefinitions();
         res.json({ addons, providers });
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async getAddon(
@@ -48,12 +43,8 @@ class AddonController extends Controller {
         res: Response,
     ): Promise<void> {
         const { id } = req.params;
-        try {
         const addon = await this.addonService.getAddon(id);
         res.json(addon);
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async updateAddon(
@@ -64,27 +55,15 @@ class AddonController extends Controller {
         const createdBy = extractUser(req);
         const data = req.body;
 
-        try {
+        const addon = await this.addonService.updateAddon(id, data, createdBy);
-            const addon = await this.addonService.updateAddon(
-                id,
-                data,
-                createdBy,
-            );
         res.status(200).json(addon);
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async createAddon(req: Request, res: Response): Promise<void> {
         const createdBy = extractUser(req);
         const data = req.body;
-        try {
         const addon = await this.addonService.createAddon(data, createdBy);
         res.status(201).json(addon);
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async deleteAddon(
@@ -93,12 +72,8 @@ class AddonController extends Controller {
     ): Promise<void> {
         const { id } = req.params;
         const username = extractUser(req);
-        try {
         await this.addonService.removeAddon(id, username);
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 }
 export default AddonController;

src/lib/routes/admin-api/archive.ts

@@ -1,5 +1,4 @@
 import { Request, Response } from 'express';
-import { handleErrors } from './util';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';
 import { Logger } from '../../logger';
@@ -36,13 +35,10 @@ export default class ArchiveController extends Controller {
 
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
     async getArchivedFeatures(req, res): Promise<void> {
-        try {
+        const features = await this.featureService.getMetadataForAllFeatures(
-            const features =
+            true,
-                await this.featureService.getMetadataForAllFeatures(true);
+        );
         res.json({ version: 2, features });
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 
     async deleteFeature(
@@ -51,25 +47,16 @@ export default class ArchiveController extends Controller {
     ): Promise<void> {
         const { featureName } = req.params;
         const user = extractUser(req);
-        try {
         await this.featureService.deleteFeature(featureName, user);
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
     async reviveFeatureToggle(req, res): Promise<void> {
         const userName = extractUser(req);
         const { featureName } = req.params;
-
-        try {
         await this.featureService.reviveToggle(featureName, userName);
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 }
 

src/lib/routes/admin-api/context.ts

@@ -2,7 +2,6 @@ import { Request, Response } from 'express';
 
 import Controller from '../controller';
 
-import { handleErrors } from './util';
 import extractUser from '../../extract-user';
 
 import {
@@ -45,12 +44,8 @@ class ContextController extends Controller {
     }
 
     async getContextFields(req: Request, res: Response): Promise<void> {
-        try {
         const fields = await this.contextService.getAll();
         res.status(200).json(fields).end();
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async getContextField(req: Request, res: Response): Promise<void> {
@@ -69,12 +64,8 @@ class ContextController extends Controller {
         const value = req.body;
         const userName = extractUser(req);
 
-        try {
         await this.contextService.createContextField(value, userName);
         res.status(201).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async updateContextField(req: Request, res: Response): Promise<void> {
@@ -84,38 +75,23 @@ class ContextController extends Controller {
 
         contextField.name = name;
 
-        try {
+        await this.contextService.updateContextField(contextField, userName);
-            await this.contextService.updateContextField(
-                contextField,
-                userName,
-            );
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async deleteContextField(req: Request, res: Response): Promise<void> {
         const name = req.params.contextField;
         const userName = extractUser(req);
 
-        try {
         await this.contextService.deleteContextField(name, userName);
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async validate(req: Request, res: Response): Promise<void> {
         const { name } = req.body;
 
-        try {
         await this.contextService.validateName(name);
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 }
 export default ContextController;

src/lib/routes/admin-api/email.ts

@@ -1,6 +1,5 @@
 import { ADMIN } from '../../types/permissions';
 import { TemplateFormat } from '../../services/email-service';
-import { handleErrors } from './util';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';
 
@@ -20,7 +19,6 @@ export default class EmailController extends Controller {
 
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
     async getHtmlPreview(req, res): Promise<void> {
-        try {
         const { template } = req.params;
         const ctx = req.query;
         const data = await this.emailService.compileTemplate(
@@ -32,14 +30,10 @@ export default class EmailController extends Controller {
         res.status(200);
         res.send(data);
         res.end();
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
     async getTextPreview(req, res) {
-        try {
         const { template } = req.params;
         const ctx = req.query;
         const data = await this.emailService.compileTemplate(
@@ -51,9 +45,6 @@ export default class EmailController extends Controller {
         res.status(200);
         res.send(data);
         res.end();
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 }
 module.exports = EmailController;

src/lib/routes/admin-api/environments-controller.ts

@@ -5,7 +5,7 @@ import { IUnleashConfig } from '../../types/option';
 import { IEnvironment } from '../../types/model';
 import EnvironmentService from '../../services/environment-service';
 import { Logger } from '../../logger';
-import { handleErrors } from './util';
+import { handleErrors } from '../util';
 import { ADMIN } from '../../types/permissions';
 
 interface EnvironmentParam {

src/lib/routes/admin-api/event.ts

@@ -1,4 +1,3 @@
-import { handleErrors } from './util';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';
 import EventService from '../../services/event-service';
@@ -25,22 +24,15 @@ export default class EventController extends Controller {
 
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
     async getEvents(req, res): Promise<void> {
-        try {
         const events = await this.eventService.getEvents();
         eventDiffer.addDiffs(events);
         res.json({ version, events });
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
     async getEventsForToggle(req, res): Promise<void> {
         const toggleName = req.params.name;
-        try {
+        const events = await this.eventService.getEventsForToggle(toggleName);
-            const events = await this.eventService.getEventsForToggle(
-                toggleName,
-            );
 
         if (events) {
             eventDiffer.addDiffs(events);
@@ -48,9 +40,6 @@ export default class EventController extends Controller {
         } else {
             res.status(404).json({ error: 'Could not find events' });
         }
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 }
 

src/lib/routes/admin-api/feature-type.ts

@@ -1,5 +1,4 @@
 import { Request, Response } from 'express';
-import { handleErrors } from './util';
 import { IUnleashServices } from '../../types/services';
 import FeatureTypeService from '../../services/feature-type-service';
 import { Logger } from '../../logger';
@@ -26,12 +25,8 @@ export default class FeatureTypeController extends Controller {
     }
 
     async getAllFeatureTypes(req: Request, res: Response): Promise<void> {
-        try {
         const types = await this.featureTypeService.getAll();
         res.json({ version, types });
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 }
 

src/lib/routes/admin-api/feature.ts

@@ -3,7 +3,6 @@ import { Request, Response } from 'express';
 
 import Controller from '../controller';
 
-import { handleErrors } from './util';
 import extractUser from '../../extract-user';
 import {
     UPDATE_FEATURE,
@@ -94,22 +93,15 @@ class FeatureController extends Controller {
 
     async getAllToggles(req: Request, res: Response): Promise<void> {
         const query = await this.prepQuery(req.query);
-        try {
+        const features = await this.featureService2.getFeatureToggles(query);
-            const features = await this.featureService2.getFeatureToggles(
-                query,
-            );
 
         res.json({ version, features });
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 
     async getToggle(
         req: Request<{ featureName: string }, any, any, any>,
         res: Response,
     ): Promise<void> {
-        try {
         const name = req.params.featureName;
         const feature = await this.featureService2.getFeatureToggle(name);
         const strategies =
@@ -119,75 +111,52 @@ class FeatureController extends Controller {
             ...feature,
             strategies,
         }).end();
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 
-    // TODO
     async listTags(req: Request, res: Response): Promise<void> {
-        try {
         const tags = await this.featureTagService.listTags(
             req.params.featureName,
         );
         res.json({ version, tags });
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 
-    // TODO
     async addTag(req: Request, res: Response): Promise<void> {
         const { featureName } = req.params;
         const userName = extractUser(req);
-        try {
         const tag = await this.featureTagService.addTag(
             featureName,
             req.body,
             userName,
         );
         res.status(201).json(tag);
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 
     // TODO
     async removeTag(req: Request, res: Response): Promise<void> {
         const { featureName, type, value } = req.params;
         const userName = extractUser(req);
-        try {
         await this.featureTagService.removeTag(
             featureName,
             { type, value },
             userName,
         );
         res.status(200).end();
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 
     async validate(req: Request, res: Response): Promise<void> {
         const { name } = req.body;
 
-        try {
         await this.featureService2.validateName(name);
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async createToggle(req: Request, res: Response): Promise<void> {
         const userName = extractUser(req);
         const toggle = req.body;
 
-        try {
         const validatedToggle = await featureSchema.validateAsync(toggle);
         const { enabled } = validatedToggle;
-            const createdFeature =
+        const createdFeature = await this.featureService2.createFeatureToggle(
-                await this.featureService2.createFeatureToggle(
             validatedToggle.project,
             validatedToggle,
             userName,
@@ -213,10 +182,6 @@ class FeatureController extends Controller {
             enabled,
             strategies,
         });
-        } catch (error) {
-            this.logger.warn(error);
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async updateToggle(req: Request, res: Response): Promise<void> {
@@ -230,7 +195,6 @@ class FeatureController extends Controller {
             featureName,
         );
         if (featureToggleExists) {
-            try {
             await this.featureService2.getFeature(featureName);
             const projectId = await this.featureService2.getProjectId(
                 updatedFeature.name,
@@ -243,9 +207,7 @@ class FeatureController extends Controller {
                 userName,
             );
 
-                await this.featureService2.removeAllStrategiesForEnv(
+            await this.featureService2.removeAllStrategiesForEnv(featureName);
-                    featureName,
-                );
             let strategies;
             if (updatedFeature.strategies) {
                 strategies = await Promise.all(
@@ -269,9 +231,6 @@ class FeatureController extends Controller {
                 enabled,
                 strategies: strategies || [],
             });
-            } catch (error) {
-                handleErrors(res, this.logger, error);
-            }
         } else {
             res.status(404)
                 .json({
@@ -285,7 +244,6 @@ class FeatureController extends Controller {
     // Kept to keep backward compatibility
     async toggle(req: Request, res: Response): Promise<void> {
         const userName = extractUser(req);
-        try {
         const name = req.params.featureName;
         const feature = await this.featureService2.toggle(
             name,
@@ -293,15 +251,11 @@ class FeatureController extends Controller {
             userName,
         );
         res.status(200).json(feature);
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async toggleOn(req: Request, res: Response): Promise<void> {
         const { featureName } = req.params;
         const userName = extractUser(req);
-        try {
         const feature = await this.featureService2.updateEnabled(
             featureName,
             GLOBAL_ENV,
@@ -309,15 +263,11 @@ class FeatureController extends Controller {
             userName,
         );
         res.json(feature);
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async toggleOff(req: Request, res: Response): Promise<void> {
         const { featureName } = req.params;
         const userName = extractUser(req);
-        try {
         const feature = await this.featureService2.updateEnabled(
             featureName,
             GLOBAL_ENV,
@@ -325,13 +275,9 @@ class FeatureController extends Controller {
             userName,
         );
         res.json(feature);
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async staleOn(req: Request, res: Response): Promise<void> {
-        try {
         const { featureName } = req.params;
         const userName = extractUser(req);
         const feature = await this.featureService2.updateStale(
@@ -340,13 +286,9 @@ class FeatureController extends Controller {
             userName,
         );
         res.json(feature).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async staleOff(req: Request, res: Response): Promise<void> {
-        try {
         const { featureName } = req.params;
         const userName = extractUser(req);
         const feature = await this.featureService2.updateStale(
@@ -355,21 +297,14 @@ class FeatureController extends Controller {
             userName,
         );
         res.json(feature).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async archiveToggle(req: Request, res: Response): Promise<void> {
         const { featureName } = req.params;
         const userName = extractUser(req);
 
-        try {
         await this.featureService2.archiveToggle(featureName, userName);
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 }
 export default FeatureController;

src/lib/routes/admin-api/metrics.ts

@@ -1,6 +1,6 @@
 import { Request, Response } from 'express';
 import Controller from '../controller';
-import { handleErrors } from './util';
+import { handleErrors } from '../util';
 import { UPDATE_APPLICATION } from '../../types/permissions';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';

src/lib/routes/admin-api/project/environments.ts

@@ -4,7 +4,7 @@ import { IUnleashConfig } from '../../../types/option';
 import { IUnleashServices } from '../../../types/services';
 import { Logger } from '../../../logger';
 import EnvironmentService from '../../../services/environment-service';
-import { handleErrors } from '../util';
+import { handleErrors } from '../../util';
 import { UPDATE_PROJECT } from '../../../types/permissions';
 
 const PREFIX = '/:projectId/environments';

src/lib/routes/admin-api/project/features.ts

@@ -10,7 +10,6 @@ import {
     IConstraint,
     IStrategyConfig,
 } from '../../../types/model';
-import { handleErrors } from '../util';
 import extractUsername from '../../../extract-user';
 import ProjectHealthService from '../../../services/project-health-service';
 
@@ -111,14 +110,10 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { projectId } = req.params;
-        try {
         const features = await this.featureService.getFeatureToggles({
             project: [projectId],
         });
         res.json({ version: 1, features });
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async createFeatureToggle(
@@ -126,7 +121,6 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { projectId } = req.params;
-        try {
         const userName = extractUsername(req);
         const created = await this.featureService.createFeatureToggle(
             projectId,
@@ -134,9 +128,6 @@ export default class ProjectFeaturesController extends Controller {
             userName,
         );
         res.status(201).json(created);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async getEnvironment(
@@ -144,17 +135,12 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { environment, featureName, projectId } = req.params;
-        try {
+        const environmentInfo = await this.featureService.getEnvironmentInfo(
-            const environmentInfo =
-                await this.featureService.getEnvironmentInfo(
             projectId,
             environment,
             featureName,
         );
         res.status(200).json(environmentInfo);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async getFeature(
@@ -162,12 +148,8 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { featureName } = req.params;
-        try {
         const feature = await this.featureService.getFeature(featureName);
         res.status(200).json(feature);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async toggleEnvironmentOn(
@@ -175,7 +157,6 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { featureName, environment } = req.params;
-        try {
         await this.featureService.updateEnabled(
             featureName,
             environment,
@@ -183,9 +164,6 @@ export default class ProjectFeaturesController extends Controller {
             extractUsername(req),
         );
         res.status(200).end();
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async toggleEnvironmentOff(
@@ -193,7 +171,6 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { featureName, environment } = req.params;
-        try {
         await this.featureService.updateEnabled(
             featureName,
             environment,
@@ -201,9 +178,6 @@ export default class ProjectFeaturesController extends Controller {
             extractUsername(req),
         );
         res.status(200).end();
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async createFeatureStrategy(
@@ -211,7 +185,6 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { projectId, featureName, environment } = req.params;
-        try {
         const featureStrategy = await this.featureService.createStrategy(
             req.body,
             projectId,
@@ -219,9 +192,6 @@ export default class ProjectFeaturesController extends Controller {
             environment,
         );
         res.status(200).json(featureStrategy);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async getFeatureStrategies(
@@ -229,7 +199,6 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { projectId, featureName, environment } = req.params;
-        try {
         const featureStrategies =
             await this.featureService.getStrategiesForEnvironment(
                 projectId,
@@ -237,9 +206,6 @@ export default class ProjectFeaturesController extends Controller {
                 environment,
             );
         res.status(200).json(featureStrategies);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async updateStrategy(
@@ -247,15 +213,11 @@ export default class ProjectFeaturesController extends Controller {
         res: Response,
     ): Promise<void> {
         const { strategyId } = req.params;
-        try {
         const updatedStrategy = await this.featureService.updateStrategy(
             strategyId,
             req.body,
         );
         res.status(200).json(updatedStrategy);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async getStrategy(
@@ -265,11 +227,7 @@ export default class ProjectFeaturesController extends Controller {
         this.logger.info('Getting strategy');
         const { strategyId } = req.params;
         this.logger.info(strategyId);
-        try {
         const strategy = await this.featureService.getStrategy(strategyId);
         res.status(200).json(strategy);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 }

src/lib/routes/admin-api/project/health-report.ts

@@ -5,7 +5,7 @@ import { IUnleashConfig } from '../../../types/option';
 import ProjectHealthService from '../../../services/project-health-service';
 import { Logger } from '../../../logger';
 import { IArchivedQuery, IProjectParam } from '../../../types/model';
-import { handleErrors } from '../util';
+import { handleErrors } from '../../util';
 
 export default class ProjectHealthReport extends Controller {
     private projectHealthService: ProjectHealthService;

src/lib/routes/admin-api/state.ts

@@ -6,7 +6,6 @@ import { Request, Response } from 'express';
 import Controller from '../controller';
 import { ADMIN } from '../../types/permissions';
 import extractUser from '../../extract-user';
-import { handleErrors } from './util';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';
 import { Logger } from '../../logger';
@@ -43,7 +42,6 @@ class StateController extends Controller {
         const userName = extractUser(req);
         const { drop, keep } = req.query;
         // TODO: Should override request type so file is a type on request
-        try {
         let data;
         // @ts-ignore
         if (req.file) {
@@ -66,9 +64,6 @@ class StateController extends Controller {
             keepExisting: paramToBool(keep, true),
         });
         res.sendStatus(202);
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 
     async export(req: Request, res: Response): Promise<void> {
@@ -84,7 +79,6 @@ class StateController extends Controller {
         const includeTags = paramToBool(req.query.tags, true);
         const includeEnvironments = paramToBool(req.query.environments, true);
 
-        try {
         const data = await this.stateService.export({
             includeStrategies,
             includeFeatureToggles,
@@ -97,18 +91,13 @@ class StateController extends Controller {
             if (downloadFile) {
                 res.attachment(`export-${timestamp}.yml`);
             }
-                res.type('yaml').send(
+            res.type('yaml').send(YAML.safeDump(data, { skipInvalid: true }));
-                    YAML.safeDump(data, { skipInvalid: true }),
-                );
         } else {
             if (downloadFile) {
                 res.attachment(`export-${timestamp}.json`);
             }
             res.json(data);
         }
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 }
 export default StateController;

src/lib/routes/admin-api/strategy.ts

@@ -6,7 +6,7 @@ import { Logger } from '../../logger';
 const Controller = require('../controller');
 
 const extractUser = require('../../extract-user');
-const { handleErrors } = require('./util');
+const { handleErrors } = require('../util');
 const {
     DELETE_STRATEGY,
     CREATE_STRATEGY,

src/lib/routes/admin-api/tag-type.ts

@@ -2,7 +2,7 @@ import { Request, Response } from 'express';
 import Controller from '../controller';
 
 import { UPDATE_FEATURE } from '../../types/permissions';
-import { handleErrors } from './util';
+import { handleErrors } from '../util';
 import extractUsername from '../../extract-user';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';

src/lib/routes/admin-api/tag.ts

@@ -7,7 +7,6 @@ import { Logger } from '../../logger';
 import Controller from '../controller';
 
 import { UPDATE_FEATURE } from '../../types/permissions';
-import { handleErrors } from './util';
 import extractUsername from '../../extract-user';
 
 const version = 1;
@@ -33,52 +32,32 @@ class TagController extends Controller {
     }
 
     async getTags(req: Request, res: Response): Promise<void> {
-        try {
         const tags = await this.tagService.getTags();
         res.json({ version, tags });
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async getTagsByType(req: Request, res: Response): Promise<void> {
-        try {
         const tags = await this.tagService.getTagsByType(req.params.type);
         res.json({ version, tags });
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async getTag(req: Request, res: Response): Promise<void> {
         const { type, value } = req.params;
-        try {
         const tag = await this.tagService.getTag({ type, value });
         res.json({ version, tag });
-        } catch (err) {
-            handleErrors(res, this.logger, err);
-        }
     }
 
     async createTag(req: Request, res: Response): Promise<void> {
         const userName = extractUsername(req);
-        try {
         await this.tagService.createTag(req.body, userName);
         res.status(201).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 
     async deleteTag(req: Request, res: Response): Promise<void> {
         const { type, value } = req.params;
         const userName = extractUsername(req);
-        try {
         await this.tagService.deleteTag({ type, value }, userName);
         res.status(200).end();
-        } catch (error) {
-            handleErrors(res, this.logger, error);
-        }
     }
 }
 export default TagController;

src/lib/routes/admin-api/user-admin.ts

@@ -4,7 +4,7 @@ import { ADMIN } from '../../types/permissions';
 import UserService from '../../services/user-service';
 import { AccessService } from '../../services/access-service';
 import { Logger } from '../../logger';
-import { handleErrors } from './util';
+import { handleErrors } from '../util';
 import { IUnleashConfig } from '../../types/option';
 import { EmailService } from '../../services/email-service';
 import ResetTokenService from '../../services/reset-token-service';

src/lib/routes/admin-api/user-feedback-controller.ts

@@ -2,11 +2,10 @@ import { Response } from 'express';
 
 import Controller from '../controller';
 import { Logger } from '../../logger';
-import { IUserRequest } from './user';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';
 import UserFeedbackService from '../../services/user-feedback-service';
-import { handleErrors } from './util';
+import { IAuthRequest } from '../unleash-types';
 
 interface IFeedbackBody {
     neverShow?: boolean;
@@ -32,7 +31,7 @@ class UserFeedbackController extends Controller {
     }
 
     private async recordFeedback(
-        req: IUserRequest<any, any, IFeedbackBody, any>,
+        req: IAuthRequest<any, any, IFeedbackBody, any>,
         res: Response,
     ): Promise<void> {
         const BAD_REQUEST = 400;
@@ -54,18 +53,12 @@ class UserFeedbackController extends Controller {
             neverShow: req.body.neverShow || false,
         };
 
-        try {
+        const updated = await this.userFeedbackService.updateFeedback(feedback);
-            const updated = await this.userFeedbackService.updateFeedback(
-                feedback,
-            );
         res.json(updated);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     private async updateFeedbackSettings(
-        req: IUserRequest<any, any, IFeedbackBody, any>,
+        req: IAuthRequest<any, any, IFeedbackBody, any>,
         res: Response,
     ): Promise<void> {
         const { user } = req;
@@ -78,14 +71,8 @@ class UserFeedbackController extends Controller {
             neverShow: req.body.neverShow || false,
         };
 
-        try {
+        const updated = await this.userFeedbackService.updateFeedback(feedback);
-            const updated = await this.userFeedbackService.updateFeedback(
-                feedback,
-            );
         res.json(updated);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 }
 

src/lib/routes/admin-api/user.ts

@@ -1,13 +1,10 @@
-import { Request, Response } from 'express';
+import { Response } from 'express';
 import { IAuthRequest } from '../unleash-types';
 import Controller from '../controller';
 import { AccessService } from '../../services/access-service';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';
 import UserService from '../../services/user-service';
-import User from '../../types/user';
-import { Logger } from '../../logger';
-import { handleErrors } from './util';
 import SessionService from '../../services/session-service';
 import UserFeedbackService from '../../services/user-feedback-service';
 
@@ -16,11 +13,6 @@ interface IChangeUserRequest {
     confirmPassword: string;
 }
 
-export interface IUserRequest<PARAM, QUERY, BODY, RESPONSE>
-    extends Request<PARAM, QUERY, BODY, RESPONSE> {
-    user: User;
-}
-
 class UserController extends Controller {
     private accessService: AccessService;
 
@@ -30,8 +22,6 @@ class UserController extends Controller {
 
     private sessionService: SessionService;
 
-    private logger: Logger;
-
     constructor(
         config: IUnleashConfig,
         {
@@ -52,7 +42,6 @@ class UserController extends Controller {
         this.userService = userService;
         this.sessionService = sessionService;
         this.userFeedbackService = userFeedbackService;
-        this.logger = config.getLogger('lib/routes/admin-api/user.ts');
 
         this.get('/', this.getUser);
         this.post('/change-password', this.updateUserPass);
@@ -76,12 +65,11 @@ class UserController extends Controller {
     }
 
     async updateUserPass(
-        req: IUserRequest<any, any, IChangeUserRequest, any>,
+        req: IAuthRequest<any, any, IChangeUserRequest, any>,
         res: Response,
     ): Promise<void> {
         const { user } = req;
         const { password, confirmPassword } = req.body;
-        try {
         if (password === confirmPassword) {
             this.userService.validatePassword(password);
             await this.userService.changePassword(user.id, password);
@@ -89,21 +77,12 @@ class UserController extends Controller {
         } else {
             res.status(400).end();
         }
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async mySessions(req: IAuthRequest, res: Response): Promise<void> {
         const { user } = req;
-        try {
+        const sessions = await this.sessionService.getSessionsForUser(user.id);
-            const sessions = await this.sessionService.getSessionsForUser(
-                user.id,
-            );
         res.json(sessions);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 }
 

src/lib/routes/auth/reset-password-controller.ts

@@ -2,7 +2,6 @@ import { Request, Response } from 'express';
 import Controller from '../controller';
 import UserService from '../../services/user-service';
 import { Logger } from '../../logger';
-import { handleErrors } from '../admin-api/util';
 import { IUnleashConfig } from '../../types/option';
 import { IUnleashServices } from '../../types/services';
 
@@ -40,23 +39,15 @@ class ResetPasswordController extends Controller {
     async sendResetPasswordEmail(req: Request, res: Response): Promise<void> {
         const { email } = req.body;
 
-        try {
         await this.userService.createResetPasswordEmail(email);
         res.status(200).end();
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async validatePassword(req: Request, res: Response): Promise<void> {
         const { password } = req.body;
 
-        try {
         this.userService.validatePassword(password);
         res.status(200).end();
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async validateToken(
@@ -64,13 +55,9 @@ class ResetPasswordController extends Controller {
         res: Response,
     ): Promise<void> {
         const { token } = req.query;
-        try {
         const user = await this.userService.getUserForToken(token);
         await this.logout(req);
         res.status(200).json(user);
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     async changePassword(
@@ -79,12 +66,8 @@ class ResetPasswordController extends Controller {
     ): Promise<void> {
         await this.logout(req);
         const { token, password } = req.body;
-        try {
         await this.userService.resetPassword(token, password);
         res.status(200).end();
-        } catch (e) {
-            handleErrors(res, this.logger, e);
-        }
     }
 
     private async logout(req: SessionRequest<any, any, any, any>) {

src/lib/routes/auth/simple-password-provider.js

@@ -1,9 +1,9 @@
 const Controller = require('../controller');
 
 class PasswordProvider extends Controller {
-    constructor({ getLogger }, { userService }) {
+    constructor(config, { userService }) {
-        super();
+        super(config);
-        this.logger = getLogger('/auth/password-provider.js');
+        this.logger = config.getLogger('/auth/password-provider.js');
         this.userService = userService;
 
         this.post('/login', this.login);

src/lib/routes/client-api/feature.ts

@@ -1,6 +1,6 @@
 import memoizee from 'memoizee';
 import { Request, Response } from 'express';
-import { handleErrors } from '../admin-api/util';
+import { handleErrors } from '../util';
 import Controller from '../controller';
 import { IUnleashServices } from '../../types/services';
 import { IUnleashConfig } from '../../types/option';

src/lib/routes/controller.ts

@@ -1,10 +1,24 @@
-import { IRouter } from 'express';
+import { IRouter, Request, Response } from 'express';
+import { Logger } from 'lib/logger';
 import { IUnleashConfig } from '../types/option';
+import { handleErrors } from './util';
 
 const { Router } = require('express');
 const NoAccessError = require('../error/no-access-error');
 const requireContentType = require('../middleware/content_type_checker');
 
+interface IRequestHandler<
+    P = any,
+    ResBody = any,
+    ReqBody = any,
+    ReqQuery = any,
+> {
+    (
+        req: Request<P, ResBody, ReqBody, ReqQuery>,
+        res: Response<ResBody>,
+    ): Promise<void> | void;
+}
+
 const checkPermission = (permission) => async (req, res, next) => {
     if (!permission) {
         return next();
@@ -17,24 +31,48 @@ const checkPermission = (permission) => async (req, res, next) => {
 
 /**
  * Base class for Controllers to standardize binding to express Router.
+ *
+ * This class will take care of the following:
+ * - try/catch inside RequestHandler
+ * - await if the RequestHandler returns a promise.
+ * - access control
  */
 export default class Controller {
+    private ownLogger: Logger;
+
     app: IRouter;
 
     config: IUnleashConfig;
 
     constructor(config: IUnleashConfig) {
+        this.ownLogger = config.getLogger(
+            `controller/${this.constructor.name}`,
+        );
         this.app = Router();
         this.config = config;
     }
 
-    get(path: string, handler: Function, permission?: string): void {
+    wrap(handler: IRequestHandler): IRequestHandler {
-        this.app.get(path, checkPermission(permission), handler.bind(this));
+        return async (req: Request, res: Response) => {
+            try {
+                await handler(req, res);
+            } catch (error) {
+                handleErrors(res, this.ownLogger, error);
+            }
+        };
+    }
+
+    get(path: string, handler: IRequestHandler, permission?: string): void {
+        this.app.get(
+            path,
+            checkPermission(permission),
+            this.wrap(handler.bind(this)),
+        );
     }
 
     post(
         path: string,
-        handler: Function,
+        handler: IRequestHandler,
         permission?: string,
         ...acceptedContentTypes: string[]
     ): void {
@@ -42,13 +80,13 @@ export default class Controller {
             path,
             checkPermission(permission),
             requireContentType(...acceptedContentTypes),
-            handler.bind(this),
+            this.wrap(handler.bind(this)),
         );
     }
 
     put(
         path: string,
-        handler: Function,
+        handler: IRequestHandler,
         permission?: string,
         ...acceptedContentTypes: string[]
     ): void {
@@ -56,17 +94,21 @@ export default class Controller {
             path,
             checkPermission(permission),
             requireContentType(...acceptedContentTypes),
-            handler.bind(this),
+            this.wrap(handler.bind(this)),
         );
     }
 
-    delete(path: string, handler: Function, permission?: string): void {
+    delete(path: string, handler: IRequestHandler, permission?: string): void {
-        this.app.delete(path, checkPermission(permission), handler.bind(this));
+        this.app.delete(
+            path,
+            checkPermission(permission),
+            this.wrap(handler.bind(this)),
+        );
     }
 
     fileupload(
         path: string,
-        filehandler: Function,
+        filehandler: IRequestHandler,
         handler: Function,
         permission?: string,
     ): void {
@@ -74,11 +116,10 @@ export default class Controller {
             path,
             checkPermission(permission),
             filehandler.bind(this),
-            handler.bind(this),
+            this.wrap(handler.bind(this)),
         );
     }
 
-    // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
     use(path: string, router: IRouter): void {
         this.app.use(path, router);
     }

src/lib/routes/health-check.ts

@@ -15,7 +15,7 @@ class HealthCheckController extends Controller {
         config: IUnleashConfig,
         { healthService }: Pick<IUnleashServices, 'healthService'>,
     ) {
-        super();
+        super(config);
         this.logger = config.getLogger('health-check.js');
         this.healthService = healthService;
         this.get('/', (req, res) => this.index(req, res));

src/lib/routes/unleash-types.ts

@@ -1,7 +1,13 @@
 import { Request } from 'express';
+import * as core from 'express-serve-static-core';
 import User from '../types/user';
 
-export interface IAuthRequest extends Request {
+export interface IAuthRequest<
+    PARAM = core.ParamsDictionary,
+    ResBody = any,
+    ReqBody = any,
+    ReqQuery = core.Query,
+> extends Request<PARAM, ResBody, ReqBody, ReqQuery> {
     user: User;
     logout: () => void;
     session: any;

src/lib/routes/util.ts

@@ -1,6 +1,6 @@
 import joi from 'joi';
 import { Response } from 'express';
-import { Logger } from '../../logger';
+import { Logger } from '../logger';
 
 export const customJoi = joi.extend((j) => ({
     type: 'isUrlFriendly',

src/lib/schema/feature-schema.ts

@@ -1,5 +1,5 @@
 import joi from 'joi';
-import { nameType } from '../routes/admin-api/util';
+import { nameType } from '../routes/util';
 
 export const nameSchema = joi
     .object()

src/lib/server-impl.ts

@@ -14,6 +14,7 @@ import User from './types/user';
 
 import * as permissions from './types/permissions';
 import AuthenticationRequired from './types/authentication-required';
+import Controller from './routes/controller';
 import * as eventType from './types/events';
 import { addEventHook } from './event-hook';
 import registerGracefulShutdown from './util/graceful-shutdown';
@@ -145,6 +146,7 @@ const serverImpl = {
     create,
     User,
     AuthenticationRequired,
+    Controller,
     permissions,
     eventType,
 };

src/lib/services/addon-schema.ts

@@ -1,5 +1,5 @@
 import joi from 'joi';
-import { nameType } from '../routes/admin-api/util';
+import { nameType } from '../routes/util';
 
 export const addonSchema = joi
     .object()

src/lib/services/context-schema.js

@@ -1,7 +1,7 @@
 'use strict';
 
 const joi = require('joi');
-const { nameType } = require('../routes/admin-api/util');
+const { nameType } = require('../routes/util');
 
 const nameSchema = joi.object().keys({ name: nameType });
 

src/lib/services/project-schema.js

@@ -1,5 +1,5 @@
 const joi = require('joi');
-const { nameType } = require('../routes/admin-api/util');
+const { nameType } = require('../routes/util');
 
 const projectSchema = joi
     .object()

src/lib/services/project-service.ts

@@ -2,7 +2,7 @@ import User from '../types/user';
 import { AccessService } from './access-service';
 import NameExistsError from '../error/name-exists-error';
 import InvalidOperationError from '../error/invalid-operation-error';
-import { nameType } from '../routes/admin-api/util';
+import { nameType } from '../routes/util';
 import schema from './project-schema';
 import NotFoundError from '../error/notfound-error';
 import {

src/lib/services/state-schema.ts

@@ -4,7 +4,7 @@ import strategySchema from './strategy-schema';
 import { tagSchema } from './tag-schema';
 import { tagTypeSchema } from './tag-type-schema';
 import projectSchema from './project-schema';
-import { nameType } from '../routes/admin-api/util';
+import { nameType } from '../routes/util';
 
 export const featureStrategySchema = joi
     .object()

src/lib/services/strategy-schema.ts

@@ -1,5 +1,5 @@
 const joi = require('joi');
-const { nameType } = require('../routes/admin-api/util');
+const { nameType } = require('../routes/util');
 
 const strategySchema = joi
     .object()

src/lib/services/tag-schema.ts

@@ -1,6 +1,6 @@
 import Joi from 'joi';
 
-import { customJoi } from '../routes/admin-api/util';
+import { customJoi } from '../routes/util';
 
 export const tagSchema = Joi.object()
     .keys({

src/lib/services/tag-type-schema.ts

@@ -1,5 +1,5 @@
 import Joi from 'joi';
-import { customJoi } from '../routes/admin-api/util';
+import { customJoi } from '../routes/util';
 
 export const tagTypeSchema = Joi.object()
     .keys({