paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 50 Packages Projects Releases Wiki Activity

Merge pull request #709 from spotify/freben/errors

Browse Source 
[backend] use common error types
This commit is contained in:
Fredrik Adelöw
2020-05-04 16:49:35 +02:00
committed by GitHub
parent 70e2a62155 9724084186
commit 1d1589680d
11 changed files with 196 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/backend-common/src/errors.ts
+77
View File
@@ -0,0 +1,77 @@
/*
* 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.
*/
class CustomErrorBase extends Error {
readonly cause?: Error;
constructor(message?: string, cause?: Error) {
let fullMessage = message;
if (cause) {
if (fullMessage) {
fullMessage += `; caused by ${cause}`;
} else {
fullMessage = `caused by ${cause}`;
}
}
super(fullMessage);
Error.captureStackTrace(this, this.constructor);
this.name = this.constructor.name;
this.cause = cause;
}
}
/**
* The given inputs are malformed and cannot be processed.
*/
export class InputError extends CustomErrorBase {}
/**
* The request requires authentication, which was not properly supplied.
*/
export class AuthenticationError extends CustomErrorBase {}
/**
* The authenticated caller is not allowed to perform this request.
*/
export class NotAllowedError extends CustomErrorBase {}
/**
* 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 CustomErrorBase {}
/**
* The request could not complete due to a conflict in the current state of the
* resource.
*/
export class ConflictError extends CustomErrorBase {}
packages/backend-common/src/index.ts
+1
View File
@@ -14,5 +14,6 @@
* limitations under the License.
*/
export * from './errors';
export * from './logging';
export * from './middleware';
packages/backend-common/src/middleware/errorHandler.test.ts
+29 -1
View File
@@ -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('/InputError', () => {
throw new errors.InputError();
});
app.use('/AuthenticationError', () => {
throw new errors.AuthenticationError();
});
app.use('/NotAllowedError', () => {
throw new errors.NotAllowedError();
});
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('/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);
expect((await r.get('/ConflictError')).status).toBe(409);
});
});
packages/backend-common/src/middleware/errorHandler.ts
+41 -6
View File
@@ -15,12 +15,21 @@
*/
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.
*
* 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
@@ -30,23 +39,32 @@ import { ErrorRequestHandler, NextFunction, Request, Response } from 'express';
*
* @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,
_request: Request,
response: Response,
_next: NextFunction,
next: NextFunction,
) => {
if (response.headersSent) {
next(error);
}
const status = getStatusCode(error);
const message = error.message;
const message = showStackTraces ? error.stack : error.message;
response.status(status).send(message);
};
}
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 +77,22 @@ function getStatusCode(error: Error): number {
}
}
// Handle well-known error types
switch (error.name) {
case errors.InputError.name:
return 400;
case errors.AuthenticationError.name:
return 401;
case errors.NotAllowedError.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;
}
packages/backend-common/tsconfig.json
+1 -1
View File
@@ -7,7 +7,7 @@
"sourceMap": true,
"declaration": true,
"strict": true,
"target": "es5",
"target": "ES2019",
"module": "commonjs",
"esModuleInterop": true,
"types": ["node", "jest"]
plugins/inventory-backend/package.json
+2 -1
View File
@@ -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",
@@ -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"
plugins/inventory-backend/src/inventory/AggregatorInventory.ts
+16 -6
View File
@@ -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) {
plugins/inventory-backend/src/inventory/StaticInventory.ts
+9 -4
View File
@@ -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;
}
}
plugins/inventory-backend/src/inventory/types.ts
+2 -2
View File
@@ -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>;
};
plugins/inventory-backend/src/service/router.ts
+3 -6
View File
@@ -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();
yarn.lock
+15 -1
View File
@@ -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=