diff --git a/.changeset/cyan-dryers-share.md b/.changeset/cyan-dryers-share.md new file mode 100644 index 0000000000..4c1ce1d553 --- /dev/null +++ b/.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. diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts index 3cc473daa0..ab07a91022 100644 --- a/packages/backend-common/src/middleware/errorHandler.test.ts +++ b/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; + + // 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 () => { diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index c36b3d96b5..81c0a6af5c 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/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/); }); }); }); diff --git a/packages/core-components/src/layout/ProxiedSignInPage/ProxiedSignInPage.test.tsx b/packages/core-components/src/layout/ProxiedSignInPage/ProxiedSignInPage.test.tsx index 3f0c01b56b..05960cd213 100644 --- a/packages/core-components/src/layout/ProxiedSignInPage/ProxiedSignInPage.test.tsx +++ b/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(); }); }); diff --git a/packages/errors/api-report.md b/packages/errors/api-report.md index 3ece63035b..894aac8413 100644 --- a/packages/errors/api-report.md +++ b/packages/errors/api-report.md @@ -128,6 +128,10 @@ export class ResponseError extends Error { }, ): Promise; readonly response: ConsumedResponse; + // (undocumented) + readonly statusCode: number; + // (undocumented) + readonly statusText: string; } // @public diff --git a/packages/errors/src/errors/ResponseError.test.ts b/packages/errors/src/errors/ResponseError.test.ts index 94f2e850d9..38db084b03 100644 --- a/packages/errors/src/errors/ResponseError.test.ts +++ b/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'); diff --git a/packages/errors/src/errors/ResponseError.ts b/packages/errors/src/errors/ResponseError.ts index f8ba0a1ba1..b5ca1e4ab8 100644 --- a/packages/errors/src/errors/ResponseError.ts +++ b/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 { 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; } } diff --git a/plugins/scaffolder-backend-module-confluence-to-markdown/src/actions/confluence/confluenceToMarkdown.test.ts b/plugins/scaffolder-backend-module-confluence-to-markdown/src/actions/confluence/confluenceToMarkdown.test.ts index 5ce8bb2697..39c9c98544 100644 --- a/plugins/scaffolder-backend-module-confluence-to-markdown/src/actions/confluence/confluenceToMarkdown.test.ts +++ b/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'); }); }); diff --git a/plugins/vault/src/api.test.ts b/plugins/vault/src/api.test.ts index 7ecd09b6f4..777137d0e7 100644 --- a/plugins/vault/src/api.test.ts +++ b/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', ); }); }); diff --git a/plugins/vault/src/components/EntityVaultCard/EntityVaultCard.test.tsx b/plugins/vault/src/components/EntityVaultCard/EntityVaultCard.test.tsx index 9450ddc41d..ec62ea8757 100644 --- a/plugins/vault/src/components/EntityVaultCard/EntityVaultCard.test.tsx +++ b/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', () => { , ); - expect( - rendered.getByText(/Add the annotation to your Component YAML/), - ).toBeInTheDocument(); + + await waitFor(() => + expect( + rendered.getByText(/Add the annotation to your Component YAML/), + ).toBeInTheDocument(), + ); }); }); diff --git a/plugins/vault/src/components/EntityVaultTable/EntityVaultTable.test.tsx b/plugins/vault/src/components/EntityVaultTable/EntityVaultTable.test.tsx index 5e191df9eb..14963ad13b 100644 --- a/plugins/vault/src/components/EntityVaultTable/EntityVaultTable.test.tsx +++ b/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(); });