From 48b4a86a97fe36c5bfd6e4f7c1ce1b849728a977 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Mon, 4 May 2020 11:19:23 +0200
Subject: [PATCH 1/5] [backend] use common error types

---
 packages/backend-common/src/errors.ts         | 107 ++++++++++++++++++
 packages/backend-common/src/index.ts          |   1 +
 .../src/middleware/errorHandler.test.ts       |  30 ++++-
 .../src/middleware/errorHandler.ts            |  29 ++++-
 plugins/inventory-backend/package.json        |   1 +
 .../src/inventory/AggregatorInventory.ts      |  22 +++-
 .../src/inventory/StaticInventory.ts          |  13 ++-
 .../inventory-backend/src/inventory/types.ts  |   4 +-
 .../inventory-backend/src/service/router.ts   |   9 +-
 yarn.lock                                     |  16 ++-
 10 files changed, 208 insertions(+), 24 deletions(-)
 create mode 100644 packages/backend-common/src/errors.ts

diff --git a/packages/backend-common/src/errors.ts b/packages/backend-common/src/errors.ts
new file mode 100644
index 0000000000..c762df9a28
--- /dev/null
+++ b/packages/backend-common/src/errors.ts
@@ -0,0 +1,107 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+ * A set of common business logic errors.
+ *
+ * The error handler middleware understands these and will translate them to
+ * well formed HTTP responses.
+ *
+ * While these are intentionally analogous to HTTP errors, they are not
+ * intended to be thrown by the request handling layer. In those places, please
+ * use e.g. the http-errors library.
+ */
+
+/**
+ * The request is malformed and cannot be processed.
+ */
+export class BadRequestError extends Error {
+  readonly cause?: Error;
+
+  constructor(message?: string, cause?: Error) {
+    super(message);
+    Object.setPrototypeOf(this, BadRequestError.prototype);
+    Error.captureStackTrace(this, BadRequestError);
+    this.name = this.constructor.name;
+    this.cause = cause;
+  }
+}
+
+/**
+ * The request requires authentication, which was not properly supplied.
+ */
+export class UnauthenticatedError extends Error {
+  readonly cause?: Error;
+
+  constructor(message?: string, cause?: Error) {
+    super(message);
+    Object.setPrototypeOf(this, UnauthenticatedError.prototype);
+    Error.captureStackTrace(this, UnauthenticatedError);
+    this.name = this.constructor.name;
+    this.cause = cause;
+  }
+}
+
+/**
+ * The authenticated caller is not permitted to perform this request.
+ */
+export class ForbiddenError extends Error {
+  readonly cause?: Error;
+
+  constructor(message?: string, cause?: Error) {
+    super(message);
+    Object.setPrototypeOf(this, ForbiddenError.prototype);
+    Error.captureStackTrace(this, ForbiddenError);
+    this.name = this.constructor.name;
+    this.cause = cause;
+  }
+}
+
+/**
+ * The requested resource could not be found.
+ *
+ * Note that this error usually is used to indicate that an entity with a given
+ * ID does not exist, rather than signalling that an entire route is missing.
+ */
+export class NotFoundError extends Error {
+  readonly cause?: Error;
+
+  constructor(message?: string, cause?: Error) {
+    super(message);
+    Object.setPrototypeOf(this, NotFoundError.prototype);
+    Error.captureStackTrace(this, NotFoundError);
+    this.name = this.constructor.name;
+    this.cause = cause;
+  }
+}
+
+/**
+ * The request could not complete due to a conflict in the current state of the
+ * resource.
+ */
+export class ConflictError extends Error {
+  readonly cause?: Error;
+
+  constructor(message?: string, cause?: Error) {
+    super(message);
+    Object.setPrototypeOf(this, ConflictError.prototype);
+    if (Error.captureStackTrace) {
+      Error.captureStackTrace(this, ConflictError);
+    }
+    this.name = this.constructor.name;
+    this.cause = cause;
+  }
+}
diff --git a/packages/backend-common/src/index.ts b/packages/backend-common/src/index.ts
index 54b9f5c40f..b2c38ab506 100644
--- a/packages/backend-common/src/index.ts
+++ b/packages/backend-common/src/index.ts
@@ -14,5 +14,6 @@
  * limitations under the License.
  */
 
