diff --git a/.changeset/chilled-kiwis-bathe.md b/.changeset/chilled-kiwis-bathe.md index 31e891a8fc..ef1517a58c 100644 --- a/.changeset/chilled-kiwis-bathe.md +++ b/.changeset/chilled-kiwis-bathe.md @@ -2,4 +2,4 @@ '@backstage/catalog-client': patch --- -Throw the new `ServerResponseError` from `@backstage/errors` +Throw the new `ResponseError` from `@backstage/errors` diff --git a/.changeset/healthy-cars-do.md b/.changeset/healthy-cars-do.md index 8de1981709..7f3b874ba3 100644 --- a/.changeset/healthy-cars-do.md +++ b/.changeset/healthy-cars-do.md @@ -16,7 +16,6 @@ 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)" @@ -24,6 +23,9 @@ After: "request": { "method": "GET", "url": "/entities/by-name/user/default/tara.macgovern2" + }, + "response": { + "statusCode": 404 } } ``` diff --git a/.changeset/olive-apples-shop.md b/.changeset/olive-apples-shop.md index 160f72af5f..754c926780 100644 --- a/.changeset/olive-apples-shop.md +++ b/.changeset/olive-apples-shop.md @@ -2,4 +2,4 @@ '@backstage/core': patch --- -Add a `ErrorResponsePanel` to render `ServerResponseError` from `@backstage/errors` +Add a `ResponseErrorPanel` to render `ResponseError` from `@backstage/errors` diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts index 50af586686..3fb7aedc37 100644 --- a/packages/backend-common/src/middleware/errorHandler.test.ts +++ b/packages/backend-common/src/middleware/errorHandler.test.ts @@ -39,15 +39,12 @@ describe('errorHandler', () => { expect(response.status).toBe(500); expect(response.body).toEqual({ - error: { - statusCode: 500, + error: expect.objectContaining({ name: 'Error', message: 'some message', - }, - request: { - method: 'GET', - url: '/breaks', - }, + }), + request: { method: 'GET', url: '/breaks' }, + response: { statusCode: 500 }, }); }); @@ -86,14 +83,17 @@ describe('errorHandler', () => { expect(response.status).toBe(432); expect(response.body).toEqual({ error: { - statusCode: 432, + expose: true, name: 'BadRequestError', message: 'Some Message', + status: 432, + statusCode: 432, }, request: { method: 'GET', url: '/breaks', }, + response: { statusCode: 432 }, }); }); diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts index 4dc6d4c85f..bee3f5557d 100644 --- a/packages/backend-common/src/middleware/errorHandler.ts +++ b/packages/backend-common/src/middleware/errorHandler.ts @@ -14,17 +14,18 @@ * limitations under the License. */ -import { ErrorRequestHandler, NextFunction, Request, Response } from 'express'; -import { Logger } from 'winston'; import { - NotModifiedError, - InputError, AuthenticationError, + ConflictError, + ErrorResponse, + InputError, NotAllowedError, NotFoundError, - ConflictError, - ServerResponseErrorBody, + NotModifiedError, + serializeError, } from '@backstage/errors'; +import { ErrorRequestHandler, NextFunction, Request, Response } from 'express'; +import { Logger } from 'winston'; import { getRootLogger } from '../logging'; export type ErrorHandlerOptions = { @@ -86,17 +87,10 @@ export function errorHandler( return; } - const body: ServerResponseErrorBody = { - error: { - statusCode, - name: error.name || 'Error', - message: error.message || '', - stack: showStackTraces ? error.stack : undefined, - }, - request: { - method: req.method, - url: req.url, - }, + const body: ErrorResponse = { + error: serializeError(error, { includeStack: showStackTraces }), + request: { method: req.method, url: req.url }, + response: { statusCode }, }; res.status(statusCode).json(body); diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 2d9a07a7a8..60e15906d3 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -21,7 +21,7 @@ import { LOCATION_ANNOTATION, stringifyLocationReference, } from '@backstage/catalog-model'; -import { ServerResponseError } from '@backstage/errors'; +import { ResponseError } from '@backstage/errors'; import fetch from 'cross-fetch'; import { AddLocationRequest, @@ -154,7 +154,7 @@ export class CatalogClient implements CatalogApi { }, ); if (!response.ok) { - throw await ServerResponseError.forResponse(response); + throw await ResponseError.fromResponse(response); } return undefined; } @@ -175,7 +175,7 @@ export class CatalogClient implements CatalogApi { }); if (!response.ok) { - throw await ServerResponseError.forResponse(response); + throw await ResponseError.fromResponse(response); } return await response.json(); @@ -196,7 +196,7 @@ export class CatalogClient implements CatalogApi { if (response.status === 404) { return undefined; } - throw await ServerResponseError.forResponse(response); + throw await ResponseError.fromResponse(response); } return await response.json(); diff --git a/packages/core/src/components/ErrorResponse/ErrorResponse.tsx b/packages/core/src/components/ErrorResponse/ErrorResponse.tsx deleted file mode 100644 index 2a3c193d6b..0000000000 --- a/packages/core/src/components/ErrorResponse/ErrorResponse.tsx +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright 2021 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. - */ - -import { ServerResponseError } from '@backstage/errors'; -import { - Divider, - List, - ListItem, - ListItemText, - makeStyles, -} from '@material-ui/core'; -import React from 'react'; -import { CodeSnippet } from '../CodeSnippet'; -import { CopyTextButton } from '../CopyTextButton'; -import { WarningPanel } from '../WarningPanel'; - -const useStyles = makeStyles(theme => ({ - text: { - fontFamily: 'monospace', - whiteSpace: 'pre', - overflowX: 'auto', - marginRight: theme.spacing(2), - }, - divider: { - margin: theme.spacing(2), - }, -})); - -type Props = { - error: Error; -}; - -/** - * Renders details about a failed server request. - * - * Has special treatment for ServerResponseError errors, to display rich - * server-provided information about what happened. - */ -export const ErrorResponse = ({ error }: Props) => { - const classes = useStyles(); - - if (error.name !== 'ServerResponseError') { - return ( - - - - ); - } - - const { body } = error as ServerResponseError; - const errorString = `${body.error.statusCode}: ${body.error.name}`; - const request = body.request && `${body.request.method} ${body.request.url}`; - const message = body.error.message.replace(/\\n/g, '\n'); - const stack = body.error.stack?.replace(/\\n/g, '\n'); - const json = JSON.stringify(error, undefined, 2); - - return ( - <> - - - - - - - - - - {request ? ( - - - - - ) : null} - {stack ? ( - - - - - ) : null} - - - } - /> - - - - - ); -}; - -/** - * Renders a warning panel as the effect of a failed server request. - * - * Has special treatment for ServerResponseError errors, to display rich - * server-provided information about what happened. - */ -export const ErrorResponsePanel = ({ error }: Props) => { - return ( - - - - ); -}; diff --git a/packages/core/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx b/packages/core/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx new file mode 100644 index 0000000000..c36be78f08 --- /dev/null +++ b/packages/core/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx @@ -0,0 +1,135 @@ +/* + * Copyright 2021 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. + */ + +import { ResponseError } from '@backstage/errors'; +import { + Divider, + List, + ListItem, + ListItemText, + makeStyles, +} from '@material-ui/core'; +import React from 'react'; +import { CodeSnippet } from '../CodeSnippet'; +import { CopyTextButton } from '../CopyTextButton'; +import { WarningPanel } from '../WarningPanel'; + +const useStyles = makeStyles(theme => ({ + text: { + fontFamily: 'monospace', + whiteSpace: 'pre', + overflowX: 'auto', + marginRight: theme.spacing(2), + }, + divider: { + margin: theme.spacing(2), + }, +})); + +type Props = { + error: Error; +}; + +/** + * Renders details about a failed server request. + * + * Has special treatment for ResponseError errors, to display rich + * server-provided information about what happened. + */ +export const ResponseErrorDetails = ({ error }: Props) => { + const classes = useStyles(); + + if (error.name !== 'ResponseError') { + return ( + + + + ); + } + + const { data, cause } = error as ResponseError; + const { request, response } = data; + + const errorString = `${response.statusCode}: ${cause.name}`; + const requestString = request && `${request.method} ${request.url}`; + const messageString = cause.message.replace(/\\n/g, '\n'); + const stackString = cause.stack?.replace(/\\n/g, '\n'); + const jsonString = JSON.stringify(data, undefined, 2); + + return ( + + + + + + + + + + {requestString && ( + + + + + )} + {stackString && ( + + + + + )} + + + } + /> + + + + ); +}; + +/** + * Renders a warning panel as the effect of a failed server request. + * + * Has special treatment for ResponseError errors, to display rich + * server-provided information about what happened. + */ +export const ResponseErrorPanel = ({ error }: Props) => { + return ( + + + + ); +}; diff --git a/packages/core/src/components/ErrorResponse/index.ts b/packages/core/src/components/ResponseErrorPanel/index.ts similarity index 88% rename from packages/core/src/components/ErrorResponse/index.ts rename to packages/core/src/components/ResponseErrorPanel/index.ts index 24463ddb37..ac62553fd5 100644 --- a/packages/core/src/components/ErrorResponse/index.ts +++ b/packages/core/src/components/ResponseErrorPanel/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { ErrorResponse, ErrorResponsePanel } from './ErrorResponse'; +export { ResponseErrorDetails, ResponseErrorPanel } from './ResponseErrorPanel'; diff --git a/packages/core/src/components/index.ts b/packages/core/src/components/index.ts index 35cd4e5b2e..c8797f576b 100644 --- a/packages/core/src/components/index.ts +++ b/packages/core/src/components/index.ts @@ -22,7 +22,7 @@ export * from './CopyTextButton'; export * from './DependencyGraph'; export * from './DismissableBanner'; export * from './EmptyState'; -export * from './ErrorResponse'; +export * from './ResponseErrorPanel'; export * from './FeatureDiscovery'; export * from './HeaderIconLinkRow'; export * from './HorizontalScrollGrid'; diff --git a/packages/errors/package.json b/packages/errors/package.json index cbc7098cb3..508cf4fab0 100644 --- a/packages/errors/package.json +++ b/packages/errors/package.json @@ -30,7 +30,8 @@ }, "dependencies": { "@backstage/config": "^0.1.2", - "cross-fetch": "^3.0.6" + "cross-fetch": "^3.0.6", + "serialize-error": "^8.0.1" }, "devDependencies": { "@backstage/cli": "^0.6.1", diff --git a/packages/errors/src/error/server.ts b/packages/errors/src/error/server.ts deleted file mode 100644 index a7b89c3baa..0000000000 --- a/packages/errors/src/error/server.ts +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2021 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. - */ - -import { - parseServerResponseErrorBody, - ServerResponseErrorBody, -} from '../payload'; - -/** - * An error thrown as the result of a failed server request. - * - * The server is expected to respond on the ServerResponseErrorBody format. - */ -export class ServerResponseError extends Error { - static async forResponse(response: Response): Promise { - const body = await parseServerResponseErrorBody(response); - const status = body.error.statusCode || response.status; - const statusText = body.error.name || response.statusText; - const message = `Request failed with ${status} ${statusText}`; - return new ServerResponseError(message, body); - } - - readonly body: ServerResponseErrorBody; - - constructor(message: string, body: ServerResponseErrorBody) { - super(message); - this.name = 'ServerResponseError'; - this.body = body; - } -} diff --git a/packages/errors/src/error/CustomErrorBase.ts b/packages/errors/src/errors/CustomErrorBase.ts similarity index 100% rename from packages/errors/src/error/CustomErrorBase.ts rename to packages/errors/src/errors/CustomErrorBase.ts diff --git a/packages/errors/src/errors/ResponseError.test.ts b/packages/errors/src/errors/ResponseError.test.ts new file mode 100644 index 0000000000..a67fb8bbfa --- /dev/null +++ b/packages/errors/src/errors/ResponseError.test.ts @@ -0,0 +1,42 @@ +/* + * Copyright 2021 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. + */ + +import { ErrorResponse } from '../serialization'; +import { ResponseError } from './ResponseError'; + +describe('ResponseError', () => { + it('constructs itself from a response', async () => { + const body: ErrorResponse = { + error: { name: 'Fours', message: 'Expected fives', stack: 'lines' }, + request: { method: 'GET', url: '/' }, + response: { statusCode: 444 }, + }; + + const response: Partial = { + status: 444, + statusText: 'Fours', + text: async () => JSON.stringify(body), + headers: new Headers({ 'Content-Type': 'application/json' }), + }; + + const e = await ResponseError.fromResponse(response as Response); + expect(e.name).toEqual('ResponseError'); + expect(e.message).toEqual('Request failed with 444 Fours'); + expect(e.cause.name).toEqual('Fours'); + expect(e.cause.message).toEqual('Expected fives'); + expect(e.cause.stack).toEqual('lines'); + }); +}); diff --git a/packages/errors/src/errors/ResponseError.ts b/packages/errors/src/errors/ResponseError.ts new file mode 100644 index 0000000000..0fff8afa66 --- /dev/null +++ b/packages/errors/src/errors/ResponseError.ts @@ -0,0 +1,88 @@ +/* + * Copyright 2021 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. + */ + +import { + parseErrorResponse, + ErrorResponse, + deserializeError, +} from '../serialization'; + +/** + * An error thrown as the result of a failed server request. + * + * The server is expected to respond on the ErrorResponse format. + */ +export class ResponseError extends Error { + /** + * The actual response, as seen by the client. + * + * Note that the body of this response is always consumed. Its parsed form is + * in the `body` field. + */ + readonly response: Response; + + /** + * The parsed JSON error body, as sent by the server. + */ + readonly data: ErrorResponse; + + /** + * The Error cause, as seen by the remote server. This is parsed out of the + * JSON error body. + * + * This error always has the plain Error constructor, however all + * serializable enumerable fields on the remote error including its name are + * preserved. Therefore, if you want to check the error type, use its name + * property rather than checking typeof or its constructor or prototype. + */ + readonly cause: Error; + + /** + * Constructs a ResponseError based on a failed response. + * + * Assumes that the response has already been checked to be not ok. This + * function consumes the body of the response, and assumes that it hasn't + * been consumed before. + */ + static async fromResponse(response: Response): Promise { + const data = await parseErrorResponse(response); + + const status = data.response.statusCode || response.status; + const statusText = data.error.name || response.statusText; + const message = `Request failed with ${status} ${statusText}`; + const cause = deserializeError(data.error); + + return new ResponseError({ + message, + response, + data, + cause, + }); + } + + constructor(props: { + message: string; + response: Response; + data: ErrorResponse; + cause: Error; + }) { + super(props.message); + this.name = 'ResponseError'; + this.response = props.response; + this.data = props.data; + this.cause = props.cause; + } +} diff --git a/packages/errors/src/error/common.test.ts b/packages/errors/src/errors/common.test.ts similarity index 100% rename from packages/errors/src/error/common.test.ts rename to packages/errors/src/errors/common.test.ts diff --git a/packages/errors/src/error/common.ts b/packages/errors/src/errors/common.ts similarity index 100% rename from packages/errors/src/error/common.ts rename to packages/errors/src/errors/common.ts diff --git a/packages/errors/src/error/index.ts b/packages/errors/src/errors/index.ts similarity index 87% rename from packages/errors/src/error/index.ts rename to packages/errors/src/errors/index.ts index 4fcac24004..f48aacf881 100644 --- a/packages/errors/src/error/index.ts +++ b/packages/errors/src/errors/index.ts @@ -15,11 +15,12 @@ */ export { - InputError, AuthenticationError, + ConflictError, + InputError, NotAllowedError, NotFoundError, - ConflictError, NotModifiedError, } from './common'; -export { ServerResponseError } from './server'; +export { CustomErrorBase } from './CustomErrorBase'; +export { ResponseError } from './ResponseError'; diff --git a/packages/errors/src/index.ts b/packages/errors/src/index.ts index 7a85cb2a81..a8d2142d88 100644 --- a/packages/errors/src/index.ts +++ b/packages/errors/src/index.ts @@ -14,5 +14,5 @@ * limitations under the License. */ -export * from './error'; -export * from './payload'; +export * from './errors'; +export * from './serialization'; diff --git a/packages/errors/src/serialization/error.test.ts b/packages/errors/src/serialization/error.test.ts new file mode 100644 index 0000000000..336dfbf299 --- /dev/null +++ b/packages/errors/src/serialization/error.test.ts @@ -0,0 +1,64 @@ +/* + * Copyright 2021 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. + */ + +import { deserializeError, serializeError } from './error'; + +class CustomError extends Error { + readonly customField: any; + constructor(message: string) { + super(message); + this.name = 'CustomError'; + this.customField = { complex: 'data' }; + } +} + +describe('serialization', () => { + it('runs the happy path with stack', () => { + const before = new CustomError('m'); + const after = deserializeError( + serializeError(before, { includeStack: true }), + ); + expect(after.message).toEqual(before.message); + expect(after.name).toEqual(before.name); + expect(after.stack).toEqual(before.stack); + expect(after.customField).toEqual(before.customField); + // Does not reconstruct the actual class! + expect(after.constructor).not.toBe(before.constructor); + }); + + it('runs the happy path without stack', () => { + const before = new CustomError('m'); + const after = deserializeError( + serializeError(before, { includeStack: false }), + ); + expect(after.message).toEqual(before.message); + expect(after.name).toEqual(before.name); + expect(after.stack).not.toEqual(before.stack); + expect(after.customField).toEqual(before.customField); + // Does not reconstruct the actual class! + expect(after.constructor).not.toBe(before.constructor); + }); + + it('serializes stack traces only when allowed', () => { + const before = new CustomError('m'); + const withStack = serializeError(before, { includeStack: true }); + const withoutStack1 = serializeError(before, { includeStack: false }); + const withoutStack2 = serializeError(before); + expect(withStack.stack).toEqual(before.stack); + expect(withoutStack1.stack).not.toBeDefined(); + expect(withoutStack2.stack).not.toBeDefined(); + }); +}); diff --git a/packages/errors/src/serialization/error.ts b/packages/errors/src/serialization/error.ts new file mode 100644 index 0000000000..15a21cee5e --- /dev/null +++ b/packages/errors/src/serialization/error.ts @@ -0,0 +1,72 @@ +/* + * Copyright 2021 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. + */ + +import { JsonObject } from '@backstage/config'; +import { + deserializeError as deserializeErrorInternal, + serializeError as serializeErrorInternal, +} from 'serialize-error'; + +/** + * The serialized form of an Error. + */ +export type SerializedError = JsonObject & { + /** The name of the exception that was thrown */ + name: string; + /** The message of the exception that was thrown */ + message: string; + /** A stringified stack trace; may not be present */ + stack?: string; + /** A custom code (not necessarily the same as an HTTP response code); may not be present */ + code?: string; +}; + +/** + * Serializes an error object to a JSON friendly form. + * + * @param error The error + * @param options.includeStackTraces: Include stack trace in the output (default false) + */ +export function serializeError( + error: Error, + options?: { includeStack?: boolean }, +): SerializedError { + const serialized = serializeErrorInternal(error); + const result: SerializedError = { + name: 'Unknown', + message: '', + ...serialized, + }; + + if (!options?.includeStack) { + delete result.stack; + } + + return result; +} + +/** + * Deserializes a serialized error object back to an Error. + */ +export function deserializeError( + data: SerializedError, +): T { + const result = deserializeErrorInternal(data) as T; + if (!data.stack) { + result.stack = undefined; + } + return result; +} diff --git a/packages/errors/src/payload/index.ts b/packages/errors/src/serialization/index.ts similarity index 74% rename from packages/errors/src/payload/index.ts rename to packages/errors/src/serialization/index.ts index 238a009dc9..18b050d4ac 100644 --- a/packages/errors/src/payload/index.ts +++ b/packages/errors/src/serialization/index.ts @@ -14,5 +14,7 @@ * limitations under the License. */ -export { parseServerResponseErrorBody } from './ServerResponseErrorBody'; -export type { ServerResponseErrorBody } from './ServerResponseErrorBody'; +export { deserializeError, serializeError } from './error'; +export type { SerializedError } from './error'; +export { parseErrorResponse } from './response'; +export type { ErrorResponse } from './response'; diff --git a/packages/errors/src/payload/ServerResponseErrorBody.test.ts b/packages/errors/src/serialization/response.test.ts similarity index 63% rename from packages/errors/src/payload/ServerResponseErrorBody.test.ts rename to packages/errors/src/serialization/response.test.ts index 533b95926c..5e0050cba3 100644 --- a/packages/errors/src/payload/ServerResponseErrorBody.test.ts +++ b/packages/errors/src/serialization/response.test.ts @@ -14,24 +14,14 @@ * limitations under the License. */ -import { - parseServerResponseErrorBody, - ServerResponseErrorBody, -} from './ServerResponseErrorBody'; +import { parseErrorResponse, ErrorResponse } from './response'; -describe('parseServerResponseErrorBody', () => { +describe('parseErrorResponse', () => { it('handles the happy path', async () => { - const body: ServerResponseErrorBody = { - error: { - statusCode: 444, - name: 'Fours', - message: 'Expected fives', - stack: 'lines', - }, - request: { - method: 'GET', - url: '/', - }, + const body: ErrorResponse = { + error: { name: 'Fours', message: 'Expected fives', stack: 'lines' }, + request: { method: 'GET', url: '/' }, + response: { statusCode: 444 }, }; const response: Partial = { @@ -41,22 +31,16 @@ describe('parseServerResponseErrorBody', () => { headers: new Headers({ 'Content-Type': 'application/json' }), }; - await expect( - parseServerResponseErrorBody(response as Response), - ).resolves.toEqual(body); + await expect(parseErrorResponse(response as Response)).resolves.toEqual( + body, + ); }); it('uses request header and text body when wrong content type, even if parsable', async () => { - const body: ServerResponseErrorBody = { - error: { - statusCode: 333, - name: 'Threes', - message: 'Expected twos', - }, - request: { - method: 'GET', - url: '/', - }, + const body: ErrorResponse = { + error: { name: 'Threes', message: 'Expected twos' }, + request: { method: 'GET', url: '/' }, + response: { statusCode: 333 }, }; const response: Partial = { @@ -66,30 +50,22 @@ describe('parseServerResponseErrorBody', () => { headers: new Headers({ 'Content-Type': 'not-application/not-json' }), }; - await expect( - parseServerResponseErrorBody(response as Response), - ).resolves.toEqual({ + await expect(parseErrorResponse(response as Response)).resolves.toEqual({ error: { - statusCode: 444, name: 'Unknown', message: `Request failed with status 444 Fours, ${JSON.stringify( body, )}`, }, + response: { statusCode: 444 }, }); }); it('uses request header and text body when not parsable', async () => { - const body: ServerResponseErrorBody = { - error: { - statusCode: 333, - name: 'Threes', - message: 'Expected twos', - }, - request: { - method: 'GET', - url: '/', - }, + const body: ErrorResponse = { + error: { name: 'Threes', message: 'Expected twos' }, + request: { method: 'GET', url: '/' }, + response: { statusCode: 333 }, }; const response: Partial = { @@ -99,16 +75,14 @@ describe('parseServerResponseErrorBody', () => { headers: new Headers({ 'Content-Type': 'application/json' }), }; - await expect( - parseServerResponseErrorBody(response as Response), - ).resolves.toEqual({ + await expect(parseErrorResponse(response as Response)).resolves.toEqual({ error: { - statusCode: 444, name: 'Unknown', message: `Request failed with status 444 Fours, ${JSON.stringify( body, ).substring(1)}`, }, + response: { statusCode: 444 }, }); }); @@ -122,14 +96,12 @@ describe('parseServerResponseErrorBody', () => { headers: new Headers({ 'Content-Type': 'application/json' }), }; - await expect( - parseServerResponseErrorBody(response as Response), - ).resolves.toEqual({ + await expect(parseErrorResponse(response as Response)).resolves.toEqual({ error: { - statusCode: 444, name: 'Unknown', message: `Request failed with status 444 Fours`, }, + response: { statusCode: 444 }, }); }); }); diff --git a/packages/errors/src/payload/ServerResponseErrorBody.ts b/packages/errors/src/serialization/response.ts similarity index 74% rename from packages/errors/src/payload/ServerResponseErrorBody.ts rename to packages/errors/src/serialization/response.ts index e6f1c80749..becc84a397 100644 --- a/packages/errors/src/payload/ServerResponseErrorBody.ts +++ b/packages/errors/src/serialization/response.ts @@ -14,43 +14,44 @@ * limitations under the License. */ +import { SerializedError } from './error'; + /** * A standard shape of JSON data returned as the body of backend errors. */ -export type ServerResponseErrorBody = { +export type ErrorResponse = { /** Details of the error that was caught */ - error: { - /** The numeric HTTP status code that was returned */ - statusCode: number; - /** The name of the exception that was thrown */ - name: string; - /** The message of the exception that was thrown */ - message: string; - /** A stringified stack trace, may not be present */ - stack?: string; - }; + error: SerializedError; - /** The incoming request */ + /** Details about the incoming request */ request?: { /** The HTTP method of the request */ method: string; /** The URL of the request (excluding protocol and host/port) */ url: string; }; + + /** Details about the response */ + response: { + /** The numeric HTTP status code that was returned */ + statusCode: number; + }; }; /** - * Attempts to extract the ServerResponseErrorBody out of a server response. - * This consumes the body of the response. + * Attempts to construct an ErrorResponse out of a failed server request. + * Assumes that the response has already been checked to be not ok. This + * function consumes the body of the response, and assumes that it hasn't + * been consumed before. * * The code is forgiving, and constructs a useful synthetic body as best it can - * if the response wasn't on the expected form. + * if the response body wasn't on the expected form. * * @param response The response of a failed request */ -export async function parseServerResponseErrorBody( +export async function parseErrorResponse( response: Response, -): Promise { +): Promise { try { const text = await response.text(); if (text) { @@ -59,7 +60,7 @@ export async function parseServerResponseErrorBody( ) { try { const body = JSON.parse(text); - if (body.error && body.request) { + if (body.error && body.response) { return body; } } catch { @@ -69,10 +70,12 @@ export async function parseServerResponseErrorBody( return { error: { - statusCode: response.status, name: 'Unknown', message: `Request failed with status ${response.status} ${response.statusText}, ${text}`, }, + response: { + statusCode: response.status, + }, }; } } catch { @@ -81,9 +84,11 @@ export async function parseServerResponseErrorBody( return { error: { - statusCode: response.status, name: 'Unknown', message: `Request failed with status ${response.status} ${response.statusText}`, }, + response: { + statusCode: response.status, + }, }; } diff --git a/plugins/catalog-backend/src/ingestion/processors/util/parse.test.ts b/plugins/catalog-backend/src/ingestion/processors/util/parse.test.ts index 53201486bc..ab6e287698 100644 --- a/plugins/catalog-backend/src/ingestion/processors/util/parse.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/util/parse.test.ts @@ -116,7 +116,7 @@ describe('parseEntityYaml', () => { }); it('should handle empty yaml documents', () => { - // This happens if the user accidentially adds a "---" + // This happens if the user accidentally adds a "---" // at the end of a file const results = Array.from( parseEntityYaml( @@ -205,7 +205,7 @@ describe('parseEntityYaml', () => { it('must be an object at root', () => { const results = Array.from( - parseEntityYaml(Buffer.from('imma-string', 'utf8'), testLoc), + parseEntityYaml(Buffer.from('i-am-a-string', 'utf8'), testLoc), ); expect(results).toEqual([ diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index 78ef2e09ac..96e424c70b 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -15,12 +15,12 @@ */ import { errorHandler } from '@backstage/backend-common'; -import { NotFoundError } from '@backstage/errors'; import type { Entity } from '@backstage/catalog-model'; import { analyzeLocationSchema, locationSpecSchema, } from '@backstage/catalog-model'; +import { NotFoundError } from '@backstage/errors'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; diff --git a/plugins/catalog/src/components/EntityPageLayout/EntityPageLayout.tsx b/plugins/catalog/src/components/EntityPageLayout/EntityPageLayout.tsx index 7109f4beb3..51c8f85116 100644 --- a/plugins/catalog/src/components/EntityPageLayout/EntityPageLayout.tsx +++ b/plugins/catalog/src/components/EntityPageLayout/EntityPageLayout.tsx @@ -20,12 +20,12 @@ import { } from '@backstage/catalog-model'; import { Content, - ErrorResponsePanel, Header, HeaderLabel, Link, Page, Progress, + ResponseErrorPanel, WarningPanel, } from '@backstage/core'; import { @@ -143,7 +143,7 @@ export const EntityPageLayout = ({ children }: PropsWithChildren<{}>) => { {error && ( - + )} diff --git a/yarn.lock b/yarn.lock index 93328ac73b..0b5a3e2fa0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1772,6 +1772,7 @@ dependencies: "@backstage/config" "^0.1.3" "@backstage/core-api" "^0.2.12" + "@backstage/errors" "^0.1.1" "@backstage/theme" "^0.2.3" "@material-ui/core" "^4.11.0" "@material-ui/icons" "^4.9.1" @@ -1813,6 +1814,7 @@ dependencies: "@backstage/config" "^0.1.3" "@backstage/core-api" "^0.2.12" + "@backstage/errors" "^0.1.1" "@backstage/theme" "^0.2.3" "@material-ui/core" "^4.11.0" "@material-ui/icons" "^4.9.1" @@ -1854,6 +1856,7 @@ dependencies: "@backstage/config" "^0.1.3" "@backstage/core-api" "^0.2.12" + "@backstage/errors" "^0.1.1" "@backstage/theme" "^0.2.3" "@material-ui/core" "^4.11.0" "@material-ui/icons" "^4.9.1" @@ -23380,6 +23383,13 @@ serialize-error@^2.1.0: resolved "https://registry.npmjs.org/serialize-error/-/serialize-error-2.1.0.tgz#50b679d5635cdf84667bdc8e59af4e5b81d5f60a" integrity sha1-ULZ51WNc34Rme9yOWa9OW4HV9go= +serialize-error@^8.0.1: + version "8.0.1" + resolved "https://registry.npmjs.org/serialize-error/-/serialize-error-8.0.1.tgz#7a67f8ecbbf28973b5a954a2852ff9f4eef52d99" + integrity sha512-r5o60rWFS+8/b49DNAbB+GXZA0SpDpuWE758JxDKgRTga05r3U5lwyksE91dYKDhXSmnu36RALj615E6Aj5pSg== + dependencies: + type-fest "^0.20.2" + serialize-javascript@^2.1.2: version "2.1.2" resolved "https://registry.npmjs.org/serialize-javascript/-/serialize-javascript-2.1.2.tgz#ecec53b0e0317bdc95ef76ab7074b7384785fa61" @@ -25456,6 +25466,11 @@ type-fest@^0.18.0: resolved "https://registry.npmjs.org/type-fest/-/type-fest-0.18.1.tgz#db4bc151a4a2cf4eebf9add5db75508db6cc841f" integrity sha512-OIAYXk8+ISY+qTOwkHtKqzAuxchoMiD9Udx+FSGQDuiRR+PJKJHc2NJAXlbhkGwTt/4/nKZxELY1w3ReWOL8mw== +type-fest@^0.20.2: + version "0.20.2" + resolved "https://registry.npmjs.org/type-fest/-/type-fest-0.20.2.tgz#1bf207f4b28f91583666cb5fbd327887301cd5f4" + integrity sha512-Ne+eE4r0/iWnpAxD852z3A+N0Bt5RN//NjJwRd2VFHEmrywxf5vsZlh4R6lixl6B+wz/8d+maTSAkN1FIkI3LQ== + type-fest@^0.4.1: version "0.4.1" resolved "https://registry.npmjs.org/type-fest/-/type-fest-0.4.1.tgz#8bdf77743385d8a4f13ba95f610f5ccd68c728f8"