From e4360d5632630ceb1dde73cb98f85641402bdade Mon Sep 17 00:00:00 2001 From: Aaron Cook Date: Mon, 3 Mar 2025 20:51:44 +0100 Subject: [PATCH] refactor: use entity that inherently doesn't log --- .../errors/http-exception-no-log.error.ts | 13 ++ .../errors/http-exception-with-log.errors.ts | 16 -- src/domain/safe/safe.repository.ts | 11 +- .../filters/global-error.filter.spec.ts | 19 +- .../common/filters/global-error.filter.ts | 10 +- ...firmations.transactions.controller.spec.ts | 9 + ...tion-by-id.transactions.controller.spec.ts | 9 + ...ransaction.transactions.controller.spec.ts | 9 + .../transaction-verifier.helper.spec.ts | 191 ++++++++---------- .../helpers/transaction-verifier.helper.ts | 113 ++--------- .../transactions/transactions.controller.ts | 4 - 11 files changed, 160 insertions(+), 244 deletions(-) create mode 100644 src/domain/common/errors/http-exception-no-log.error.ts delete mode 100644 src/domain/common/errors/http-exception-with-log.errors.ts diff --git a/src/domain/common/errors/http-exception-no-log.error.ts b/src/domain/common/errors/http-exception-no-log.error.ts new file mode 100644 index 0000000000..ddef81760c --- /dev/null +++ b/src/domain/common/errors/http-exception-no-log.error.ts @@ -0,0 +1,13 @@ +import { HttpException } from '@nestjs/common'; +import type { HttpStatus } from '@nestjs/common'; + +/** + * The following errors combine with out {@link GlobalErrorFilter} to prevent + * logging if thrown. This is because {@link GlobalErrorFilter} catches all + * errors and logs them, but we may want to manually log + */ +export class HttpExceptionNoLog extends HttpException { + constructor(message: string, code: HttpStatus) { + super(HttpExceptionNoLog.createBody(message, undefined!, code), code); + } +} diff --git a/src/domain/common/errors/http-exception-with-log.errors.ts b/src/domain/common/errors/http-exception-with-log.errors.ts deleted file mode 100644 index b9d993aa10..0000000000 --- a/src/domain/common/errors/http-exception-with-log.errors.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { HttpException } from '@nestjs/common'; -import type { HttpStatus } from '@nestjs/common'; - -/** - * The following errors combine with out {@link GlobalErrorFilter} to prevent - * being toggle logging if thrown. This is because the {@link GlobalErrorFilter} - * will catch the error and log it, and we might have logging where thrown. - */ -export class HttpExceptionWithLog extends HttpException { - public log: boolean; - - constructor(args: { message: string; code: HttpStatus; log: boolean }) { - super(args.message, args.code); - this.log = args.log; - } -} diff --git a/src/domain/safe/safe.repository.ts b/src/domain/safe/safe.repository.ts index a6dc6b8d2f..28e76ada4f 100644 --- a/src/domain/safe/safe.repository.ts +++ b/src/domain/safe/safe.repository.ts @@ -35,7 +35,7 @@ import { TransactionVerifierHelper } from '@/routes/transactions/helpers/transac import { IContractsRepository } from '@/domain/contracts/contracts.repository.interface'; import { IConfigurationService } from '@/config/configuration.service.interface'; import { Operation } from '@/domain/safe/entities/operation.entity'; -import { HttpExceptionWithLog } from '@/domain/common/errors/http-exception-with-log.errors'; +import { HttpExceptionNoLog } from '@/domain/common/errors/http-exception-no-log.error'; @Injectable() export class SafeRepository implements ISafeRepository { @@ -639,11 +639,10 @@ export class SafeRepository implements ISafeRepository { args.chainId, ); - const error = new HttpExceptionWithLog({ - message: 'Delegate call is disabled', - code: HttpStatus.UNPROCESSABLE_ENTITY, - log: true, - }); + const error = new HttpExceptionNoLog( + 'Delegate call is disabled', + HttpStatus.UNPROCESSABLE_ENTITY, + ); if (args.proposeTransactionDto.operation === Operation.DELEGATE) { if (!this.isTrustedDelegateCallEnabled) { throw error; diff --git a/src/routes/common/filters/global-error.filter.spec.ts b/src/routes/common/filters/global-error.filter.spec.ts index 810a3132f2..95d2be9b46 100644 --- a/src/routes/common/filters/global-error.filter.spec.ts +++ b/src/routes/common/filters/global-error.filter.spec.ts @@ -17,7 +17,7 @@ import { ConfigurationModule } from '@/config/configuration.module'; import configuration from '@/config/entities/__tests__/configuration'; import { GlobalErrorFilter } from '@/routes/common/filters/global-error.filter'; import { Server } from 'net'; -import { HttpExceptionWithLog } from '@/domain/common/errors/http-exception-with-log.errors'; +import { HttpExceptionNoLog } from '@/domain/common/errors/http-exception-no-log.error'; import { ILoggingService, LoggingService } from '@/logging/logging.interface'; @Controller({}) @@ -29,11 +29,7 @@ class TestController { @Post('no-log-http-exception') noLogHttpException(@Body() body: { code: HttpStatus }): void { - throw new HttpExceptionWithLog({ - message: 'Some no log http exception', - code: body.code, - log: false, - }); + throw new HttpExceptionNoLog('Some no log http exception', body.code); } @Get('non-http-exception') @@ -46,9 +42,11 @@ describe('GlobalErrorFilter tests', () => { let app: INestApplication; let loggingService: ILoggingService; - beforeEach(async () => { + beforeEach(() => { jest.resetAllMocks(); + }); + beforeAll(async () => { const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [TestLoggingModule, ConfigurationModule.register(configuration)], controllers: [TestController], @@ -69,7 +67,7 @@ describe('GlobalErrorFilter tests', () => { await app.init(); }); - afterEach(async () => { + afterAll(async () => { await app.close(); }); @@ -91,7 +89,7 @@ describe('GlobalErrorFilter tests', () => { .post('/no-log-http-exception') .send({ code }) .expect(code) - .expect({ message: 'Some no log http exception' }); + .expect({ message: 'Some no log http exception', statusCode: code }); }); it('non http exception returns correct error code and message', async () => { @@ -112,8 +110,7 @@ describe('GlobalErrorFilter tests', () => { await request(app.getHttpServer()) .post('/http-exception') .send({ code }) - .expect(code) - .expect({ message: 'Some http exception' }); + .expect(code); expect(loggingService.info).not.toHaveBeenCalled(); expect(loggingService.error).toHaveBeenCalledTimes(1); diff --git a/src/routes/common/filters/global-error.filter.ts b/src/routes/common/filters/global-error.filter.ts index e0b6543071..e2d751f811 100644 --- a/src/routes/common/filters/global-error.filter.ts +++ b/src/routes/common/filters/global-error.filter.ts @@ -8,7 +8,7 @@ import { } from '@nestjs/common'; import { ILoggingService, LoggingService } from '@/logging/logging.interface'; import { HttpAdapterHost } from '@nestjs/core'; -import { HttpExceptionWithLog } from '@/domain/common/errors/http-exception-with-log.errors'; +import { HttpExceptionNoLog } from '@/domain/common/errors/http-exception-no-log.error'; @Catch() export class GlobalErrorFilter implements ExceptionFilter { @@ -23,14 +23,14 @@ export class GlobalErrorFilter implements ExceptionFilter { const ctx = host.switchToHttp(); const isHttpException = exception instanceof HttpException; - const isHttpExceptionWithLog = exception instanceof HttpExceptionWithLog; + const isHttpExceptionNoLog = exception.constructor === HttpExceptionNoLog; const httpStatus = - isHttpException || isHttpExceptionWithLog + isHttpException || isHttpExceptionNoLog ? exception.getStatus() : HttpStatus.INTERNAL_SERVER_ERROR; - if (!isHttpExceptionWithLog || exception.log) { + if (!isHttpExceptionNoLog) { const logMessage = { name: exception.name, message: exception.message, @@ -44,7 +44,7 @@ export class GlobalErrorFilter implements ExceptionFilter { } const responseBody = - isHttpException || isHttpExceptionWithLog + isHttpException || isHttpExceptionNoLog ? exception.getResponse() : { code: httpStatus, diff --git a/src/routes/transactions/__tests__/controllers/add-transaction-confirmations.transactions.controller.spec.ts b/src/routes/transactions/__tests__/controllers/add-transaction-confirmations.transactions.controller.spec.ts index 9163333481..58331aed18 100644 --- a/src/routes/transactions/__tests__/controllers/add-transaction-confirmations.transactions.controller.spec.ts +++ b/src/routes/transactions/__tests__/controllers/add-transaction-confirmations.transactions.controller.spec.ts @@ -28,6 +28,8 @@ import { addConfirmationDtoBuilder } from '@/routes/transactions/__tests__/entit import type { Server } from 'net'; import { rawify } from '@/validation/entities/raw.entity'; import { generatePrivateKey, privateKeyToAccount } from 'viem/accounts'; +import { GlobalErrorFilter } from '@/routes/common/filters/global-error.filter'; +import { APP_FILTER } from '@nestjs/core'; describe('Add transaction confirmations - Transactions Controller (Unit)', () => { let app: INestApplication; @@ -47,6 +49,13 @@ describe('Add transaction confirmations - Transactions Controller (Unit)', () => TestLoggingModule, TestNetworkModule, ], + providers: [ + // TODO: Add to all tests to reflect app implementation + { + provide: APP_FILTER, + useClass: GlobalErrorFilter, + }, + ], }).compile(); const configurationService = moduleFixture.get( diff --git a/src/routes/transactions/__tests__/controllers/get-transaction-by-id.transactions.controller.spec.ts b/src/routes/transactions/__tests__/controllers/get-transaction-by-id.transactions.controller.spec.ts index e3cb0b60e2..c285838cca 100644 --- a/src/routes/transactions/__tests__/controllers/get-transaction-by-id.transactions.controller.spec.ts +++ b/src/routes/transactions/__tests__/controllers/get-transaction-by-id.transactions.controller.spec.ts @@ -48,6 +48,8 @@ import { TargetedMessagingDatasourceModule } from '@/datasources/targeted-messag import { rawify } from '@/validation/entities/raw.entity'; import { generatePrivateKey, privateKeyToAccount } from 'viem/accounts'; import { SignatureType } from '@/domain/common/entities/signature-type.entity'; +import { GlobalErrorFilter } from '@/routes/common/filters/global-error.filter'; +import { APP_FILTER } from '@nestjs/core'; describe('Get by id - Transactions Controller (Unit)', () => { let app: INestApplication; @@ -57,6 +59,13 @@ describe('Get by id - Transactions Controller (Unit)', () => { async function initApp(config: typeof configuration): Promise { const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [AppModule.register(config)], + providers: [ + // TODO: Add to all tests to reflect app implementation + { + provide: APP_FILTER, + useClass: GlobalErrorFilter, + }, + ], }) .overrideModule(PostgresDatabaseModule) .useModule(TestPostgresDatabaseModule) diff --git a/src/routes/transactions/__tests__/controllers/propose-transaction.transactions.controller.spec.ts b/src/routes/transactions/__tests__/controllers/propose-transaction.transactions.controller.spec.ts index 9203451372..a407553d83 100644 --- a/src/routes/transactions/__tests__/controllers/propose-transaction.transactions.controller.spec.ts +++ b/src/routes/transactions/__tests__/controllers/propose-transaction.transactions.controller.spec.ts @@ -40,6 +40,8 @@ import { rawify } from '@/validation/entities/raw.entity'; import { generatePrivateKey, privateKeyToAccount } from 'viem/accounts'; import { SignatureType } from '@/domain/common/entities/signature-type.entity'; import { Operation } from '@/domain/safe/entities/operation.entity'; +import { GlobalErrorFilter } from '@/routes/common/filters/global-error.filter'; +import { APP_FILTER } from '@nestjs/core'; describe('Propose transaction - Transactions Controller (Unit)', () => { let app: INestApplication; @@ -49,6 +51,13 @@ describe('Propose transaction - Transactions Controller (Unit)', () => { async function initApp(config: typeof configuration): Promise { const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [AppModule.register(config)], + providers: [ + // TODO: Add to all tests to reflect app implementation + { + provide: APP_FILTER, + useClass: GlobalErrorFilter, + }, + ], }) .overrideModule(PostgresDatabaseModule) .useModule(TestPostgresDatabaseModule) diff --git a/src/routes/transactions/helpers/transaction-verifier.helper.spec.ts b/src/routes/transactions/helpers/transaction-verifier.helper.spec.ts index 106c820798..c22308ae22 100644 --- a/src/routes/transactions/helpers/transaction-verifier.helper.spec.ts +++ b/src/routes/transactions/helpers/transaction-verifier.helper.spec.ts @@ -10,7 +10,7 @@ import { confirmationBuilder } from '@/domain/safe/entities/__tests__/multisig-t import { safeBuilder } from '@/domain/safe/entities/__tests__/safe.builder'; import { proposeTransactionDtoBuilder } from '@/routes/transactions/entities/__tests__/propose-transaction.dto.builder'; import { TransactionVerifierHelper } from '@/routes/transactions/helpers/transaction-verifier.helper'; -import { HttpExceptionWithLog } from '@/domain/common/errors/http-exception-with-log.errors'; +import { HttpExceptionNoLog } from '@/domain/common/errors/http-exception-no-log.error'; import type { IConfigurationService } from '@/config/configuration.service.interface'; import type { DelegatesV2Repository } from '@/domain/delegate/v2/delegates.v2.repository'; import type { ILoggingService } from '@/logging/logging.interface'; @@ -199,11 +199,10 @@ describe('TransactionVerifierHelper', () => { await expect(() => { return target.verifyApiTransaction({ chainId, safe, transaction }); }).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.BAD_GATEWAY, - message: 'Could not calculate safeTxHash', - log: false, - }), + new HttpExceptionNoLog( + 'Could not calculate safeTxHash', + HttpStatus.BAD_GATEWAY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -263,11 +262,7 @@ describe('TransactionVerifierHelper', () => { await expect(() => { return target.verifyApiTransaction({ chainId, safe, transaction }); }).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.BAD_GATEWAY, - message: 'Invalid safeTxHash', - log: false, - }), + new HttpExceptionNoLog('Invalid safeTxHash', HttpStatus.BAD_GATEWAY), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -481,11 +476,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyApiTransaction({ chainId, safe, transaction }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.BAD_GATEWAY, - message: 'Duplicate owners in confirmations', - log: false, - }), + new HttpExceptionNoLog( + 'Duplicate owners in confirmations', + HttpStatus.BAD_GATEWAY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -527,11 +521,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyApiTransaction({ chainId, safe, transaction }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.BAD_GATEWAY, - message: 'Duplicate signatures in confirmations', - log: false, - }), + new HttpExceptionNoLog( + 'Duplicate signatures in confirmations', + HttpStatus.BAD_GATEWAY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -570,11 +563,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyApiTransaction({ chainId, safe, transaction }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.BAD_GATEWAY, - message: 'Could not recover address', - log: false, - }), + new HttpExceptionNoLog( + 'Could not recover address', + HttpStatus.BAD_GATEWAY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -611,11 +603,7 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyApiTransaction({ chainId, safe, transaction }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.BAD_GATEWAY, - message: 'Invalid signature', - log: false, - }), + new HttpExceptionNoLog('Invalid signature', HttpStatus.BAD_GATEWAY), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -650,11 +638,7 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyApiTransaction({ chainId, safe, transaction }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.BAD_GATEWAY, - message: 'Invalid signature', - log: false, - }), + new HttpExceptionNoLog('Invalid signature', HttpStatus.BAD_GATEWAY), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -725,11 +709,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyApiTransaction({ chainId, safe, transaction }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.BAD_GATEWAY, - message: 'Unauthorized address', - log: false, - }), + new HttpExceptionNoLog( + 'Unauthorized address', + HttpStatus.BAD_GATEWAY, + ), ); expect(mockLoggingRepository.error).not.toHaveBeenCalled(); @@ -829,11 +812,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Could not calculate safeTxHash', - log: false, - }), + new HttpExceptionNoLog( + 'Could not calculate safeTxHash', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -908,11 +890,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Invalid safeTxHash', - log: false, - }), + new HttpExceptionNoLog( + 'Invalid safeTxHash', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -1232,11 +1213,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Could not recover address', - log: false, - }), + new HttpExceptionNoLog( + 'Could not recover address', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -1311,11 +1291,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Could not recover address', - log: false, - }), + new HttpExceptionNoLog( + 'Could not recover address', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -1384,11 +1363,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Invalid signature', - log: false, - }), + new HttpExceptionNoLog( + 'Invalid signature', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -1457,11 +1435,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Invalid signature', - log: false, - }), + new HttpExceptionNoLog( + 'Invalid signature', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -1534,11 +1511,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Invalid signature', - log: false, - }), + new HttpExceptionNoLog( + 'Invalid signature', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -1618,11 +1594,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Unauthorized address', - log: false, - }), + new HttpExceptionNoLog( + 'Unauthorized address', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); }); }); @@ -1683,11 +1658,10 @@ describe('TransactionVerifierHelper', () => { await expect( target.verifyProposal({ chainId, safe, proposal }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'eth_sign is disabled', - log: false, - }), + new HttpExceptionNoLog( + 'eth_sign is disabled', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).not.toHaveBeenCalled(); @@ -1770,11 +1744,10 @@ describe('TransactionVerifierHelper', () => { signature: transaction.confirmations![0].signature!, }); }).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Could not calculate safeTxHash', - log: false, - }), + new HttpExceptionNoLog( + 'Could not calculate safeTxHash', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -1838,11 +1811,10 @@ describe('TransactionVerifierHelper', () => { signature: transaction.confirmations![0].signature!, }); }).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Invalid safeTxHash', - log: false, - }), + new HttpExceptionNoLog( + 'Invalid safeTxHash', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -1985,11 +1957,10 @@ describe('TransactionVerifierHelper', () => { signature: transaction.confirmations![0].signature, }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Could not recover address', - log: false, - }), + new HttpExceptionNoLog( + 'Could not recover address', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -2028,11 +1999,10 @@ describe('TransactionVerifierHelper', () => { signature: transaction.confirmations![0].signature!, }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'Invalid signature', - log: false, - }), + new HttpExceptionNoLog( + 'Invalid signature', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).toHaveBeenCalledTimes(1); @@ -2086,11 +2056,10 @@ describe('TransactionVerifierHelper', () => { signature: transaction.confirmations![0].signature!, }), ).rejects.toThrow( - new HttpExceptionWithLog({ - code: HttpStatus.UNPROCESSABLE_ENTITY, - message: 'eth_sign is disabled', - log: false, - }), + new HttpExceptionNoLog( + 'eth_sign is disabled', + HttpStatus.UNPROCESSABLE_ENTITY, + ), ); expect(mockLoggingRepository.error).not.toHaveBeenCalled(); diff --git a/src/routes/transactions/helpers/transaction-verifier.helper.ts b/src/routes/transactions/helpers/transaction-verifier.helper.ts index 5b9b4ea1b9..b1bec1534c 100644 --- a/src/routes/transactions/helpers/transaction-verifier.helper.ts +++ b/src/routes/transactions/helpers/transaction-verifier.helper.ts @@ -19,7 +19,7 @@ import { splitConcatenatedSignatures, splitSignature, } from '@/domain/common/utils/signatures'; -import { HttpExceptionWithLog } from '@/domain/common/errors/http-exception-with-log.errors'; +import { HttpExceptionNoLog } from '@/domain/common/errors/http-exception-no-log.error'; import { ILoggingService, LoggingService } from '@/logging/logging.interface'; import { LogType } from '@/domain/common/entities/log-type.entity'; @@ -139,11 +139,7 @@ export class TransactionVerifierHelper { ...args, safeTxHash: args.transaction.safeTxHash, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.MalformedHash, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.MalformedHash, args.code); } if (safeTxHash !== args.transaction.safeTxHash) { @@ -151,11 +147,7 @@ export class TransactionVerifierHelper { ...args, safeTxHash: args.transaction.safeTxHash, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.HashMismatch, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.HashMismatch, args.code); } } @@ -184,11 +176,7 @@ export class TransactionVerifierHelper { transaction, safeTxHash: args.proposal.safeTxHash, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.MalformedHash, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.MalformedHash, args.code); } if (safeTxHash !== args.proposal.safeTxHash) { @@ -197,11 +185,7 @@ export class TransactionVerifierHelper { transaction, safeTxHash: args.proposal.safeTxHash, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.HashMismatch, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.HashMismatch, args.code); } } @@ -220,11 +204,7 @@ export class TransactionVerifierHelper { transaction: args.transaction, safeTxHash: args.transaction.safeTxHash, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.MalformedHash, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.MalformedHash, args.code); } if (safeTxHash !== args.transaction.safeTxHash) { @@ -232,11 +212,7 @@ export class TransactionVerifierHelper { ...args, safeTxHash: args.transaction.safeTxHash, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.HashMismatch, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.HashMismatch, args.code); } } @@ -263,11 +239,7 @@ export class TransactionVerifierHelper { confirmations: args.transaction.confirmations, type: 'owners', }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.DuplicateOwners, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.DuplicateOwners, args.code); } const uniqueSignatures = new Set( @@ -280,11 +252,7 @@ export class TransactionVerifierHelper { confirmations: args.transaction.confirmations, type: 'signatures', }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.DuplicateSignatures, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.DuplicateSignatures, args.code); } for (const confirmation of args.transaction.confirmations) { @@ -306,11 +274,7 @@ export class TransactionVerifierHelper { const isBlocked = address && this.blocklist.includes(address); if (isBlocked) { - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.BlockedAddress, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.BlockedAddress, args.code); } if ( @@ -325,11 +289,7 @@ export class TransactionVerifierHelper { signerAddress: confirmation.owner, signature: confirmation.signature, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.InvalidSignature, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.InvalidSignature, args.code); } } } @@ -354,22 +314,17 @@ export class TransactionVerifierHelper { safeTxHash: args.proposal.safeTxHash, signature: args.proposal.signature, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.UnrecoverableAddress, - log: false, - }); + throw new HttpExceptionNoLog( + ErrorMessage.UnrecoverableAddress, + args.code, + ); } const recoveredAddresses: Array<`0x${string}`> = []; for (const signature of signatures) { const { v } = splitSignature(signature); if (isEthSignV(v) && !this.isEthSignEnabled) { - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.EthSignDisabled, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.EthSignDisabled, args.code); } const recoveredAddress = await this.recoverAddress({ @@ -386,11 +341,7 @@ export class TransactionVerifierHelper { return this.blocklist.includes(address); }); if (hasBlockedAddresses) { - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.BlockedAddress, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.BlockedAddress, args.code); } const isSender = recoveredAddresses.includes(args.proposal.sender); @@ -401,11 +352,7 @@ export class TransactionVerifierHelper { signerAddress: args.proposal.sender, signature: args.proposal.signature, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.InvalidSignature, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.InvalidSignature, args.code); } const areOwners = recoveredAddresses.every((address) => @@ -432,11 +379,7 @@ export class TransactionVerifierHelper { signerAddress: args.proposal.sender, signature: args.proposal.signature, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.InvalidSignature, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.InvalidSignature, args.code); } private async verifyConfirmationSignature(args: { @@ -448,11 +391,7 @@ export class TransactionVerifierHelper { }): Promise { const { v } = splitSignature(args.signature); if (isEthSignV(v) && !this.isEthSignEnabled) { - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.EthSignDisabled, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.EthSignDisabled, args.code); } const address = await this.recoverAddress({ @@ -468,11 +407,7 @@ export class TransactionVerifierHelper { signerAddress: address, signature: args.signature, }); - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.InvalidSignature, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.InvalidSignature, args.code); } } @@ -502,11 +437,7 @@ export class TransactionVerifierHelper { this.logUnrecoverableAddress(args); } - throw new HttpExceptionWithLog({ - code: args.code, - message: ErrorMessage.UnrecoverableAddress, - log: false, - }); + throw new HttpExceptionNoLog(ErrorMessage.UnrecoverableAddress, args.code); } private logMalformedSafeTxHash(args: { diff --git a/src/routes/transactions/transactions.controller.ts b/src/routes/transactions/transactions.controller.ts index 5f55553adc..7df6371984 100644 --- a/src/routes/transactions/transactions.controller.ts +++ b/src/routes/transactions/transactions.controller.ts @@ -10,7 +10,6 @@ import { ParseIntPipe, Post, Query, - UseFilters, } from '@nestjs/common'; import { ApiOkResponse, ApiQuery, ApiTags } from '@nestjs/swagger'; import { PaginationDataDecorator } from '@/routes/common/decorators/pagination.data.decorator'; @@ -45,15 +44,12 @@ import { TimezoneSchema } from '@/validation/entities/schemas/timezone.schema'; import { TXSMultisigTransaction } from '@/routes/transactions/entities/txs-multisig-transaction.entity'; import { TXSMultisigTransactionPage } from '@/routes/transactions/entities/txs-multisig-transaction-page.entity'; import { TXSCreationTransaction } from '@/routes/transactions/entities/txs-creation-transaction.entity'; -import { GlobalErrorFilter } from '@/routes/common/filters/global-error.filter'; @ApiTags('transactions') @Controller({ path: '', version: '1', }) -// TODO: Globally attach to tests so this can be removed here -@UseFilters(GlobalErrorFilter) export class TransactionsController { constructor(private readonly transactionsService: TransactionsService) {}