Merge pull request #4051 from weseek/imprv/GW-5883-error-handling-using-http-errors

Imprv/gw 5883 error handling using http errors

packages/app/package.json

@@ -57,6 +57,7 @@
     "growi-plugin-lsx": "^4.0.3",
     "growi-plugin-pukiwiki-like-linker": "^3.1.0",
     "helmet": "^3.13.0",
+    "http-errors": "~1.6.2",
     "i18next": "^20.3.2",
     "i18next-express-middleware": "^2.0.0",
     "i18next-node-fs-backend": "^2.1.0",

packages/slack/package.json

@@ -20,6 +20,7 @@
     "bunyan": "^1.8.15",
     "dotenv-flow": "^3.2.0",
     "extensible-custom-error": "^0.0.7",
+    "http-errors": "^1.8.0",
     "universal-bunyan": "^0.9.2"
   },
   "devDependencies": {

packages/slack/src/middlewares/verify-growi-to-slack-request.ts

@@ -1,5 +1,6 @@
 import { Response, NextFunction } from 'express';
 
+import createError from 'http-errors';
 import loggerFactory from '../utils/logger';
 import { RequestFromGrowi } from '../interfaces/request-between-growi-and-proxy';
 
@@ -15,14 +16,14 @@ export const verifyGrowiToSlackRequest = (req: RequestFromGrowi, res: Response,
   if (str == null) {
     const message = 'The value of header \'x-growi-gtop-tokens\' must not be empty.';
     logger.warn(message, { body: req.body });
-    return res.status(400).send({ message });
+    return next(createError(400, message));
   }
 
   const tokens = str.split(',').map(value => value.trim());
   if (tokens.length === 0) {
     const message = 'The value of header \'x-growi-gtop-tokens\' must include at least one or more tokens.';
     logger.warn(message, { body: req.body });
-    return res.status(400).send({ message });
+    return next(createError(400, message));
   }
 
   req.tokenGtoPs = tokens;

packages/slack/src/middlewares/verify-slack-request.ts

@@ -2,6 +2,7 @@ import { createHmac, timingSafeEqual } from 'crypto';
 import { stringify } from 'qs';
 import { Response, NextFunction } from 'express';
 
+import createError from 'http-errors';
 import loggerFactory from '../utils/logger';
 import { RequestFromSlack } from '../interfaces/request-from-slack';
 
@@ -17,7 +18,7 @@ export const verifySlackRequest = (req: RequestFromSlack, res: Response, next: N
   if (signingSecret == null) {
     const message = 'No signing secret.';
     logger.warn(message, { body: req.body });
-    return res.status(400).send({ message });
+    return next(createError(400, message));
   }
 
   // take out slackSignature and timestamp from header
@@ -27,7 +28,7 @@ export const verifySlackRequest = (req: RequestFromSlack, res: Response, next: N
   if (slackSignature == null || timestamp == null) {
     const message = 'Forbidden. Enter from Slack workspace';
     logger.warn(message, { body: req.body });
-    return res.status(403).send({ message });
+    return next(createError(403, message));
   }
 
   // protect against replay attacks
@@ -35,7 +36,7 @@ export const verifySlackRequest = (req: RequestFromSlack, res: Response, next: N
   if (Math.abs(time - timestamp) > 300) {
     const message = 'Verification failed.';
     logger.warn(message, { body: req.body });
-    return res.status(403).send({ message });
+    return next(createError(403, message));
   }
 
   // generate growi signature
@@ -52,5 +53,5 @@ export const verifySlackRequest = (req: RequestFromSlack, res: Response, next: N
 
   const message = 'Verification failed.';
   logger.warn(message, { body: req.body });
-  return res.status(403).send({ message });
+  return next(createError(403, message));
 };

packages/slackbot-proxy/package.json

@@ -47,6 +47,7 @@
     "express-graceful-exit": "=0.5.0",
     "extensible-custom-error": "^0.0.7",
     "helmet": "^4.6.0",
+    "http-errors": "^1.8.0",
     "method-override": "^3.0.0",
     "mysql2": "^2.2.5",
     "typeorm": "^0.2.31",

packages/slackbot-proxy/src/Server.ts

@@ -18,6 +18,8 @@ import { createTerminus } from '@godaddy/terminus';
 
 import swaggerSettingsForDev from '~/config/swagger/config.dev';
 import swaggerSettingsForProd from '~/config/swagger/config.prod';
+import { GlobalHttpErrorHandlingMiddleware } from './middlewares/GlobalHttpErrorHandlingMiddleware';
+import './filters/CustomHttpErrorFilter';
 import './filters/ResourceNotFoundFilter';
 import loggerFactory from '~/utils/logger';
 
@@ -146,6 +148,10 @@ export class Server {
     this.setupLogger();
   }
 
+  $afterRoutesInit(): void {
+    this.app.use(GlobalHttpErrorHandlingMiddleware);
+  }
+
   $beforeListen(): void {
     const expressApp = this.app.getApp();
     const server = this.injector.get<HttpServer>(HttpServer);

packages/slackbot-proxy/src/controllers/growi-to-slack.ts

@@ -2,6 +2,7 @@ import {
   Controller, Get, Post, Inject, Req, Res, UseBefore, PathParams,
 } from '@tsed/common';
 import axios from 'axios';
+import createError from 'http-errors';
 
 import { WebAPICallResult } from '@slack/web-api';
 
@@ -99,7 +100,7 @@ export class GrowiToSlackCtrl {
     const { tokenGtoPs } = req;
 
     if (tokenGtoPs.length !== 1) {
-      return res.status(400).send({ message: 'installation is invalid' });
+      throw createError(400, 'installation is invalid');
     }
 
     const tokenGtoP = tokenGtoPs[0];
@@ -116,7 +117,7 @@ export class GrowiToSlackCtrl {
 
       const token = relation.installation.data.bot?.token;
       if (token == null) {
-        return res.status(400).send({ message: 'installation is invalid' });
+        throw createError(400, 'installation is invalid');
       }
 
       try {
@@ -124,12 +125,12 @@ export class GrowiToSlackCtrl {
       }
       catch (err) {
         logger.error(err);
-        return res.status(400).send({ message: `failed to request to GROWI. err: ${err.message}` });
+        throw createError(400, `failed to request to GROWI. err: ${err.message}`);
       }
 
       const status = await getConnectionStatus(token);
       if (status.error != null) {
-        return res.status(400).send({ message: `failed to get connection. err: ${status.error}` });
+        throw createError(400, `failed to get connection. err: ${status.error}`);
       }
 
       return res.send({ relation, slackBotToken: token });
@@ -143,7 +144,7 @@ export class GrowiToSlackCtrl {
       .getOne();
 
     if (order == null || order.isExpired()) {
-      return res.status(400).send({ message: 'order has expired or does not exist.' });
+      throw createError(400, 'order has expired or does not exist.');
     }
 
     // Access the GROWI URL saved in the Order record and check if the GtoP token is valid.
@@ -152,19 +153,19 @@ export class GrowiToSlackCtrl {
     }
     catch (err) {
       logger.error(err);
-      return res.status(400).send({ message: `failed to request to GROWI. err: ${err.message}` });
+      throw createError(400, `failed to request to GROWI. err: ${err.message}`);
     }
 
     logger.debug('order found', order);
 
     const token = order.installation.data.bot?.token;
     if (token == null) {
-      return res.status(400).send({ message: 'installation is invalid' });
+      throw createError(400, 'installation is invalid');
     }
 
     const status = await getConnectionStatus(token);
     if (status.error != null) {
-      return res.status(400).send({ message: `failed to get connection. err: ${status.error}` });
+      throw createError(400, `failed to get connection. err: ${status.error}`);
     }
 
     logger.debug('relation test is success', order);

packages/slackbot-proxy/src/filters/CustomHttpErrorFilter.ts

@@ -0,0 +1,22 @@
+import {
+  Catch, ExceptionFilterMethods, PlatformContext, PlatformResponse,
+} from '@tsed/common';
+
+import { CustomHttpError } from '~/models/errors';
+
+@Catch(CustomHttpError)
+export class CustomHttpErrorFilter implements ExceptionFilterMethods {
+
+  async catch(exception: CustomHttpError, ctx: PlatformContext): Promise<PlatformResponse<any>> {
+    const { httpError } = exception;
+    const { response } = ctx;
+
+    return response
+      .status(httpError.status)
+      .body({
+        status: httpError.status,
+        message: httpError.message,
+      });
+  }
+
+}

packages/slackbot-proxy/src/middlewares/GlobalHttpErrorHandlingMiddleware.ts

@@ -0,0 +1,28 @@
+import {
+  Err, Middleware, Next, PlatformContext, PlatformResponse,
+} from '@tsed/common';
+
+import { HttpError, isHttpError } from 'http-errors';
+
+@Middleware()
+export class GlobalHttpErrorHandlingMiddleware {
+
+  use(@Err() err: unknown, @Next() next: Next, ctx: PlatformContext): PlatformResponse<any>|void {
+
+    // handle if the err is a HttpError instance
+    if (isHttpError(err)) {
+      const httpError = err as HttpError;
+      const { response } = ctx;
+
+      return response
+        .status(httpError.status)
+        .body({
+          status: httpError.status,
+          message: httpError.message,
+        });
+    }
+
+    next(err);
+  }
+
+}

packages/slackbot-proxy/src/models/errors.ts

@@ -1,5 +1,7 @@
 import ExtensibleCustomError from 'extensible-custom-error';
 
+import { HttpError } from 'http-errors';
+
 export class InvalidUrlError extends ExtensibleCustomError {
 
   constructor(url: string) {
@@ -7,3 +9,14 @@ export class InvalidUrlError extends ExtensibleCustomError {
   }
 
 }
+
+export class CustomHttpError extends Error {
+
+  httpError: HttpError
+
+  constructor(httpError: HttpError) {
+    super(httpError.message);
+    this.httpError = httpError;
+  }
+
+}

src/server/crowi/express-init.js

@@ -1,5 +1,4 @@
 
-
 module.exports = function(crowi, app) {
   const debug = require('debug')('growi:crowi:express-init');
   const path = require('path');

src/server/crowi/index.js

@@ -13,6 +13,7 @@ const mongoose = require('mongoose');
 const models = require('../models');
 
 const PluginService = require('../plugins/plugin.service');
+const httpErrorHandler = require('../middlewares/http-error-handler');
 
 const sep = path.sep;
 
@@ -451,6 +452,9 @@ Crowi.prototype.start = async function() {
   // setup Express Routes
   this.setupRoutesAtLast();
 
+  // setup Global Error Handlers
+  this.setupGlobalErrorHandlers();
+
   return serverListening;
 };
 
@@ -486,6 +490,14 @@ Crowi.prototype.setupRoutesAtLast = function() {
   require('../routes')(this, this.express);
 };
 
+/**
+ * setup global error handlers
+ * !! this must be after the Routes setup !!
+ */
+Crowi.prototype.setupGlobalErrorHandlers = function() {
+  this.express.use(httpErrorHandler);
+};
+
 /**
  * require API for plugins
  *

src/server/middlewares/http-error-handler.js

@@ -0,0 +1,32 @@
+const { HttpError } = require('../../../node_modules/http-errors');
+
+const isHttpError = (val) => {
+  if (!val || typeof val !== 'object') {
+    return false;
+  }
+
+  if (val instanceof HttpError) {
+    return true;
+  }
+
+  return val instanceof Error
+    && typeof val.expose === 'boolean'
+    && typeof val.statusCode === 'number'
+    && val.status === val.statusCode;
+};
+
+module.exports = async(err, req, res, next) => {
+  // handle if the err is a HttpError instance
+  if (isHttpError(err)) {
+    const httpError = err;
+
+    return res
+      .status(httpError.status)
+      .send({
+        status: httpError.status,
+        message: httpError.message,
+      });
+  }
+
+  next(err);
+};

src/server/service/app.js

@@ -113,6 +113,7 @@ class AppService extends S2sMessageHandlable {
   async setupAfterInstall() {
     this.crowi.pluginService.autoDetectAndLoadPlugins();
     this.crowi.setupRoutesAtLast();
+    this.crowi.setupGlobalErrorHandlers();
 
     // remove message handler
     const { s2sMessagingService } = this;

yarn.lock

@@ -9669,6 +9669,17 @@ http-errors@1.7.3, http-errors@~1.7.2:
     statuses ">= 1.5.0 < 2"
     toidentifier "1.0.0"
 
+http-errors@^1.8.0:
+  version "1.8.0"
+  resolved "https://registry.yarnpkg.com/http-errors/-/http-errors-1.8.0.tgz#75d1bbe497e1044f51e4ee9e704a62f28d336507"
+  integrity sha512-4I8r0C5JDhT5VkvI47QktDW75rNlGVsUf/8hzjCC/wkWI/jdTRmBb9aI7erSG82r1bjKY3F6k28WnsVxB1C73A==
+  dependencies:
+    depd "~1.1.2"
+    inherits "2.0.4"
+    setprototypeof "1.2.0"
+    statuses ">= 1.5.0 < 2"
+    toidentifier "1.0.0"
+
 http-proxy-agent@^4.0.0, http-proxy-agent@^4.0.1:
   version "4.0.1"
   resolved "https://registry.yarnpkg.com/http-proxy-agent/-/http-proxy-agent-4.0.1.tgz#8a8c8ef7f5932ccf953c296ca8291b95aa74aa3a"
@@ -17082,6 +17093,11 @@ setprototypeof@1.1.1:
   resolved "https://registry.yarnpkg.com/setprototypeof/-/setprototypeof-1.1.1.tgz#7e95acb24aa92f5885e0abef5ba131330d4ae683"
   integrity sha512-JvdAWfbXeIGaZ9cILp38HntZSFSo3mWg6xGcJJsd+d4aRMOqauag1C63dJfDw7OaMYwEbHMOxEZ1lqVRYP2OAw==
 
+setprototypeof@1.2.0:
+  version "1.2.0"
+  resolved "https://registry.yarnpkg.com/setprototypeof/-/setprototypeof-1.2.0.tgz#66c9a24a73f9fc28cbe66b09fed3d33dcaf1b424"
+  integrity sha512-E5LDX7Wrp85Kil5bhZv46j8jOeboKq5JMmYM3gVGdGH8xFpPWXUMsNrlODCrkoxMEeNi/XZIwuRvY4XNwYMJpw==
+
 sha.js@^2.4.0, sha.js@^2.4.8:
   version "2.4.9"
   resolved "https://registry.yarnpkg.com/sha.js/-/sha.js-2.4.9.tgz#98f64880474b74f4a38b8da9d3c0f2d104633e7d"