Skip to content

Commit

Permalink
Return 502 for gateway validation errors (#2111)
Browse files Browse the repository at this point in the history 
Changes server validation-related error codes to `502`:

- Change server validation-related errors codes
  • Loading branch information
@iamacook
iamacook authored Dec 5, 2024
1 parent 77f0e72 commit 22b5f65
Show file tree
Hide file tree
Showing 18 changed files with 87 additions and 124 deletions.
4 changes: 4 additions & 0 deletions src/datasources/balances-api/safe-balances-api.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/routes/balances/balances.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
29 changes: 10 additions & 19 deletions src/routes/chains/chains.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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' });
});
});

Expand Down Expand Up @@ -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' });
});
});

Expand Down
23 changes: 19 additions & 4 deletions src/routes/common/filters/zod-error.filter.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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() })
Expand All @@ -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() })
Expand All @@ -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({
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions src/routes/common/filters/zod-error.filter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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',
});
}
}
Expand Down
56 changes: 16 additions & 40 deletions src/routes/community/community.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down
7 changes: 2 additions & 5 deletions src/routes/contracts/contracts.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
});
});
});
7 changes: 2 additions & 5 deletions src/routes/delegates/delegates.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
7 changes: 2 additions & 5 deletions src/routes/delegates/v2/delegates.v2.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
7 changes: 2 additions & 5 deletions src/routes/estimations/estimations.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
4 changes: 2 additions & 2 deletions src/routes/messages/messages.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
14 changes: 4 additions & 10 deletions src/routes/owners/owners.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
});
});

Expand Down Expand Up @@ -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' });
});
});
});
7 changes: 2 additions & 5 deletions src/routes/safe-apps/safe-apps.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
});
});
});
Loading

0 comments on commit 22b5f65

Please sign in to comment.