Merge pull request #8144 from backstage/jhaals/errorz

errors: rework error response naming

.changeset/afraid-chairs-warn.md

@@ -0,0 +1,6 @@
+---
+'@backstage/errors': patch
+---
+
+Deprecate `parseErrorResponse` in favour of `parseErrorResponseBody`. Deprecate `data` field inside `ErrorResponse` in favour of `body`.
+Rename the error name for unknown errors from `unknown` to `error`.

packages/errors/api-report.md

@@ -33,8 +33,11 @@ export type ErrorLike = {
   [unknownKeys: string]: unknown;
 };
 
+// @public @deprecated
+export type ErrorResponse = ErrorResponseBody;
+
 // @public
-export type ErrorResponse = {
+export type ErrorResponseBody = {
   error: SerializedError;
   request?: {
     method: string;
@@ -65,19 +68,27 @@ export class NotFoundError extends CustomErrorBase {}
 // @public
 export class NotModifiedError extends CustomErrorBase {}
 
-// @public
+// @public @deprecated
 export function parseErrorResponse(response: Response): Promise<ErrorResponse>;
 
+// @public
+export function parseErrorResponseBody(
+  response: Response,
+): Promise<ErrorResponseBody>;
+
 // @public
 export class ResponseError extends Error {
+  // @deprecated
   constructor(props: {
     message: string;
     response: Response;
-    data: ErrorResponse;
+    data: ErrorResponseBody;
     cause: Error;
   });
+  readonly body: ErrorResponseBody;
   readonly cause: Error;
-  readonly data: ErrorResponse;
+  // @deprecated
+  get data(): ErrorResponseBody;
   static fromResponse(response: Response): Promise<ResponseError>;
   readonly response: Response;
 }

packages/errors/src/errors/ResponseError.ts

@@ -14,16 +14,16 @@
  * limitations under the License.
  */
 
+import { deserializeError } from '../serialization';
 import {
-  parseErrorResponse,
-  ErrorResponse,
-  deserializeError,
-} from '../serialization';
+  ErrorResponseBody,
+  parseErrorResponseBody,
+} from '../serialization/response';
 
 /**
  * An error thrown as the result of a failed server request.
  *
- * The server is expected to respond on the ErrorResponse format.
+ * The server is expected to respond on the ErrorResponseBody format.
  *
  * @public
  */
@@ -39,7 +39,7 @@ export class ResponseError extends Error {
   /**
    * The parsed JSON error body, as sent by the server.
    */
-  readonly data: ErrorResponse;
+  readonly body: ErrorResponseBody;
 
   /**
    * The Error cause, as seen by the remote server. This is parsed out of the
@@ -60,7 +60,7 @@ export class ResponseError extends Error {
    * been consumed before.
    */
   static async fromResponse(response: Response): Promise<ResponseError> {
-    const data = await parseErrorResponse(response);
+    const data = await parseErrorResponseBody(response);
 
     const status = data.response.statusCode || response.status;
     const statusText = data.error.name || response.statusText;
@@ -75,16 +75,28 @@ export class ResponseError extends Error {
     });
   }
 
+  /**
+   * @deprecated will be removed.
+   **/
   constructor(props: {
     message: string;
     response: Response;
-    data: ErrorResponse;
+    data: ErrorResponseBody;
     cause: Error;
   }) {
     super(props.message);
     this.name = 'ResponseError';
     this.response = props.response;
-    this.data = props.data;
+    this.body = props.data;
     this.cause = props.cause;
   }
+  /**
+   * The parsed JSON error body, as sent by the server.
+   * @deprecated use body instead.
+   */
+  get data(): ErrorResponseBody {
+    // eslint-disable-next-line no-console
+    console.warn('ErrorResponse.data is deprecated, use .body instead.');
+    return this.body;
+  }
 }

packages/errors/src/serialization/index.ts

@@ -16,5 +16,5 @@
 
 export { deserializeError, serializeError, stringifyError } from './error';
 export type { SerializedError } from './error';
-export { parseErrorResponse } from './response';
-export type { ErrorResponse } from './response';
+export { parseErrorResponse, parseErrorResponseBody } from './response';
+export type { ErrorResponse, ErrorResponseBody } from './response';

packages/errors/src/serialization/response.test.ts

@@ -14,11 +14,11 @@
  * limitations under the License.
  */
 
-import { parseErrorResponse, ErrorResponse } from './response';
+import { parseErrorResponseBody, ErrorResponseBody } from './response';
 
-describe('parseErrorResponse', () => {
+describe('parseErrorResponseBody', () => {
   it('handles the happy path', async () => {
-    const body: ErrorResponse = {
+    const body: ErrorResponseBody = {
       error: { name: 'Fours', message: 'Expected fives', stack: 'lines' },
       request: { method: 'GET', url: '/' },
       response: { statusCode: 444 },
@@ -31,13 +31,13 @@ describe('parseErrorResponse', () => {
       headers: new Headers({ 'Content-Type': 'application/json' }),
     };
 
-    await expect(parseErrorResponse(response as Response)).resolves.toEqual(
+    await expect(parseErrorResponseBody(response as Response)).resolves.toEqual(
       body,
     );
   });
 
   it('uses request header and text body when wrong content type, even if parsable', async () => {
-    const body: ErrorResponse = {
+    const body: ErrorResponseBody = {
       error: { name: 'Threes', message: 'Expected twos' },
       request: { method: 'GET', url: '/' },
       response: { statusCode: 333 },
@@ -50,19 +50,21 @@ describe('parseErrorResponse', () => {
       headers: new Headers({ 'Content-Type': 'not-application/not-json' }),
     };
 
-    await expect(parseErrorResponse(response as Response)).resolves.toEqual({
-      error: {
-        name: 'Unknown',
-        message: `Request failed with status 444 Fours, ${JSON.stringify(
-          body,
-        )}`,
+    await expect(parseErrorResponseBody(response as Response)).resolves.toEqual(
+      {
+        error: {
+          name: 'Error',
+          message: `Request failed with status 444 Fours, ${JSON.stringify(
+            body,
+          )}`,
+        },
+        response: { statusCode: 444 },
       },
-      response: { statusCode: 444 },
-    });
+    );
   });
 
   it('uses request header and text body when not parsable', async () => {
-    const body: ErrorResponse = {
+    const body: ErrorResponseBody = {
       error: { name: 'Threes', message: 'Expected twos' },
       request: { method: 'GET', url: '/' },
       response: { statusCode: 333 },
@@ -75,15 +77,17 @@ describe('parseErrorResponse', () => {
       headers: new Headers({ 'Content-Type': 'application/json' }),
     };
 
-    await expect(parseErrorResponse(response as Response)).resolves.toEqual({
-      error: {
-        name: 'Unknown',
-        message: `Request failed with status 444 Fours, ${JSON.stringify(
-          body,
-        ).substring(1)}`,
+    await expect(parseErrorResponseBody(response as Response)).resolves.toEqual(
+      {
+        error: {
+          name: 'Error',
+          message: `Request failed with status 444 Fours, ${JSON.stringify(
+            body,
+          ).substring(1)}`,
+        },
+        response: { statusCode: 444 },
       },
-      response: { statusCode: 444 },
-    });
+    );
   });
 
   it('uses request header when failing to get body', async () => {
@@ -96,12 +100,14 @@ describe('parseErrorResponse', () => {
       headers: new Headers({ 'Content-Type': 'application/json' }),
     };
 
-    await expect(parseErrorResponse(response as Response)).resolves.toEqual({
-      error: {
-        name: 'Unknown',
-        message: `Request failed with status 444 Fours`,
+    await expect(parseErrorResponseBody(response as Response)).resolves.toEqual(
+      {
+        error: {
+          name: 'Error',
+          message: `Request failed with status 444 Fours`,
+        },
+        response: { statusCode: 444 },
       },
-      response: { statusCode: 444 },
-    });
+    );
   });
 });

packages/errors/src/serialization/response.ts

@@ -20,8 +20,16 @@ import { SerializedError } from './error';
  * A standard shape of JSON data returned as the body of backend errors.
  *
  * @public
+ * @deprecated - Use {@link ErrorResponseBody} instead.
  */
-export type ErrorResponse = {
+export type ErrorResponse = ErrorResponseBody;
+
+/**
+ * A standard shape of JSON data returned as the body of backend errors.
+ *
+ * @public
+ */
+export type ErrorResponseBody = {
   /** Details of the error that was caught */
   error: SerializedError;
 
@@ -51,10 +59,29 @@ export type ErrorResponse = {
  *
  * @public
  * @param response - The response of a failed request
+ * @deprecated - Use {@link parseErrorResponseBody} instead.
  */
 export async function parseErrorResponse(
   response: Response,
 ): Promise<ErrorResponse> {
+  return parseErrorResponseBody(response);
+}
+
+/**
+ * Attempts to construct an ErrorResponseBody 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 body wasn't on the expected form.
+ *
+ * @public
+ * @param response - The response of a failed request
+ */
+export async function parseErrorResponseBody(
+  response: Response,
+): Promise<ErrorResponseBody> {
   try {
     const text = await response.text();
     if (text) {
@@ -73,7 +100,7 @@ export async function parseErrorResponse(
 
       return {
         error: {
-          name: 'Unknown',
+          name: 'Error',
           message: `Request failed with status ${response.status} ${response.statusText}, ${text}`,
         },
         response: {
@@ -87,7 +114,7 @@ export async function parseErrorResponse(
 
   return {
     error: {
-      name: 'Unknown',
+      name: 'Error',
       message: `Request failed with status ${response.status} ${response.statusText}`,
     },
     response: {