From c9e5b59f78db5f3bcdb6f3c3aeaf3723d63894ff Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 1 Feb 2024 16:26:14 +0100 Subject: [PATCH 1/9] errors: set statusCode in ResponseError Signed-off-by: Vincenzo Scamporlino --- .../errors/src/errors/ResponseError.test.ts | 2 ++ packages/errors/src/errors/ResponseError.ts | 26 +++++++++++++------ 2 files changed, 20 insertions(+), 8 deletions(-) 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; } } From b4cb0085b9ca7de247870f7d5b23f2ec4b4549ac Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 1 Feb 2024 16:26:29 +0100 Subject: [PATCH 2/9] backend-common: test for ResponseError Signed-off-by: Vincenzo Scamporlino --- .../src/middleware/errorHandler.test.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) 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 () => { From 2636075b2fe50cbca9d3c7f3752f4e35eeb149ab Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 1 Feb 2024 18:16:25 +0100 Subject: [PATCH 3/9] ResponseError changeset Signed-off-by: Vincenzo Scamporlino --- .changeset/cyan-dryers-share.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cyan-dryers-share.md 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. From f4cf3f3dd4a28cf567e0c3c383f66804bda92f6c Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 15 Feb 2024 13:17:31 +0100 Subject: [PATCH 4/9] catalog-client: fix error message Signed-off-by: Vincenzo Scamporlino --- packages/catalog-client/src/CatalogClient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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/); }); }); }); From 276781c2dd4fb52745dddf2e6a7a66294559ffed Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 15 Feb 2024 13:25:01 +0100 Subject: [PATCH 5/9] vault: fix error message Signed-off-by: Vincenzo Scamporlino --- plugins/vault/src/api.test.ts | 2 +- .../src/components/EntityVaultTable/EntityVaultTable.test.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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/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(); }); From 8a3932ffe8a93e3ca393e60d988a95d7ee80f24d Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 15 Feb 2024 13:25:15 +0100 Subject: [PATCH 6/9] vault: fix warning in test Signed-off-by: Vincenzo Scamporlino --- .../EntityVaultCard/EntityVaultCard.test.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) 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(), + ); }); }); From b354046dadfe15e805ec45080406f26453e2ad6a Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 15 Feb 2024 13:26:10 +0100 Subject: [PATCH 7/9] scaffolder-backend-module-confluence-to-markdown: fix error message Signed-off-by: Vincenzo Scamporlino --- .../src/actions/confluence/confluenceToMarkdown.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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'); }); }); From 20340074c47bad515df547798f0b5e6df1585d20 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 15 Feb 2024 13:26:27 +0100 Subject: [PATCH 8/9] core-components: fix error text Signed-off-by: Vincenzo Scamporlino --- .../src/layout/ProxiedSignInPage/ProxiedSignInPage.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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(); }); }); From 406c5675a12f277984874e12a154f541c986ce09 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 15 Feb 2024 13:41:55 +0100 Subject: [PATCH 9/9] errors: api report Signed-off-by: Vincenzo Scamporlino --- packages/errors/api-report.md | 4 ++++ 1 file changed, 4 insertions(+) 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