Merge pull request #22645 from backstage/fix/response-error-status-code

Fix ResponseError HTTP status

.changeset/cyan-dryers-share.md

@@ -0,0 +1,5 @@
+---
+'@backstage/errors': patch
+---
+
+Fixed an issue that was causing ResponseError not to report the HTTP status from the provided response.

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

@@ -21,11 +21,13 @@ import {
   NotAllowedError,
   NotFoundError,
   NotModifiedError,
+  ResponseError,
 } from '@backstage/errors';
 import express from 'express';
 import createError from 'http-errors';
 import request from 'supertest';
 import { errorHandler } from './errorHandler';
+import { STATUS_CODES } from 'http';
 
 describe('errorHandler', () => {
   it('gives default code and message', async () => {
@@ -116,6 +118,53 @@ describe('errorHandler', () => {
     app.use('/ConflictError', () => {
       throw new ConflictError();
     });
+    app.use('/ResponseErrorBackstagePlugin', async (_req, _res, next) => {
+      const mockedResponse = {
+        status: jest.fn(() => mockedResponse),
+        json: jest.fn(() => mockedResponse),
+      } as unknown as jest.Mocked<express.Response>;
+
+      // serialize AuthenticationError in mockedResponse
+      errorHandler()(
+        new AuthenticationError('an error'),
+        { method: 'GET', url: '' } as express.Request,
+        mockedResponse,
+        jest.fn(),
+      );
+
+      const status = mockedResponse.status.mock.calls[0][0];
+      next(
+        await ResponseError.fromResponse({
+          headers: new Headers({
+            'content-type': 'application/json',
+          }),
+          ok: false,
+          redirected: false,
+          status,
+          statusText: STATUS_CODES[status]!,
+          type: 'default',
+          url: '',
+          text: async () =>
+            JSON.stringify(mockedResponse.json.mock.calls[0][0]),
+        }),
+      );
+    });
+    app.use('/ResponseError', async (_req, _res, next) => {
+      next(
+        await ResponseError.fromResponse({
+          headers: new Headers({
+            'content-type': 'application/json',
+          }),
+          ok: false,
+          redirected: false,
+          status: 403,
+          statusText: STATUS_CODES[403]!,
+          type: 'default',
+          url: '',
+          text: async () => JSON.stringify({}),
+        }),
+      );
+    });
     app.use(errorHandler());
 
     const r = request(app);
@@ -138,6 +187,14 @@ describe('errorHandler', () => {
     expect((await r.get('/ConflictError')).body.error.name).toBe(
       'ConflictError',
     );
+    expect((await r.get('/ResponseErrorBackstagePlugin')).status).toBe(401);
+    expect((await r.get('/ResponseErrorBackstagePlugin')).body.error.name).toBe(
+      'ResponseError',
+    );
+    expect((await r.get('/ResponseError')).status).toBe(403);
+    expect((await r.get('/ResponseError')).body.error.name).toBe(
+      'ResponseError',
+    );
   });
 
   it('logs all 500 errors', async () => {

packages/catalog-client/src/CatalogClient.test.ts

@@ -761,7 +761,7 @@ describe('CatalogClient', () => {
           },
           'url:http://example.com',
         ),
-      ).rejects.toThrow(/Request failed with 500 Error/);
+      ).rejects.toThrow(/Request failed with 500 Internal Server Error/);
     });
   });
 });

packages/core-components/src/layout/ProxiedSignInPage/ProxiedSignInPage.test.tsx

@@ -97,7 +97,7 @@ describe('ProxiedSignInPage', () => {
     render(Subject);
 
     await expect(
-      screen.findByText('Request failed with 401 Error'),
+      screen.findByText('Request failed with 401 Unauthorized'),
     ).resolves.toBeInTheDocument();
   });
 });

packages/errors/api-report.md

@@ -128,6 +128,10 @@ export class ResponseError extends Error {
     },
   ): Promise<ResponseError>;
   readonly response: ConsumedResponse;
+  // (undocumented)
+  readonly statusCode: number;
+  // (undocumented)
+  readonly statusText: string;
 }
 
 // @public

packages/errors/src/errors/ResponseError.test.ts

