Introduce the @backstage/errors package.

Encode thrown errors in the backend as a JSON payload using a facility in that package, and render helpful frontend displays of those errors.

Signed-off-by: Fredrik Adelöw <[email protected]>

.changeset/chilled-kiwis-bathe.md

@@ -0,0 +1,5 @@
+---
+'@backstage/catalog-client': patch
+---
+
+Throw the new `ServerResponseError` from `@backstage/errors`

.changeset/healthy-cars-do.md

@@ -0,0 +1,29 @@
+---
+'@backstage/backend-common': minor
+---
+
+Encode thrown errors in the backend as a JSON payload. This is technically a breaking change, since the response format even of errors are part of the contract. If you relied on the response being text, you will now have some extra JSON "noise" in it. It should still be readable by end users though.
+
+Before:
+
+```
+NotFoundError: No entity named 'tara.macgovern2' found, with kind 'user' in namespace 'default'
+    at eval (webpack-internal:///../../plugins/catalog-backend/src/service/router.ts:117:17)
+```
+
+After:
+
+```json
+{
+  "error": {
+    "statusCode": 404,
+    "name": "NotFoundError",
+    "message": "No entity named 'tara.macgovern2' found, with kind 'user' in namespace 'default'",
+    "stack": "NotFoundError: No entity named 'tara.macgovern2' found, with kind 'user' in namespace 'default'\n    at eval (webpack-internal:///../../plugins/catalog-backend/src/service/router.ts:117:17)"
+  },
+  "request": {
+    "method": "GET",
+    "url": "/entities/by-name/user/default/tara.macgovern2"
+  }
+}
+```

.changeset/olive-apples-shop.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core': patch
+---
+
+Add a `ErrorResponsePanel` to render `ServerResponseError` from `@backstage/errors`

.changeset/silver-weeks-hide.md

@@ -0,0 +1,10 @@
+---
+'@backstage/techdocs-common': patch
+'@backstage/plugin-auth-backend': patch
+'@backstage/plugin-catalog': patch
+'@backstage/plugin-catalog-backend': patch
+'@backstage/plugin-scaffolder-backend': patch
+'@backstage/plugin-techdocs-backend': patch
+---
+
+Use errors from `@backstage/errors`

.changeset/young-suits-sit.md

@@ -0,0 +1,10 @@
+---
+'@backstage/backend-common': minor
+---
+
+Removed the custom error types (e.g. `NotFoundError`). Those are now instead in the new `@backstage/errors` package. This is a breaking change, and you will have to update your imports if you were using these types.
+
+```diff
+-import { NotFoundError } from '@backstage/backend-common';
++import { NotFoundError } from '@backstage/errors';
+```

packages/backend-common/package.json

@@ -32,6 +32,7 @@
     "@backstage/cli-common": "^0.1.1",
     "@backstage/config": "^0.1.2",
     "@backstage/config-loader": "^0.5.1",
+    "@backstage/errors": "^0.1.1",
     "@backstage/integration": "^0.5.1",
     "@octokit/rest": "^18.0.12",
     "@types/cors": "^2.8.6",

packages/backend-common/src/index.ts

@@ -17,7 +17,6 @@
 export * from './config';
 export * from './database';
 export * from './discovery';
-export * from './errors';
 export * from './hot';
 export * from './logging';
 export * from './middleware';

packages/backend-common/src/middleware/errorHandler.test.ts

@@ -14,10 +14,17 @@
  * limitations under the License.
  */
 
+import {
+  AuthenticationError,
+  ConflictError,
+  InputError,
+  NotAllowedError,
+  NotFoundError,
+  NotModifiedError,
+} from '@backstage/errors';
 import express from 'express';
 import createError from 'http-errors';
 import request from 'supertest';
