From f43192207ae6de1976d4376ac1d6ad9fc7fdb49b Mon Sep 17 00:00:00 2001 From: NHI TRAN Date: Fri, 19 Feb 2021 14:05:27 -0500 Subject: [PATCH] enhancement: 4423 switch sending back data from server to res.json() instead of res.send() changeset --- .changeset/kind-pens-deliver.md | 13 ++++++++++++ .../src/middleware/errorHandler.test.ts | 4 ++-- .../GithubCreateAppServer.ts | 2 +- .../src/stages/publish/awsS3.ts | 3 +-- .../src/stages/publish/azureBlobStorage.ts | 4 ++-- .../src/lib/oauth/OAuthAdapter.test.ts | 20 ++++++++++++------- .../src/lib/oauth/OAuthAdapter.ts | 14 +++++++------ .../src/providers/aws-alb/provider.test.ts | 15 +++++++------- .../src/providers/aws-alb/provider.ts | 6 +++--- .../src/providers/saml/provider.ts | 2 +- plugins/kafka-backend/src/service/router.ts | 2 +- .../kubernetes-backend/src/service/router.ts | 2 +- .../scaffolder-backend/src/service/router.ts | 2 +- .../techdocs-backend/src/service/router.ts | 7 +++---- 14 files changed, 57 insertions(+), 39 deletions(-) create mode 100644 .changeset/kind-pens-deliver.md diff --git a/.changeset/kind-pens-deliver.md b/.changeset/kind-pens-deliver.md new file mode 100644 index 0000000000..50ac410fe3 --- /dev/null +++ b/.changeset/kind-pens-deliver.md @@ -0,0 +1,13 @@ +--- +'@backstage/backend-common': minor +'@backstage/cli': minor +'@backstage/techdocs-common': minor +'@backstage/plugin-auth-backend': minor +'@backstage/plugin-catalog': minor +'@backstage/plugin-kafka-backend': minor +'@backstage/plugin-kubernetes-backend': minor +'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-techdocs-backend': minor +--- + +remove usage of res.send() for use of res.json() and res.end() diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts index 6d22c2f175..cab5df0fb8 100644 --- a/packages/backend-common/src/middleware/errorHandler.test.ts +++ b/packages/backend-common/src/middleware/errorHandler.test.ts @@ -39,12 +39,12 @@ describe('errorHandler', () => { const mockSend = jest.fn(); app.use('/works_with_async_fail', (_, res) => { - res.status(200).send('hello'); + res.status(200).json('hello'); // mutate the response object to test the middlware. // it's hard to catch errors inside middleware from the outside. // @ts-ignore - res.send = mockSend; + res.json = mockSend; throw new Error('some message'); }); diff --git a/packages/cli/src/commands/create-github-app/GithubCreateAppServer.ts b/packages/cli/src/commands/create-github-app/GithubCreateAppServer.ts index 45671c2ead..780704aee4 100644 --- a/packages/cli/src/commands/create-github-app/GithubCreateAppServer.ts +++ b/packages/cli/src/commands/create-github-app/GithubCreateAppServer.ts @@ -129,7 +129,7 @@ export class GithubCreateAppServer { body = body.replace('ACTION_URL', this.actionUrl); res.setHeader('content-type', 'text/html'); - res.send(body); + res.json(body); }; private async listen(app: Express) { diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 20e6bc424d..a6a1c5b960 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -231,8 +231,7 @@ export class AwsS3Publish implements PublisherBase { )) { res.setHeader(headerKey, headerValue); } - - res.send(fileContent); + res.json(); }) .catch(err => { this.logger.warn(err.message); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index a02639e63f..6d5e112641 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -228,11 +228,11 @@ export class AzureBlobStoragePublish implements PublisherBase { )) { res.setHeader(headerKey, headerValue); } - res.send(fileContent); + res.json(fileContent); }); } catch (e) { this.logger.error(e.message); - res.status(404).send(e.message); + res.status(404).json(e.message); } }; } diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index d2b31213f2..a9f0aefbfd 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -172,7 +172,8 @@ describe('OAuthAdapter', () => { const mockResponse = ({ cookie: jest.fn().mockReturnThis(), - send: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + status: jest.fn().mockReturnThis(), } as unknown) as express.Response; await oauthProvider.logout(mockRequest, mockResponse); @@ -200,12 +201,15 @@ describe('OAuthAdapter', () => { } as unknown) as express.Request; const mockResponse = ({ - send: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + status: jest.fn().mockReturnThis(), } as unknown) as express.Response; await oauthProvider.refresh(mockRequest, mockResponse); - expect(mockResponse.send).toHaveBeenCalledTimes(1); - expect(mockResponse.send).toHaveBeenCalledWith({ + expect(mockResponse.status).toHaveBeenCalledTimes(1); + expect(mockResponse.status).toHaveBeenCalledWith(200); + expect(mockResponse.json).toHaveBeenCalledTimes(1); + expect(mockResponse.json).toHaveBeenCalledWith({ ...mockResponseData, backstageIdentity: { id: mockResponseData.backstageIdentity.id, @@ -229,12 +233,14 @@ describe('OAuthAdapter', () => { } as unknown) as express.Request; const mockResponse = ({ - send: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + status: jest.fn().mockReturnThis(), } as unknown) as express.Response; await oauthProvider.refresh(mockRequest, mockResponse); - expect(mockResponse.send).toHaveBeenCalledTimes(1); - expect(mockResponse.send).toHaveBeenCalledWith( + expect(mockResponse.status).toHaveBeenCalledWith(406); + expect(mockResponse.json).toHaveBeenCalledTimes(1); + expect(mockResponse.json).toHaveBeenCalledWith( 'Refresh token not supported for provider: test-provider', ); }); diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 62cb8e444d..2807bfd5c7 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -155,7 +155,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { // remove refresh token cookie before logout this.removeRefreshTokenCookie(res); } - res.send('logout!'); + res.status(204).json('logout!'); } async refresh(req: express.Request, res: express.Response): Promise { @@ -165,9 +165,11 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { } if (!this.handlers.refresh || this.options.disableRefresh) { - res.send( - `Refresh token not supported for provider: ${this.options.providerId}`, - ); + res + .status(406) + .json( + `Refresh token not supported for provider: ${this.options.providerId}`, + ); return; } @@ -198,9 +200,9 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { this.setRefreshTokenCookie(res, response.providerInfo.refreshToken); } - res.send(response); + res.status(200).json(response); } catch (error) { - res.status(401).send(`${error.message}`); + res.status(401).json(`${error.message}`); } } diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts index d05b6bbfaf..5122ff8a93 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts @@ -77,7 +77,6 @@ describe('AwsALBAuthProvider', () => { getEntityByName: jest.fn(), }; - const mockResponseSend = jest.fn(); const mockRequest = ({ header: jest.fn(() => { return 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImZvbyIsImlzcyI6ImZvbyJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.T2BNS4G-6RoiFnXc8Q8TiwdWzTpNitY8jcsGM3N3-Yo'; @@ -90,7 +89,8 @@ describe('AwsALBAuthProvider', () => { } as unknown) as express.Request; const mockResponse = ({ header: () => jest.fn(), - send: mockResponseSend, + json: jest.fn().mockReturnThis(), + status: jest.fn(), } as unknown) as express.Response; describe('should transform to type OAuthResponse', () => { @@ -107,7 +107,7 @@ describe('AwsALBAuthProvider', () => { await provider.refresh(mockRequest, mockResponse); - expect(mockResponseSend.mock.calls[0][0]).toEqual({ + expect(mockResponse.json).toHaveBeenCalledWith({ backstageIdentity: { id: 'foo', idToken: '', @@ -129,7 +129,7 @@ describe('AwsALBAuthProvider', () => { await provider.refresh(mockRequestWithoutJwt, mockResponse); - expect(mockResponseSend.mock.calls[0][0]).toEqual(401); + expect(mockResponse.status).toHaveBeenCalledWith(401); }); it('JWT is invalid', async () => { @@ -145,7 +145,7 @@ describe('AwsALBAuthProvider', () => { await provider.refresh(mockRequest, mockResponse); - expect(mockResponseSend.mock.calls[0][0]).toEqual(401); + expect(mockResponse.status).toHaveBeenCalledWith(401); }); it('issuer is invalid', async () => { @@ -158,8 +158,7 @@ describe('AwsALBAuthProvider', () => { jwtMock.verify.mockReturnValueOnce({}); await provider.refresh(mockRequest, mockResponse); - - expect(mockResponseSend.mock.calls[0][0]).toEqual(401); + expect(mockResponse.status).toHaveBeenCalledWith(401); }); it('identity resolution callback rejects', async () => { @@ -173,7 +172,7 @@ describe('AwsALBAuthProvider', () => { await provider.refresh(mockRequest, mockResponse); - expect(mockResponseSend.mock.calls[0][0]).toEqual(401); + expect(mockResponse.status).toHaveBeenCalledWith(401); }); }); }); diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index 61ea10947e..4186f4ca99 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -78,13 +78,13 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { payload, this.catalogClient, ); - res.send(resolvedEntity); + res.json(resolvedEntity); } catch (e) { this.logger.error('exception occurred during JWT processing', e); - res.send(401); + res.status(401); } } else { - res.send(401); + res.status(401); } } diff --git a/plugins/auth-backend/src/providers/saml/provider.ts b/plugins/auth-backend/src/providers/saml/provider.ts index 74541d8294..9ef3b4d0a1 100644 --- a/plugins/auth-backend/src/providers/saml/provider.ts +++ b/plugins/auth-backend/src/providers/saml/provider.ts @@ -105,7 +105,7 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { } async logout(_req: express.Request, res: express.Response): Promise { - res.send('noop'); + res.json('noop'); } identifyEnv(): string | undefined { diff --git a/plugins/kafka-backend/src/service/router.ts b/plugins/kafka-backend/src/service/router.ts index 465347ed8f..3360bbb1de 100644 --- a/plugins/kafka-backend/src/service/router.ts +++ b/plugins/kafka-backend/src/service/router.ts @@ -68,7 +68,7 @@ export const makeRouter = ( })); }), ); - res.send({ consumerId, offsets: groupWithTopicOffsets.flat() }); + res.json({ consumerId, offsets: groupWithTopicOffsets.flat() }); }); return router; diff --git a/plugins/kubernetes-backend/src/service/router.ts b/plugins/kubernetes-backend/src/service/router.ts index 38a02ad970..0d4f2b7922 100644 --- a/plugins/kubernetes-backend/src/service/router.ts +++ b/plugins/kubernetes-backend/src/service/router.ts @@ -80,7 +80,7 @@ export const makeRouter = ( logger, requestBody, ); - res.send(response); + res.json(response); } catch (e) { logger.error( `action=retrieveObjectsByServiceId service=${serviceId}, error=${e}`, diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 9098e5d05e..731480371e 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -73,7 +73,7 @@ export async function createRouter( return; } - res.send({ + res.json({ id: job.id, metadata: { ...job.context, diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index fe7b52ab52..609a95b55f 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -63,7 +63,7 @@ export async function createRouter({ entityName, ); - res.send(techdocsMetadata); + res.json(techdocsMetadata); } catch (err) { logger.error( `Unable to get metadata for ${entityName.namespace}/${entityName.name} with error ${err}`, @@ -89,7 +89,7 @@ export async function createRouter({ ).json()) as Entity; const locationMetadata = getLocationForEntity(entity); - res.send({ ...entity, locationMetadata }); + res.json({ ...entity, locationMetadata }); } catch (err) { logger.info( `Unable to get metadata for ${kind}/${namespace}/${name} with error ${err}`, @@ -112,8 +112,7 @@ export async function createRouter({ const catalogRes = await fetch(`${catalogUrl}/entities/by-name/${triple}`); if (!catalogRes.ok) { const catalogResText = await catalogRes.text(); - res.status(catalogRes.status); - res.send(catalogResText); + res.status(catalogRes.status).json(catalogResText); return; }