+export * from './errors';
 export * from './logging';
 export * from './middleware';
diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts
index f90794b07b..8f4a8cfea4 100644
--- a/packages/backend-common/src/middleware/errorHandler.test.ts
+++ b/packages/backend-common/src/middleware/errorHandler.test.ts
@@ -17,6 +17,7 @@
 import express from 'express';
 import createError from 'http-errors';
 import request from 'supertest';
+import * as errors from '../errors';
 import { errorHandler } from './errorHandler';
 
 describe('errorHandler', () => {
@@ -33,7 +34,7 @@ describe('errorHandler', () => {
     expect(response.text).toBe('some message');
   });
 
-  it('takes code from StatusCodeError', async () => {
+  it('takes code from http-errors library errors', async () => {
     const app = express();
     app.use('/breaks', () => {
       throw createError(432, 'Some Message');
@@ -45,4 +46,31 @@ describe('errorHandler', () => {
     expect(response.status).toBe(432);
     expect(response.text).toContain('Some Message');
   });
+
+  it('handles well-known error classes', async () => {
+    const app = express();
+    app.use('/BadRequestError', () => {
+      throw new errors.BadRequestError();
+    });
+    app.use('/UnauthenticatedError', () => {
+      throw new errors.UnauthenticatedError();
+    });
+    app.use('/ForbiddenError', () => {
+      throw new errors.ForbiddenError();
+    });
+    app.use('/NotFoundError', () => {
+      throw new errors.NotFoundError();
+    });
+    app.use('/ConflictError', () => {
+      throw new errors.ConflictError();
+    });
+    app.use(errorHandler());
+
+    const r = request(app);
+    expect((await r.get('/BadRequestError')).status).toBe(400);
+    expect((await r.get('/UnauthenticatedError')).status).toBe(401);
+    expect((await r.get('/ForbiddenError')).status).toBe(403);
+    expect((await r.get('/NotFoundError')).status).toBe(404);
+    expect((await r.get('/ConflictError')).status).toBe(409);
+  });
 });
diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts
index 655d93ad91..d14c32ce16 100644
--- a/packages/backend-common/src/middleware/errorHandler.ts
+++ b/packages/backend-common/src/middleware/errorHandler.ts
@@ -15,12 +15,12 @@
  */
 
 import { ErrorRequestHandler, NextFunction, Request, Response } from 'express';
+import * as errors from '../errors';
 
 /**
  * Express middleware to handle errors during request processing.
  *
- * This is commonly the second to last middleware in the chain (before the
- * notFoundHandler).
+ * This is commonly the very last middleware in the chain.
  *
  * Its primary purpose is not to do translation of business logic exceptions,
  * but rather to be a gobal catch-all for uncaught "fatal" errors that are
@@ -36,8 +36,12 @@ export function errorHandler(): ErrorRequestHandler {
     error: Error,
     _request: Request,
     response: Response,
-    _next: NextFunction,
+    next: NextFunction,
   ) => {
+    if (response.headersSent) {
+      next(error);
+    }
+
     const status = getStatusCode(error);
     const message = error.message;
     response.status(status).send(message);
@@ -45,8 +49,8 @@ export function errorHandler(): ErrorRequestHandler {
 }
 
 function getStatusCode(error: Error): number {
+  // Look for common http library status codes
   const knownStatusCodeFields = ['statusCode', 'status'];
-
   for (const field of knownStatusCodeFields) {
     const statusCode = (error as any)[field];
     if (
@@ -59,5 +63,22 @@ function getStatusCode(error: Error): number {
     }
   }
 
+  // Handle well-known error types
+  switch (error.name) {
+    case errors.BadRequestError.name:
+      return 400;
+    case errors.UnauthenticatedError.name:
+      return 401;
+    case errors.ForbiddenError.name:
+      return 403;
+    case errors.NotFoundError.name:
+      return 404;
+    case errors.ConflictError.name:
+      return 409;
+    default:
+      break;
+  }
+
+  // Fall back to internal server error
   return 500;
 }
diff --git a/plugins/inventory-backend/package.json b/plugins/inventory-backend/package.json
index 4b7fd0ecc7..86e3981147 100644
--- a/plugins/inventory-backend/package.json
+++ b/plugins/inventory-backend/package.json
@@ -21,6 +21,7 @@
     "compression": "^1.7.4",
     "cors": "^2.8.5",
     "express": "^4.17.1",
+    "express-promise-router": "^3.0.3",
     "helmet": "^3.22.0",
     "morgan": "^1.10.0",
     "winston": "^3.2.1"
diff --git a/plugins/inventory-backend/src/inventory/AggregatorInventory.ts b/plugins/inventory-backend/src/inventory/AggregatorInventory.ts
index ef01be962f..b768b8eb23 100644
--- a/plugins/inventory-backend/src/inventory/AggregatorInventory.ts
+++ b/plugins/inventory-backend/src/inventory/AggregatorInventory.ts
@@ -14,19 +14,29 @@
  * limitations under the License.
  */
 
+import { NotFoundError } from '@backstage/backend-common';
 import { Component, Inventory } from './types';
 
 export class AggregatorInventory implements Inventory {
   inventories: Inventory[] = [];
 
-  list(): Promise<Component[]> {
-    return Promise.all(this.inventories.map((i) => i.list())).then((lists) =>
-      lists.flat(),
-    );
+  async list(): Promise<Component[]> {
+    const lists = await Promise.all(this.inventories.map((i) => i.list()));
+    return lists.flat();
   }
 
-  item(id: string): Promise<Component | undefined> {
-    return this.list().then((items) => items.find((i) => i.id === id));
+  item(id: string): Promise<Component> {
+    return new Promise((resolve, reject) => {
+      const promises = this.inventories.map((i) =>
+        i.item(id).then(resolve, () => {
+          // For now, just swallow errors in individual inventories;
+          // should handle partial errors better
+        }),
+      );
+      Promise.all(promises).then(() =>
+        reject(new NotFoundError(`Found no component with ID ${id}`)),
+      );
+    });
   }
 
   enlist(inventory: Inventory) {
diff --git a/plugins/inventory-backend/src/inventory/StaticInventory.ts b/plugins/inventory-backend/src/inventory/StaticInventory.ts
index 95a4466392..4569ae8ed2 100644
--- a/plugins/inventory-backend/src/inventory/StaticInventory.ts
+++ b/plugins/inventory-backend/src/inventory/StaticInventory.ts
@@ -14,16 +14,21 @@
  * limitations under the License.
  */
 
+import { NotFoundError } from '@backstage/backend-common';
 import { Component, Inventory } from './types';
 
 export class StaticInventory implements Inventory {
   constructor(private components: Component[]) {}
 
-  list(): Promise<Component[]> {
-    return Promise.resolve([...this.components]);
+  async list(): Promise<Component[]> {
+    return this.components.slice();
   }
 
-  item(id: string): Promise<Component | undefined> {
-    return this.list().then((items) => items.find((i) => i.id === id));
+  async item(id: string): Promise<Component> {
+    const item = this.components.find((i) => i.id === id);
+    if (!item) {
+      throw new NotFoundError(`Found no component with ID ${id}`);
+    }
+    return item;
   }
 }
diff --git a/plugins/inventory-backend/src/inventory/types.ts b/plugins/inventory-backend/src/inventory/types.ts
index 3d30b11881..863dccb291 100644
--- a/plugins/inventory-backend/src/inventory/types.ts
+++ b/plugins/inventory-backend/src/inventory/types.ts
@@ -19,6 +19,6 @@ export type Component = {
 };
 
 export type Inventory = {
-  list: () => Promise<Component[]>;
-  item: (id: string) => Promise<Component | undefined>;
+  list(): Promise<Component[]>;
+  item(id: string): Promise<Component>;
 };
diff --git a/plugins/inventory-backend/src/service/router.ts b/plugins/inventory-backend/src/service/router.ts
index efaff00cb8..a05fac08be 100644
--- a/plugins/inventory-backend/src/service/router.ts
+++ b/plugins/inventory-backend/src/service/router.ts
@@ -16,6 +16,7 @@
 
 import express from 'express';
 import { Logger } from 'winston';
+import Router from 'express-promise-router';
 import { Inventory } from '../inventory';
 
 export interface RouterOptions {
@@ -29,7 +30,7 @@ export async function createRouter(
   const inventory = options.inventory;
   const logger = options.logger.child({ plugin: 'inventory' });
 
-  const router = express.Router();
+  const router = Router();
   router
     .get('/', async (req, res) => {
       const components = await inventory.list();
@@ -38,11 +39,7 @@ export async function createRouter(
     .get('/:id', async (req, res) => {
       const { id } = req.params;
       const component = await inventory.item(id);
-      if (component) {
-        res.status(200).send(component);
-      } else {
-        res.status(404).send();
-      }
+      res.status(200).send(component);
     });
 
   const app = express();
diff --git a/yarn.lock b/yarn.lock
index 686575eec3..85fb313077 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -9208,6 +9208,15 @@ expect@^25.1.0:
     jest-message-util "^25.1.0"
     jest-regex-util "^25.1.0"
 
+express-promise-router@^3.0.3:
+  version "3.0.3"
+  resolved "https://registry.npmjs.org/express-promise-router/-/express-promise-router-3.0.3.tgz#5e6d22a5a3f013d71833172fe8d7ab780c3f6b70"
+  integrity sha1-Xm0ipaPwE9cYMxcv6NereAw/a3A=
+  dependencies:
+    is-promise "^2.1.0"
+    lodash.flattendeep "^4.0.0"
+    methods "^1.0.0"
+
 express@^4.17.0, express@^4.17.1:
   version "4.17.1"
   resolved "https://registry.npmjs.org/express/-/express-4.17.1.tgz#4491fc38605cf51f8629d39c2b5d026f98a4c134"
@@ -13627,6 +13636,11 @@ lodash.escaperegexp@^4.1.2:
   resolved "https://registry.npmjs.org/lodash.escaperegexp/-/lodash.escaperegexp-4.1.2.tgz#64762c48618082518ac3df4ccf5d5886dae20347"
   integrity sha1-ZHYsSGGAglGKw99Mz11YhtriA0c=
 
+lodash.flattendeep@^4.0.0:
+  version "4.4.0"
+  resolved "https://registry.npmjs.org/lodash.flattendeep/-/lodash.flattendeep-4.4.0.tgz#fb030917f86a3134e5bc9bec0d69e0013ddfedb2"
+  integrity sha1-+wMJF/hqMTTlvJvsDWngAT3f7bI=
+
 lodash.get@^4.4.2:
   version "4.4.2"
   resolved "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99"
@@ -14152,7 +14166,7 @@ merge@^1.2.1:
   resolved "https://registry.npmjs.org/merge/-/merge-1.2.1.tgz#38bebf80c3220a8a487b6fcfb3941bb11720c145"
   integrity sha512-VjFo4P5Whtj4vsLzsYBu5ayHhoHJ0UqNm7ibvShmbmoz7tGi0vXaoJbGdB+GmDMLUdg8DpQXEIeVDAe8MaABvQ==
 
-methods@^1.1.1, methods@^1.1.2, methods@~1.1.2:
+methods@^1.0.0, methods@^1.1.1, methods@^1.1.2, methods@~1.1.2:
   version "1.1.2"
   resolved "https://registry.npmjs.org/methods/-/methods-1.1.2.tgz#5529a4d67654134edcc5266656835b0f851afcee"
   integrity sha1-VSmk1nZUE07cxSZmVoNbD4Ua/O4=

From 1b4b98cd053770fac1f2906604ed171e2c700bdf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Mon, 4 May 2020 13:06:43 +0200
Subject: [PATCH 2/5] Show stack in dev

---
 .../src/middleware/errorHandler.ts             | 18 ++++++++++++++++--
 plugins/inventory-backend/package.json         |  2 +-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts
index d14c32ce16..eb1b0597a7 100644
--- a/packages/backend-common/src/middleware/errorHandler.ts
+++ b/packages/backend-common/src/middleware/errorHandler.ts
@@ -17,6 +17,15 @@
 import { ErrorRequestHandler, NextFunction, Request, Response } from 'express';
 import * as errors from '../errors';
 
+export type ErrorHandlerOptions = {
+  /**
+   * Whether error response bodies should show error stack traces or not.
+   *
+   * If not specified, by default shows stack traces only in development mode.
+   */
+  showStackTraces?: boolean;
+};
+
 /**
  * Express middleware to handle errors during request processing.
  *
@@ -30,7 +39,12 @@ import * as errors from '../errors';
  *
  * @returns An Express error request handler
  */
-export function errorHandler(): ErrorRequestHandler {
+export function errorHandler(
+  options: ErrorHandlerOptions = {},
+): ErrorRequestHandler {
+  const showStackTraces =
+    options.showStackTraces ?? process.env.NODE_ENV === 'development';
+
   /* eslint-disable @typescript-eslint/no-unused-vars */
   return (
     error: Error,
@@ -43,7 +57,7 @@ export function errorHandler(): ErrorRequestHandler {
     }
 
     const status = getStatusCode(error);
-    const message = error.message;
+    const message = showStackTraces ? error.stack : error.message;
     response.status(status).send(message);
   };
 }
diff --git a/plugins/inventory-backend/package.json b/plugins/inventory-backend/package.json
index 86e3981147..16d510a968 100644
--- a/plugins/inventory-backend/package.json
+++ b/plugins/inventory-backend/package.json
@@ -5,7 +5,7 @@
   "license": "Apache-2.0",
   "private": true,
   "scripts": {
-    "start": "tsc-watch --onFirstSuccess \"nodemon dist/run.js\"",
+    "start": "tsc-watch --onFirstSuccess \"cross-env NODE_ENV=development nodemon dist/run.js\"",
     "build": "tsc",
     "lint": "backstage-cli lint",
     "test": "backstage-cli test",

From e5529c249ba06b2341990b34ceac7d322e483915 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Mon, 4 May 2020 15:54:07 +0200
Subject: [PATCH 3/5] Better errors

---
 packages/backend-common/src/errors.ts | 74 ++++++++++++---------------
 1 file changed, 32 insertions(+), 42 deletions(-)

diff --git a/packages/backend-common/src/errors.ts b/packages/backend-common/src/errors.ts
index c762df9a28..c3cf53453e 100644
--- a/packages/backend-common/src/errors.ts
+++ b/packages/backend-common/src/errors.ts
@@ -25,48 +25,52 @@
  * use e.g. the http-errors library.
  */
 
+class CustomErrorBase extends Error {
+  readonly cause?: Error;
+
+  constructor(constructor: Function, message?: string, cause?: Error) {
+    super(message);
+    Object.setPrototypeOf(this, constructor.prototype);
+    Error.captureStackTrace(this, constructor);
+    this.name = this.constructor.name;
+    this.cause = cause;
+  }
+
+  toString() {
+    let result = super.toString();
+
+    if (this.cause) {
+      result += `; caused by ${this.cause.toString()}`;
+    }
+
+    return result;
+  }
+}
+
 /**
  * The request is malformed and cannot be processed.
  */
-export class BadRequestError extends Error {
-  readonly cause?: Error;
-
+export class BadRequestError extends CustomErrorBase {
   constructor(message?: string, cause?: Error) {
-    super(message);
-    Object.setPrototypeOf(this, BadRequestError.prototype);
-    Error.captureStackTrace(this, BadRequestError);
-    this.name = this.constructor.name;
-    this.cause = cause;
+    super(BadRequestError, message, cause);
   }
 }
 
 /**
  * The request requires authentication, which was not properly supplied.
  */
-export class UnauthenticatedError extends Error {
-  readonly cause?: Error;
-
+export class UnauthenticatedError extends CustomErrorBase {
   constructor(message?: string, cause?: Error) {
-    super(message);
-    Object.setPrototypeOf(this, UnauthenticatedError.prototype);
-    Error.captureStackTrace(this, UnauthenticatedError);
-    this.name = this.constructor.name;
-    this.cause = cause;
+    super(UnauthenticatedError, message, cause);
   }
 }
 
 /**
  * The authenticated caller is not permitted to perform this request.
  */
-export class ForbiddenError extends Error {
-  readonly cause?: Error;
-
+export class ForbiddenError extends CustomErrorBase {
   constructor(message?: string, cause?: Error) {
-    super(message);
-    Object.setPrototypeOf(this, ForbiddenError.prototype);
-    Error.captureStackTrace(this, ForbiddenError);
-    this.name = this.constructor.name;
-    this.cause = cause;
+    super(ForbiddenError, message, cause);
   }
 }
 
@@ -76,15 +80,9 @@ export class ForbiddenError extends Error {
  * Note that this error usually is used to indicate that an entity with a given
  * ID does not exist, rather than signalling that an entire route is missing.
  */
-export class NotFoundError extends Error {
-  readonly cause?: Error;
-
+export class NotFoundError extends CustomErrorBase {
   constructor(message?: string, cause?: Error) {
-    super(message);
-    Object.setPrototypeOf(this, NotFoundError.prototype);
-    Error.captureStackTrace(this, NotFoundError);
-    this.name = this.constructor.name;
-    this.cause = cause;
+    super(NotFoundError, message, cause);
   }
 }
 
@@ -92,16 +90,8 @@ export class NotFoundError extends Error {
  * The request could not complete due to a conflict in the current state of the
  * resource.
  */
-export class ConflictError extends Error {
-  readonly cause?: Error;
-
+export class ConflictError extends CustomErrorBase {
   constructor(message?: string, cause?: Error) {
-    super(message);
-    Object.setPrototypeOf(this, ConflictError.prototype);
-    if (Error.captureStackTrace) {
-      Error.captureStackTrace(this, ConflictError);
-    }
-    this.name = this.constructor.name;
-    this.cause = cause;
+    super(ConflictError, message, cause);
   }
 }

From 129991fcfd4e2faf66ae1b3f334fcf39d0378bf5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Mon, 4 May 2020 16:34:17 +0200
Subject: [PATCH 4/5] Even denser errors

---
 packages/backend-common/src/errors.ts         | 60 +++++++------------
 .../src/middleware/errorHandler.test.ts       | 12 ++--
 .../src/middleware/errorHandler.ts            |  4 +-
 packages/backend-common/tsconfig.json         |  2 +-
 4 files changed, 29 insertions(+), 49 deletions(-)

diff --git a/packages/backend-common/src/errors.ts b/packages/backend-common/src/errors.ts
index c3cf53453e..34a605a576 100644
--- a/packages/backend-common/src/errors.ts
+++ b/packages/backend-common/src/errors.ts
@@ -28,51 +28,39 @@
 class CustomErrorBase extends Error {
   readonly cause?: Error;
 
-  constructor(constructor: Function, message?: string, cause?: Error) {
-    super(message);
-    Object.setPrototypeOf(this, constructor.prototype);
-    Error.captureStackTrace(this, constructor);
-    this.name = this.constructor.name;
-    this.cause = cause;
-  }
-
-  toString() {
-    let result = super.toString();
-
-    if (this.cause) {
-      result += `; caused by ${this.cause.toString()}`;
+  constructor(message?: string, cause?: Error) {
+    let fullMessage = message;
+    if (cause) {
+      if (fullMessage) {
+        fullMessage += `; caused by ${cause}`;
+      } else {
+        fullMessage = `caused by ${cause}`;
+      }
     }
 
-    return result;
+    super(fullMessage);
+
+    Error.captureStackTrace(this, this.constructor);
+
+    this.name = this.constructor.name;
+    this.cause = cause;
   }
 }
 
 /**
  * The request is malformed and cannot be processed.
  */
-export class BadRequestError extends CustomErrorBase {
-  constructor(message?: string, cause?: Error) {
-    super(BadRequestError, message, cause);
-  }
-}
+export class BadRequestError extends CustomErrorBase {}
 
 /**
  * The request requires authentication, which was not properly supplied.
  */
-export class UnauthenticatedError extends CustomErrorBase {
-  constructor(message?: string, cause?: Error) {
-    super(UnauthenticatedError, message, cause);
-  }
-}
+export class AuthenticationError extends CustomErrorBase {}
 
 /**
- * The authenticated caller is not permitted to perform this request.
+ * The authenticated caller is not allowed to perform this request.
  */
-export class ForbiddenError extends CustomErrorBase {
-  constructor(message?: string, cause?: Error) {
-    super(ForbiddenError, message, cause);
-  }
-}
+export class NotAllowedError extends CustomErrorBase {}
 
 /**
  * The requested resource could not be found.
@@ -80,18 +68,10 @@ export class ForbiddenError extends CustomErrorBase {
  * Note that this error usually is used to indicate that an entity with a given
  * ID does not exist, rather than signalling that an entire route is missing.
  */
-export class NotFoundError extends CustomErrorBase {
-  constructor(message?: string, cause?: Error) {
-    super(NotFoundError, message, cause);
-  }
-}
+export class NotFoundError extends CustomErrorBase {}
 
 /**
  * The request could not complete due to a conflict in the current state of the
  * resource.
  */
-export class ConflictError extends CustomErrorBase {
-  constructor(message?: string, cause?: Error) {
-    super(ConflictError, message, cause);
-  }
-}
+export class ConflictError extends CustomErrorBase {}
diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts
index 8f4a8cfea4..d282ccb99b 100644
--- a/packages/backend-common/src/middleware/errorHandler.test.ts
+++ b/packages/backend-common/src/middleware/errorHandler.test.ts
@@ -52,11 +52,11 @@ describe('errorHandler', () => {
     app.use('/BadRequestError', () => {
       throw new errors.BadRequestError();
     });
-    app.use('/UnauthenticatedError', () => {
-      throw new errors.UnauthenticatedError();
+    app.use('/AuthenticationError', () => {
+      throw new errors.AuthenticationError();
     });
-    app.use('/ForbiddenError', () => {
-      throw new errors.ForbiddenError();
+    app.use('/NotAllowedError', () => {
+      throw new errors.NotAllowedError();
     });
     app.use('/NotFoundError', () => {
       throw new errors.NotFoundError();
@@ -68,8 +68,8 @@ describe('errorHandler', () => {
 
     const r = request(app);
     expect((await r.get('/BadRequestError')).status).toBe(400);
-    expect((await r.get('/UnauthenticatedError')).status).toBe(401);
-    expect((await r.get('/ForbiddenError')).status).toBe(403);
+    expect((await r.get('/AuthenticationError')).status).toBe(401);
+    expect((await r.get('/NotAllowedError')).status).toBe(403);
     expect((await r.get('/NotFoundError')).status).toBe(404);
     expect((await r.get('/ConflictError')).status).toBe(409);
   });
diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts
index eb1b0597a7..43782e4d7f 100644
--- a/packages/backend-common/src/middleware/errorHandler.ts
+++ b/packages/backend-common/src/middleware/errorHandler.ts
@@ -81,9 +81,9 @@ function getStatusCode(error: Error): number {
   switch (error.name) {
     case errors.BadRequestError.name:
       return 400;
-    case errors.UnauthenticatedError.name:
+    case errors.AuthenticationError.name:
       return 401;
-    case errors.ForbiddenError.name:
+    case errors.NotAllowedError.name:
       return 403;
     case errors.NotFoundError.name:
       return 404;
diff --git a/packages/backend-common/tsconfig.json b/packages/backend-common/tsconfig.json
index b463ac102f..6d7ca21afa 100644
--- a/packages/backend-common/tsconfig.json
+++ b/packages/backend-common/tsconfig.json
@@ -7,7 +7,7 @@
     "sourceMap": true,
     "declaration": true,
     "strict": true,
-    "target": "es5",
+    "target": "ES2019",
     "module": "commonjs",
     "esModuleInterop": true,
     "types": ["node", "jest"]

From 9724084186b684053b76d8ab7393960f22545dd3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Mon, 4 May 2020 16:42:40 +0200
Subject: [PATCH 5/5] It's InputError then

---
 packages/backend-common/src/errors.ts                       | 4 ++--
 packages/backend-common/src/middleware/errorHandler.test.ts | 6 +++---
 packages/backend-common/src/middleware/errorHandler.ts      | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/backend-common/src/errors.ts b/packages/backend-common/src/errors.ts
index 34a605a576..b68dc8e3f0 100644
--- a/packages/backend-common/src/errors.ts
+++ b/packages/backend-common/src/errors.ts
@@ -48,9 +48,9 @@ class CustomErrorBase extends Error {
 }
 
 /**
- * The request is malformed and cannot be processed.
+ * The given inputs are malformed and cannot be processed.
  */
-export class BadRequestError extends CustomErrorBase {}
+export class InputError extends CustomErrorBase {}
 
 /**
  * The request requires authentication, which was not properly supplied.
diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts
index d282ccb99b..022802dff8 100644
--- a/packages/backend-common/src/middleware/errorHandler.test.ts
+++ b/packages/backend-common/src/middleware/errorHandler.test.ts
@@ -49,8 +49,8 @@ describe('errorHandler', () => {
 
   it('handles well-known error classes', async () => {
     const app = express();
-    app.use('/BadRequestError', () => {
-      throw new errors.BadRequestError();
+    app.use('/InputError', () => {
+      throw new errors.InputError();
     });
     app.use('/AuthenticationError', () => {
       throw new errors.AuthenticationError();
@@ -67,7 +67,7 @@ describe('errorHandler', () => {
     app.use(errorHandler());
 
     const r = request(app);
-    expect((await r.get('/BadRequestError')).status).toBe(400);
+    expect((await r.get('/InputError')).status).toBe(400);
     expect((await r.get('/AuthenticationError')).status).toBe(401);
     expect((await r.get('/NotAllowedError')).status).toBe(403);
     expect((await r.get('/NotFoundError')).status).toBe(404);
diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts
index 43782e4d7f..14f6cfa8d7 100644
--- a/packages/backend-common/src/middleware/errorHandler.ts
+++ b/packages/backend-common/src/middleware/errorHandler.ts
@@ -79,7 +79,7 @@ function getStatusCode(error: Error): number {
 
   // Handle well-known error types
   switch (error.name) {
-    case errors.BadRequestError.name:
+    case errors.InputError.name:
       return 400;
     case errors.AuthenticationError.name:
       return 401;