Merge pull request #217 from bcgov/feature/exceptions

RFC 7807 Compliance

app/app.js

@@ -3,7 +3,6 @@ const compression = require('compression');
 const config = require('config');
 const cors = require('cors');
 const express = require('express');
-const { ValidationError } = require('express-validation');
 
 const { AuthMode, DEFAULTCORS } = require('./src/components/constants');
 const log = require('./src/components/log')(module.filename);
@@ -72,11 +71,11 @@ if (config.has('server.privacyMask')) {
 }
 
 // Block requests until service is ready
-app.use((_req, res, next) => {
+app.use((_req, _res, next) => {
   if (state.shutdown) {
-    new Problem(503, { details: 'Server is shutting down' }).send(res);
+    throw new Problem(503, { detail: 'Server is shutting down' });
   } else if (!state.ready) {
-    new Problem(503, { details: 'Server is not ready' }).send(res);
+    throw new Problem(503, { detail: 'Server is not ready' });
   } else {
     next();
   }
@@ -108,31 +107,21 @@ apiRouter.use('/v1', v1Router);
 // Root level Router
 app.use(/(\/api)?/, apiRouter);
 
-// Handle ValidationError & 500
-// eslint-disable-next-line no-unused-vars
-app.use((err, _req, res, _next) => {
+// Handle 404
+app.use((req, _res) => { // eslint-disable-line no-unused-vars
+  throw new Problem(404, { instance: req.originalUrl });
+});
+
+// Handle Problem Responses
+app.use((err, req, res, _next) => { // eslint-disable-line no-unused-vars
   if (err instanceof Problem) {
     err.send(res);
-  } else if (err instanceof ValidationError) {
-    log.debug(err);
-    return res.status(err.statusCode).json(err);
   } else {
-    // Only log unexpected errors
-    if (err.stack) log.error(err);
-
-    new Problem(500, 'Server Error', {
-      detail: (err.message) ? err.message : err
-    }).send(res);
+    if (err.stack) log.error(err); // Only log unexpected errors
+    new Problem(500, { detail: err.message ?? err, instance: req.originalUrl }).send(res);
   }
 });
 
-// Handle 404
-app.use((req, res) => {
-  new Problem(404, 'Page Not Found', {
-    detail: req.originalUrl
-  }).send(res);
-});
-
 // Ensure unhandled errors gracefully shut down the application
 process.on('unhandledRejection', err => {
   log.error(`Unhandled Rejection: ${err.message ?? err}`, { function: 'onUnhandledRejection' });

app/package.json

@@ -40,8 +40,8 @@
     "date-fns": "^2.30.0",
     "express": "^4.18.2",
     "express-basic-auth": "^1.2.1",
-    "express-validation": "^4.1.0",
     "express-winston": "^4.2.0",
+    "joi": "^17.10.2",
     "js-yaml": "^4.1.0",
     "jsonwebtoken": "^9.0.1",
     "knex": "^2.5.1",

app/src/components/errorToProblem.js

@@ -40,8 +40,10 @@ function errorToProblem(service, e) {
     if (e.$response && e.$response.body) delete e.$response.body;
     return new Problem(e.$metadata.httpStatusCode, { detail: e });
   } else {
-    log.error(`Unknown error calling ${service}: ${e.message}`, { function: 'errorToProblem', status: 502 });
-    return new Problem(502, `Unknown ${service} Error`, { detail: e.message });
+    // Handle all other errors
+    const message = `${service} Error: ${e.message}`;
+    log.error(message, { error: e, function: 'errorToProblem', status: 500 });
+    return new Problem(500, { detail: message });
   }
 }
 

app/src/components/utils.js

@@ -97,7 +97,7 @@ const utils = {
       return data;
     } catch (err) {
       log.error(err.message, { function: 'getBucket' });
-      throw new Problem(404, { details: err.message });
+      throw new Problem(404, { detail: err.message });
     }
   },