@@ -35,6 +35,8 @@ describe('ResponseError', () => {
     const e = await ResponseError.fromResponse(response as Response);
     expect(e.name).toEqual('ResponseError');
     expect(e.message).toEqual('Request failed with 444 Fours');
+    expect(e.statusCode).toEqual(444);
+    expect(e.statusText).toEqual('Fours');
     expect(e.cause.name).toEqual('Fours');
     expect(e.cause.message).toEqual('Expected fives');
     expect(e.cause.stack).toEqual('lines');

packages/errors/src/errors/ResponseError.ts

@@ -53,6 +53,9 @@ export class ResponseError extends Error {
    */
   readonly cause: Error;
 
+  readonly statusCode: number;
+
+  readonly statusText: string;
   /**
    * Constructs a ResponseError based on a failed response.
    *
@@ -65,9 +68,9 @@ export class ResponseError extends Error {
   ): Promise<ResponseError> {
     const data = await parseErrorResponseBody(response);
 
-    const status = data.response.statusCode || response.status;
-    const statusText = data.error.name || response.statusText;
-    const message = `Request failed with ${status} ${statusText}`;
+    const statusCode = data.response.statusCode || response.status;
+    const statusText = response.statusText;
+    const message = `Request failed with ${statusCode} ${statusText}`;
     const cause = deserializeError(data.error);
 
     return new ResponseError({
@@ -75,19 +78,26 @@ export class ResponseError extends Error {
       response,
       data,
       cause,
+      statusCode,
+      statusText,
     });
   }
 
-  private constructor(props: {
+  private constructor(opts: {
     message: string;
     response: ConsumedResponse;
     data: ErrorResponseBody;
     cause: Error;
+    statusCode: number;
+    statusText: string;
   }) {
-    super(props.message);
+    super(opts.message);
+
     this.name = 'ResponseError';
-    this.response = props.response;
-    this.body = props.data;
-    this.cause = props.cause;
+    this.response = opts.response;
+    this.body = opts.data;
+    this.cause = opts.cause;
+    this.statusCode = opts.statusCode;
+    this.statusText = opts.statusText;
   }
 }

plugins/scaffolder-backend-module-confluence-to-markdown/src/actions/confluence/confluenceToMarkdown.test.ts

@@ -221,7 +221,7 @@ describe('confluence:transform:markdown', () => {
     const action = createConfluenceToMarkdownAction(options);
     await expect(async () => {
       await action.handler(mockContext);
-    }).rejects.toThrow('Request failed with 401 Error');
+    }).rejects.toThrow('Request failed with 401 nope');
   });
 
   it('should return nothing in results from the first api call and fail', async () => {
@@ -284,6 +284,6 @@ describe('confluence:transform:markdown', () => {
     const action = createConfluenceToMarkdownAction(options);
     await expect(async () => {
       await action.handler(mockContext);
-    }).rejects.toThrow('Request failed with 404 Error');
+    }).rejects.toThrow('Request failed with 404 nope');
   });
 });

plugins/vault/src/api.test.ts

@@ -110,7 +110,7 @@ describe('api', () => {
 
   it('should throw an error if the Vault API responds with a non-successful HTTP status code', async () => {
     await expect(api.listSecrets('test/error')).rejects.toThrow(
-      'Request failed with 400 Error',
+      'Request failed with 400 Bad Request',
     );
   });
 });

plugins/vault/src/components/EntityVaultCard/EntityVaultCard.test.tsx

@@ -18,7 +18,7 @@ import React from 'react';
 import { setupServer } from 'msw/node';
 import { setupRequestMockHandlers } from '@backstage/test-utils';
 import { ComponentEntity } from '@backstage/catalog-model';
-import { render } from '@testing-library/react';
+import { render, waitFor } from '@testing-library/react';
 import { EntityVaultCard } from './EntityVaultCard';
 import { EntityProvider } from '@backstage/plugin-catalog-react';
 
@@ -45,8 +45,11 @@ describe('EntityVaultCard', () => {
         <EntityVaultCard />
       </EntityProvider>,
     );
-    expect(
-      rendered.getByText(/Add the annotation to your Component YAML/),
-    ).toBeInTheDocument();
+
+    await waitFor(() =>
+      expect(
+        rendered.getByText(/Add the annotation to your Component YAML/),
+      ).toBeInTheDocument(),
+    );
   });
 });

plugins/vault/src/components/EntityVaultTable/EntityVaultTable.test.tsx

@@ -161,7 +161,7 @@ describe('EntityVaultTable', () => {
 
     expect(
       rendered.getByText(
-        /Unexpected error while fetching secrets from path \'test\/error\'\: Request failed with 400 Error/,
+        /Unexpected error while fetching secrets from path \'test\/error\'\: Request failed with 400 Bad Request/,
       ),
     ).toBeInTheDocument();
   });