From 22b5f654764c8387493f5758d28c8dd209077ccb Mon Sep 17 00:00:00 2001 From: Aaron Cook Date: Thu, 5 Dec 2024 10:09:20 +0100 Subject: [PATCH] Return `502` for gateway validation errors (#2111) Changes server validation-related error codes to `502`: - Change server validation-related errors codes --- .../balances-api/safe-balances-api.service.ts | 4 ++ .../balances/balances.controller.spec.ts | 8 +-- src/routes/chains/chains.controller.spec.ts | 29 ++++------ .../common/filters/zod-error.filter.spec.ts | 23 ++++++-- src/routes/common/filters/zod-error.filter.ts | 8 +-- .../community/community.controller.spec.ts | 56 ++++++------------- .../contracts/contracts.controller.spec.ts | 7 +-- .../delegates/delegates.controller.spec.ts | 7 +-- .../v2/delegates.v2.controller.spec.ts | 7 +-- .../estimations.controller.spec.ts | 7 +-- .../messages/messages.controller.spec.ts | 4 +- src/routes/owners/owners.controller.spec.ts | 14 ++--- .../safe-apps/safe-apps.controller.spec.ts | 7 +-- ...rs-by-safe.transactions.controller.spec.ts | 7 ++- ...ns-by-safe.transactions.controller.spec.ts | 7 +-- ...ns-by-safe.transactions.controller.spec.ts | 8 +-- ...ns-by-safe.transactions.controller.spec.ts | 4 +- .../transactions-history.controller.spec.ts | 4 +- 18 files changed, 87 insertions(+), 124 deletions(-) diff --git a/src/datasources/balances-api/safe-balances-api.service.ts b/src/datasources/balances-api/safe-balances-api.service.ts index 8b8f9663d9..6dd630aa54 100644 --- a/src/datasources/balances-api/safe-balances-api.service.ts +++ b/src/datasources/balances-api/safe-balances-api.service.ts @@ -16,6 +16,7 @@ import { Injectable } from '@nestjs/common'; import { Chain } from '@/domain/chains/entities/chain.entity'; import { rawify, type Raw } from '@/validation/entities/raw.entity'; import { AssetPricesSchema } from '@/datasources/balances-api/entities/asset-price.entity'; +import { ZodError } from 'zod'; @Injectable() export class SafeBalancesApi implements IBalancesApi { @@ -87,6 +88,9 @@ export class SafeBalancesApi implements IBalancesApi { chain: args.chain, }); } catch (error) { + if (error instanceof ZodError) { + throw error; + } throw this.httpErrorFactory.from(error); } } diff --git a/src/routes/balances/balances.controller.spec.ts b/src/routes/balances/balances.controller.spec.ts index fee90b67ff..d629390fff 100644 --- a/src/routes/balances/balances.controller.spec.ts +++ b/src/routes/balances/balances.controller.spec.ts @@ -808,7 +808,7 @@ describe('Balances Controller (Unit)', () => { }); }); - it(`503 error if validation fails`, async () => { + it(`502 error if validation fails`, async () => { const chainId = '1'; const safeAddress = getAddress(faker.finance.ethereumAddress()); const chainResponse = chainBuilder().with('chainId', chainId).build(); @@ -838,10 +838,10 @@ describe('Balances Controller (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/chains/${chainId}/safes/${safeAddress}/balances/usd`) - .expect(503) + .expect(502) .expect({ - code: 503, - message: 'Service unavailable', + statusCode: 502, + message: 'Bad gateway', }); expect(networkService.get.mock.calls.length).toBe(3); diff --git a/src/routes/chains/chains.controller.spec.ts b/src/routes/chains/chains.controller.spec.ts index a8e7008d79..2eada4d387 100644 --- a/src/routes/chains/chains.controller.spec.ts +++ b/src/routes/chains/chains.controller.spec.ts @@ -304,10 +304,10 @@ describe('Chains Controller (Unit)', () => { status: 200, }); - await request(app.getHttpServer()).get('/v1/chains').expect(500).expect({ - statusCode: 500, - message: 'Internal server error', - }); + await request(app.getHttpServer()) + .get('/v1/chains') + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); expect(networkService.get).toHaveBeenCalledTimes(1); expect(networkService.get).toHaveBeenCalledWith({ url: `${safeConfigUrl}/api/v1/chains`, @@ -444,11 +444,8 @@ describe('Chains Controller (Unit)', () => { await request(app.getHttpServer()) .get('/v1/chains/1/about/backbone') - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); expect(networkService.get).toHaveBeenCalledTimes(2); expect(networkService.get.mock.calls[0][0].url).toBe( @@ -633,11 +630,8 @@ describe('Chains Controller (Unit)', () => { await request(app.getHttpServer()) .get('/v1/chains/1/about/master-copies') - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); }); @@ -766,11 +760,8 @@ describe('Chains Controller (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/chains/${chainResponse.chainId}/about/indexing`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); }); diff --git a/src/routes/common/filters/zod-error.filter.spec.ts b/src/routes/common/filters/zod-error.filter.spec.ts index 18d9862b50..1889edf6c1 100644 --- a/src/routes/common/filters/zod-error.filter.spec.ts +++ b/src/routes/common/filters/zod-error.filter.spec.ts @@ -55,6 +55,11 @@ class TestController { return body; } + @Post('zod-standard') + zodStandardError(): number { + return z.number().parse('string'); + } + @Get('non-zod-exception') nonZodException(): void { throw new Error('Some random error'); @@ -84,7 +89,7 @@ describe('ZodErrorFilter tests', () => { await app.close(); }); - it('ZodError exception returns first issue', async () => { + it('ZodErrorWithCode exception returns first issue', async () => { await request(app.getHttpServer()) .post('/zod-exception') .send({ value: faker.number.int() }) @@ -99,7 +104,7 @@ describe('ZodErrorFilter tests', () => { }); }); - it('ZodError union exception returns first issue of first union issue', async () => { + it('ZodErrorWithCode union exception returns first issue of first union issue', async () => { await request(app.getHttpServer()) .post('/zod-union-exception') .send({ value: faker.datatype.boolean() }) @@ -114,7 +119,7 @@ describe('ZodErrorFilter tests', () => { }); }); - it('ZodError nested union exception returns first issue of nested union issue', async () => { + it('ZodErrorWithCode nested union exception returns first issue of nested union issue', async () => { await request(app.getHttpServer()) .post('/zod-union-exception') .send({ @@ -133,7 +138,17 @@ describe('ZodErrorFilter tests', () => { }); }); - it('non-ZodError exception returns correct error code and message', async () => { + it('ZodError returns 502', async () => { + await request(app.getHttpServer()) + .post('/zod-standard') + .expect(502) + .expect({ + statusCode: 502, + message: 'Bad gateway', + }); + }); + + it('non-ZodError/ZodErrorWithCode exception returns correct error code and message', async () => { await request(app.getHttpServer()) .get('/non-zod-exception') .expect(500) diff --git a/src/routes/common/filters/zod-error.filter.ts b/src/routes/common/filters/zod-error.filter.ts index 5acd24d483..4b86b5a3c6 100644 --- a/src/routes/common/filters/zod-error.filter.ts +++ b/src/routes/common/filters/zod-error.filter.ts @@ -14,7 +14,7 @@ import { ZodErrorWithCode } from '@/validation/pipes/validation.pipe'; * * It builds a JSON payload which contains a code and a message. * The message is read from the initial {@link ZodIssue} and the code - * from {@link ZodErrorWithCode.code} or 500 if {@link ZodError}. + * from {@link ZodErrorWithCode.code} or 502 if {@link ZodError}. */ @Catch(ZodError, ZodErrorWithCode) export class ZodErrorFilter implements ExceptionFilter { @@ -32,9 +32,9 @@ export class ZodErrorFilter implements ExceptionFilter { }); } else { // Don't expose validation as it may contain sensitive data - response.status(HttpStatus.INTERNAL_SERVER_ERROR).json({ - statusCode: HttpStatus.INTERNAL_SERVER_ERROR, - message: 'Internal server error', + response.status(HttpStatus.BAD_GATEWAY).json({ + statusCode: HttpStatus.BAD_GATEWAY, + message: 'Bad gateway', }); } } diff --git a/src/routes/community/community.controller.spec.ts b/src/routes/community/community.controller.spec.ts index bb61f871e6..0eb138ca3a 100644 --- a/src/routes/community/community.controller.spec.ts +++ b/src/routes/community/community.controller.spec.ts @@ -152,11 +152,8 @@ describe('Community (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/community/campaigns`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should forward the pagination parameters', async () => { @@ -272,11 +269,8 @@ describe('Community (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/community/campaigns/${invalidCampaign.resourceId}`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should forward an error from the service', async () => { @@ -486,11 +480,8 @@ describe('Community (Unit)', () => { .get( `/v1/community/campaigns/${campaign.resourceId}/activities?holder=${holder}`, ) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should forward an error from the service', () => { @@ -589,11 +580,8 @@ describe('Community (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/community/campaigns/${campaign.resourceId}/leaderboard`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should forward the pagination parameters', async () => { @@ -727,11 +715,8 @@ describe('Community (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/community/campaigns/${resourceId}/leaderboard/${safeAddress}`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should forward an error from the service', async () => { @@ -904,11 +889,8 @@ describe('Community (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/community/locking/leaderboard`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should forward an error from the service', async () => { @@ -989,11 +971,8 @@ describe('Community (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/community/locking/${safeAddress}/rank`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should forward an error from the service', async () => { @@ -1166,11 +1145,8 @@ describe('Community (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/community/locking/${safeAddress}/history`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should forward an error from the service', async () => { diff --git a/src/routes/contracts/contracts.controller.spec.ts b/src/routes/contracts/contracts.controller.spec.ts index 1d4681cb60..1d4f9cec31 100644 --- a/src/routes/contracts/contracts.controller.spec.ts +++ b/src/routes/contracts/contracts.controller.spec.ts @@ -150,11 +150,8 @@ describe('Contracts controller', () => { await request(app.getHttpServer()) .get(`/v1/chains/${chain.chainId}/contracts/${contract.address}`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); }); }); diff --git a/src/routes/delegates/delegates.controller.spec.ts b/src/routes/delegates/delegates.controller.spec.ts index 8b7d65a079..f53c802df8 100644 --- a/src/routes/delegates/delegates.controller.spec.ts +++ b/src/routes/delegates/delegates.controller.spec.ts @@ -125,11 +125,8 @@ describe('Delegates controller', () => { await request(app.getHttpServer()) .get(`/v1/chains/${chain.chainId}/delegates?safe=${safe}`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('Should return empty result', async () => { diff --git a/src/routes/delegates/v2/delegates.v2.controller.spec.ts b/src/routes/delegates/v2/delegates.v2.controller.spec.ts index dd6a2b7054..accf167a80 100644 --- a/src/routes/delegates/v2/delegates.v2.controller.spec.ts +++ b/src/routes/delegates/v2/delegates.v2.controller.spec.ts @@ -133,11 +133,8 @@ describe('Delegates controller', () => { await request(app.getHttpServer()) .get(`/v2/chains/${chain.chainId}/delegates?safe=${safe}`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should return empty result', async () => { diff --git a/src/routes/estimations/estimations.controller.spec.ts b/src/routes/estimations/estimations.controller.spec.ts index 33d7896b2d..b0546710ff 100644 --- a/src/routes/estimations/estimations.controller.spec.ts +++ b/src/routes/estimations/estimations.controller.spec.ts @@ -177,11 +177,8 @@ describe('Estimations Controller (Unit)', () => { data: faker.string.hexadecimal({ length: 32 }), operation: 0, }) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('Should get a validation error', async () => { diff --git a/src/routes/messages/messages.controller.spec.ts b/src/routes/messages/messages.controller.spec.ts index 9902c30617..d571ffccd6 100644 --- a/src/routes/messages/messages.controller.spec.ts +++ b/src/routes/messages/messages.controller.spec.ts @@ -496,8 +496,8 @@ describe('Messages controller', () => { await request(app.getHttpServer()) .get(`/v1/chains/${chain.chainId}/safes/${safe.address}/messages`) - .expect(500) - .expect({ statusCode: 500, message: 'Internal server error' }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should get a message with a date label', async () => { diff --git a/src/routes/owners/owners.controller.spec.ts b/src/routes/owners/owners.controller.spec.ts index 3cc1d99414..789dae0b03 100644 --- a/src/routes/owners/owners.controller.spec.ts +++ b/src/routes/owners/owners.controller.spec.ts @@ -193,11 +193,8 @@ describe('Owners Controller (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/chains/${chainId}/owners/${ownerAddress}/safes`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); }); @@ -452,11 +449,8 @@ describe('Owners Controller (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/owners/${ownerAddress}/safes`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); }); }); diff --git a/src/routes/safe-apps/safe-apps.controller.spec.ts b/src/routes/safe-apps/safe-apps.controller.spec.ts index 47bd92e05a..308c383cb8 100644 --- a/src/routes/safe-apps/safe-apps.controller.spec.ts +++ b/src/routes/safe-apps/safe-apps.controller.spec.ts @@ -229,11 +229,8 @@ describe('Safe Apps Controller (Unit)', () => { await request(app.getHttpServer()) .get(`/v1/chains/${chain.chainId}/safe-apps`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); }); }); diff --git a/src/routes/transactions/__tests__/controllers/list-incoming-transfers-by-safe.transactions.controller.spec.ts b/src/routes/transactions/__tests__/controllers/list-incoming-transfers-by-safe.transactions.controller.spec.ts index 2b6f2dfb05..c1f56b7550 100644 --- a/src/routes/transactions/__tests__/controllers/list-incoming-transfers-by-safe.transactions.controller.spec.ts +++ b/src/routes/transactions/__tests__/controllers/list-incoming-transfers-by-safe.transactions.controller.spec.ts @@ -168,8 +168,8 @@ describe('List incoming transfers by Safe - Transactions Controller (Unit)', () await request(app.getHttpServer()) .get(`/v1/chains/${chainId}/safes/${safeAddress}/incoming-transfers`) - .expect(500) - .expect({ statusCode: 500, message: 'Internal server error' }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('Failure: data page validation fails', async () => { @@ -189,7 +189,8 @@ describe('List incoming transfers by Safe - Transactions Controller (Unit)', () .get( `/v1/chains/${chain.chainId}/safes/${safe.address}/incoming-transfers/`, ) - .expect({ statusCode: 500, message: 'Internal server error' }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('Should get a trusted ERC20 incoming transfer mapped to the expected format', async () => { diff --git a/src/routes/transactions/__tests__/controllers/list-module-transactions-by-safe.transactions.controller.spec.ts b/src/routes/transactions/__tests__/controllers/list-module-transactions-by-safe.transactions.controller.spec.ts index 84a5295bfc..5d330b78ad 100644 --- a/src/routes/transactions/__tests__/controllers/list-module-transactions-by-safe.transactions.controller.spec.ts +++ b/src/routes/transactions/__tests__/controllers/list-module-transactions-by-safe.transactions.controller.spec.ts @@ -144,11 +144,8 @@ describe('List module transactions by Safe - Transactions Controller (Unit)', () await request(app.getHttpServer()) .get(`/v1/chains/${chainId}/safes/${safeAddress}/module-transactions`) - .expect(500) - .expect({ - statusCode: 500, - message: 'Internal server error', - }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('Get module transaction should return 404', async () => { diff --git a/src/routes/transactions/__tests__/controllers/list-multisig-transactions-by-safe.transactions.controller.spec.ts b/src/routes/transactions/__tests__/controllers/list-multisig-transactions-by-safe.transactions.controller.spec.ts index ce58661154..3d4d8890b3 100644 --- a/src/routes/transactions/__tests__/controllers/list-multisig-transactions-by-safe.transactions.controller.spec.ts +++ b/src/routes/transactions/__tests__/controllers/list-multisig-transactions-by-safe.transactions.controller.spec.ts @@ -167,8 +167,8 @@ describe('List multisig transactions by Safe - Transactions Controller (Unit)', await request(app.getHttpServer()) .get(`/v1/chains/${chainId}/safes/${safeAddress}/multisig-transactions`) - .expect(500) - .expect({ statusCode: 500, message: 'Internal server error' }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('Failure: data page validation fails', async () => { @@ -187,8 +187,8 @@ describe('List multisig transactions by Safe - Transactions Controller (Unit)', await request(app.getHttpServer()) .get(`/v1/chains/${chainId}/safes/${safeAddress}/multisig-transactions`) - .expect(500) - .expect({ statusCode: 500, message: 'Internal server error' }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('Should get a ERC20 transfer mapped to the expected format', async () => { diff --git a/src/routes/transactions/__tests__/controllers/list-queued-transactions-by-safe.transactions.controller.spec.ts b/src/routes/transactions/__tests__/controllers/list-queued-transactions-by-safe.transactions.controller.spec.ts index f4ac93ef59..231be2ee76 100644 --- a/src/routes/transactions/__tests__/controllers/list-queued-transactions-by-safe.transactions.controller.spec.ts +++ b/src/routes/transactions/__tests__/controllers/list-queued-transactions-by-safe.transactions.controller.spec.ts @@ -108,8 +108,8 @@ describe('List queued transactions by Safe - Transactions Controller (Unit)', () await request(app.getHttpServer()) .get(`/v1/chains/${chainId}/safes/${safe.address}/transactions/queued`) - .expect(500) - .expect({ statusCode: 500, message: 'Internal server error' }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('should get a transactions queue with labels and conflict headers', async () => { diff --git a/src/routes/transactions/transactions-history.controller.spec.ts b/src/routes/transactions/transactions-history.controller.spec.ts index b68aff4395..98accebbff 100644 --- a/src/routes/transactions/transactions-history.controller.spec.ts +++ b/src/routes/transactions/transactions-history.controller.spec.ts @@ -196,8 +196,8 @@ describe('Transactions History Controller (Unit)', () => { .get( `/v1/chains/${chain.chainId}/safes/${safeAddress}/transactions/history/`, ) - .expect(500) - .expect({ statusCode: 500, message: 'Internal server error' }); + .expect(502) + .expect({ statusCode: 502, message: 'Bad gateway' }); }); it('Should return only creation transaction', async () => {