-import * as errors from '../errors';
 import { errorHandler } from './errorHandler';
 
 describe('errorHandler', () => {
@@ -31,17 +38,27 @@ describe('errorHandler', () => {
     const response = await request(app).get('/breaks');
 
     expect(response.status).toBe(500);
-    expect(response.text).toBe('some message');
+    expect(response.body).toEqual({
+      error: {
+        statusCode: 500,
+        name: 'Error',
+        message: 'some message',
+      },
+      request: {
+        method: 'GET',
+        url: '/breaks',
+      },
+    });
   });
 
-  it('doesnt try to send the response again if its already been sent', async () => {
+  it('does not try to send the response again if its already been sent', async () => {
     const app = express();
     const mockSend = jest.fn();
 
     app.use('/works_with_async_fail', (_, res) => {
       res.status(200).send('hello');
 
-      // mutate the response object to test the middlware.
+      // mutate the response object to test the middleware.
       // it's hard to catch errors inside middleware from the outside.
       // @ts-ignore
       res.send = mockSend;
@@ -67,38 +84,61 @@ describe('errorHandler', () => {
     const response = await request(app).get('/breaks');
 
     expect(response.status).toBe(432);
-    expect(response.text).toContain('Some Message');
+    expect(response.body).toEqual({
+      error: {
+        statusCode: 432,
+        name: 'BadRequestError',
+        message: 'Some Message',
+      },
+      request: {
+        method: 'GET',
+        url: '/breaks',
+      },
+    });
   });
 
   it('handles well-known error classes', async () => {
     const app = express();
     app.use('/NotModifiedError', () => {
-      throw new errors.NotModifiedError();
+      throw new NotModifiedError();
     });
     app.use('/InputError', () => {
-      throw new errors.InputError();
+      throw new InputError();
     });
     app.use('/AuthenticationError', () => {
-      throw new errors.AuthenticationError();
+      throw new AuthenticationError();
     });
     app.use('/NotAllowedError', () => {
-      throw new errors.NotAllowedError();
+      throw new NotAllowedError();
     });
     app.use('/NotFoundError', () => {
-      throw new errors.NotFoundError();
+      throw new NotFoundError();
     });
     app.use('/ConflictError', () => {
-      throw new errors.ConflictError();
+      throw new ConflictError();
     });
     app.use(errorHandler());
 
     const r = request(app);
     expect((await r.get('/NotModifiedError')).status).toBe(304);
     expect((await r.get('/InputError')).status).toBe(400);
+    expect((await r.get('/InputError')).body.error.name).toBe('InputError');
     expect((await r.get('/AuthenticationError')).status).toBe(401);
+    expect((await r.get('/AuthenticationError')).body.error.name).toBe(
+      'AuthenticationError',
+    );
     expect((await r.get('/NotAllowedError')).status).toBe(403);
+    expect((await r.get('/NotAllowedError')).body.error.name).toBe(
+      'NotAllowedError',
+    );
     expect((await r.get('/NotFoundError')).status).toBe(404);
+    expect((await r.get('/NotFoundError')).body.error.name).toBe(
+      'NotFoundError',
+    );
     expect((await r.get('/ConflictError')).status).toBe(409);
+    expect((await r.get('/ConflictError')).body.error.name).toBe(
+      'ConflictError',
+    );
   });
 
   it('logs all 500 errors', async () => {
@@ -126,7 +166,7 @@ describe('errorHandler', () => {
     mockLogger.child.mockImplementation(() => mockLogger as any);
 
     app.use('/NotFound', () => {
-      throw new errors.NotFoundError();
+      throw new NotFoundError();
     });
     app.use(errorHandler({ logger: mockLogger as any }));
 
@@ -142,7 +182,7 @@ describe('errorHandler', () => {
     mockLogger.child.mockImplementation(() => mockLogger as any);
 
     app.use('/NotFound', () => {
-      throw new errors.NotFoundError();
+      throw new NotFoundError();
     });
     app.use(errorHandler({ logger: mockLogger as any, logClientErrors: true }));
 

packages/backend-common/src/middleware/errorHandler.ts

@@ -16,7 +16,15 @@
 
 import { ErrorRequestHandler, NextFunction, Request, Response } from 'express';
 import { Logger } from 'winston';
-import * as errors from '../errors';
+import {
+  NotModifiedError,
+  InputError,
+  AuthenticationError,
+  NotAllowedError,
+  NotFoundError,
+  ConflictError,
+  ServerResponseErrorBody,
+} from '@backstage/errors';
 import { getRootLogger } from '../logging';
 
 export type ErrorHandlerOptions = {
@@ -48,14 +56,13 @@ export type ErrorHandlerOptions = {
  * 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
+ * but rather to be a global catch-all for uncaught "fatal" errors that are
  * expected to result in a 500 error. However, it also does handle some common
  * error types (such as http-error exceptions) and returns the enclosed status
  * code accordingly.
  *
  * @returns An Express error request handler
  */
-
 export function errorHandler(
   options: ErrorHandlerOptions = {},
 ): ErrorRequestHandler {
@@ -66,29 +73,33 @@ export function errorHandler(
     type: 'errorHandler',
   });
 
-  return (
-    error: Error,
-    _request: Request,
-    res: Response,
-    next: NextFunction,
-  ) => {
+  return (error: Error, req: Request, res: Response, next: NextFunction) => {
+    const statusCode = getStatusCode(error);
+    if (options.logClientErrors || statusCode >= 500) {
+      logger.error(error);
+    }
+
     if (res.headersSent) {
       // If the headers have already been sent, do not send the response again
       // as this will throw an error in the backend.
       next(error);
       return;
     }
 
-    const status = getStatusCode(error);
-    const message = showStackTraces ? error.stack : error.message;
-
-    if (options.logClientErrors || status >= 500) {
-      logger.error(error);
-    }
+    const body: ServerResponseErrorBody = {
+      error: {
+        statusCode,
+        name: error.name || 'Error',
+        message: error.message || '<no reason given>',
+        stack: showStackTraces ? error.stack : undefined,
+      },
+      request: {
+        method: req.method,
+        url: req.url,
+      },
+    };
 
-    res.status(status);
-    res.setHeader('content-type', 'text/plain');
-    res.send(message);
+    res.status(statusCode).json(body);
   };
 }
 
@@ -109,17 +120,17 @@ function getStatusCode(error: Error): number {
 
   // Handle well-known error types
   switch (error.name) {
-    case errors.NotModifiedError.name:
+    case NotModifiedError.name:
       return 304;
-    case errors.InputError.name:
+    case InputError.name:
       return 400;
-    case errors.AuthenticationError.name:
+    case AuthenticationError.name:
       return 401;
-    case errors.NotAllowedError.name:
+    case NotAllowedError.name:
       return 403;
-    case errors.NotFoundError.name:
+    case NotFoundError.name:
       return 404;
-    case errors.ConflictError.name:
+    case ConflictError.name:
       return 409;
     default:
       break;

packages/backend-common/src/reading/AzureUrlReader.test.ts

@@ -26,7 +26,7 @@ import { rest } from 'msw';
 import { setupServer } from 'msw/node';
 import * as os from 'os';
 import path from 'path';
-import { NotModifiedError } from '../errors';
+import { NotModifiedError } from '@backstage/errors';
 import { getVoidLogger } from '../logging';
 import { AzureUrlReader } from './AzureUrlReader';
 import { ReadTreeResponseFactory } from './tree';

packages/backend-common/src/reading/AzureUrlReader.ts

@@ -26,7 +26,7 @@ import fetch from 'cross-fetch';
 import parseGitUrl from 'git-url-parse';
 import { Minimatch } from 'minimatch';
 import { Readable } from 'stream';
-import { NotFoundError, NotModifiedError } from '../errors';
+import { NotFoundError, NotModifiedError } from '@backstage/errors';
 import { ReadTreeResponseFactory } from './tree';
 import { stripFirstDirectoryFromPath } from './tree/util';
 import {

packages/backend-common/src/reading/BitbucketUrlReader.test.ts

@@ -26,7 +26,7 @@ import { rest } from 'msw';
 import { setupServer } from 'msw/node';
 import os from 'os';
 import path from 'path';
-import { NotModifiedError } from '../errors';
+import { NotModifiedError } from '@backstage/errors';
 import { BitbucketUrlReader } from './BitbucketUrlReader';
 import { ReadTreeResponseFactory } from './tree';
 

packages/backend-common/src/reading/BitbucketUrlReader.ts

@@ -26,7 +26,7 @@ import fetch from 'cross-fetch';
 import parseGitUrl from 'git-url-parse';
 import { Minimatch } from 'minimatch';
 import { Readable } from 'stream';
-import { NotFoundError, NotModifiedError } from '../errors';
+import { NotFoundError, NotModifiedError } from '@backstage/errors';
 import { ReadTreeResponseFactory } from './tree';
 import { stripFirstDirectoryFromPath } from './tree/util';
 import {

packages/backend-common/src/reading/FetchUrlReader.ts

@@ -15,7 +15,7 @@
  */
 
 import fetch from 'cross-fetch';
-import { NotFoundError } from '../errors';
+import { NotFoundError } from '@backstage/errors';
 import {
   ReaderFactory,
   ReadTreeResponse,

packages/backend-common/src/reading/GithubUrlReader.test.ts

@@ -27,7 +27,7 @@ import { rest } from 'msw';
 import { setupServer } from 'msw/node';
 import os from 'os';
 import path from 'path';
-import { NotFoundError, NotModifiedError } from '../errors';
+import { NotFoundError, NotModifiedError } from '@backstage/errors';
 import {
   GhBlobResponse,
   GhBranchResponse,

packages/backend-common/src/reading/GithubUrlReader.ts

@@ -25,7 +25,7 @@ import fetch from 'cross-fetch';
 import parseGitUrl from 'git-url-parse';
 import { Minimatch } from 'minimatch';
 import { Readable } from 'stream';
-import { NotFoundError, NotModifiedError } from '../errors';
+import { NotFoundError, NotModifiedError } from '@backstage/errors';
 import { ReadTreeResponseFactory } from './tree';
 import {
   ReaderFactory,

packages/backend-common/src/reading/GitlabUrlReader.test.ts

@@ -25,7 +25,7 @@ import path from 'path';
 import { getVoidLogger } from '../logging';
 import { GitlabUrlReader } from './GitlabUrlReader';
 import { ReadTreeResponseFactory } from './tree';
-import { NotModifiedError, NotFoundError } from '../errors';
+import { NotModifiedError, NotFoundError } from '@backstage/errors';
 import {
   GitLabIntegration,
   readGitLabIntegrationConfig,