-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
add error management for failed Validation Pipes (#169)
* add error management for failed Validation Pipes * format * test exception on ping controller * registered exception filters to policy-engine * format * Update apps/armory/src/shared/filter/application-exception.filter.ts Co-authored-by: William Calderipe <wcalderipe@gmail.com> * ported filter test file --------- Co-authored-by: William Calderipe <wcalderipe@gmail.com>
- Loading branch information
1 parent
da31f53
commit 2d0a9d1
Showing
7 changed files
with
236 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import { ArgumentsHost, Catch, ExceptionFilter, HttpException, Logger } from '@nestjs/common' | ||
import { ConfigService } from '@nestjs/config' | ||
import { Response } from 'express' | ||
import { Config, Env } from '../../armory.config' | ||
|
||
@Catch(HttpException) | ||
export class HttpExceptionFilter implements ExceptionFilter { | ||
private logger = new Logger(HttpExceptionFilter.name) | ||
|
||
constructor(private configService: ConfigService<Config, true>) {} | ||
|
||
catch(exception: HttpException, host: ArgumentsHost) { | ||
const ctx = host.switchToHttp() | ||
const response = ctx.getResponse<Response>() | ||
const status = exception.getStatus() | ||
|
||
const isProduction = this.configService.get('env') === Env.PRODUCTION | ||
|
||
this.logger.error(exception) | ||
|
||
response.status(status).json( | ||
isProduction | ||
? { | ||
statusCode: status, | ||
message: exception.message, | ||
response: exception.getResponse() | ||
} | ||
: { | ||
statusCode: status, | ||
message: exception.message, | ||
response: exception.getResponse(), | ||
stack: exception.stack | ||
} | ||
) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,6 @@ export class AppController { | |
this.logger.log({ | ||
message: 'Received ping' | ||
}) | ||
|
||
return 'pong' | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
82 changes: 82 additions & 0 deletions
82
apps/policy-engine/src/shared/filter/__test__/unit/application-exception.filter.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
import { ArgumentsHost, HttpStatus } from '@nestjs/common' | ||
import { HttpArgumentsHost } from '@nestjs/common/interfaces' | ||
import { ConfigService } from '@nestjs/config' | ||
import { Response } from 'express' | ||
import { mock } from 'jest-mock-extended' | ||
import { Config, Env } from '../../../../policy-engine.config' | ||
import { ApplicationException } from '../../../exception/application.exception' | ||
import { ApplicationExceptionFilter } from '../../application-exception.filter' | ||
|
||
describe(ApplicationExceptionFilter.name, () => { | ||
const exception = new ApplicationException({ | ||
message: 'Test application exception filter', | ||
suggestedHttpStatusCode: HttpStatus.INTERNAL_SERVER_ERROR, | ||
context: { | ||
additional: 'information', | ||
to: 'debug' | ||
} | ||
}) | ||
|
||
const buildArgumentsHostMock = (): [ArgumentsHost, jest.Mock, jest.Mock] => { | ||
const jsonMock = jest.fn() | ||
const statusMock = jest.fn().mockReturnValue( | ||
mock<Response>({ | ||
json: jsonMock | ||
}) | ||
) | ||
|
||
const host = mock<ArgumentsHost>({ | ||
switchToHttp: jest.fn().mockReturnValue( | ||
mock<HttpArgumentsHost>({ | ||
getResponse: jest.fn().mockReturnValue( | ||
mock<Response>({ | ||
status: statusMock | ||
}) | ||
) | ||
}) | ||
) | ||
}) | ||
|
||
return [host, statusMock, jsonMock] | ||
} | ||
|
||
const buildConfigServiceMock = (env: Env) => | ||
mock<ConfigService<Config, true>>({ | ||
get: jest.fn().mockReturnValue(env) | ||
}) | ||
|
||
describe('catch', () => { | ||
describe('when environment is production', () => { | ||
it('responds with exception status and short message', () => { | ||
const filter = new ApplicationExceptionFilter(buildConfigServiceMock(Env.PRODUCTION)) | ||
const [host, statusMock, jsonMock] = buildArgumentsHostMock() | ||
|
||
filter.catch(exception, host) | ||
|
||
expect(statusMock).toHaveBeenCalledWith(exception.getStatus()) | ||
expect(jsonMock).toHaveBeenCalledWith({ | ||
statusCode: exception.getStatus(), | ||
message: exception.message, | ||
context: exception.context | ||
}) | ||
}) | ||
}) | ||
|
||
describe('when environment is not production', () => { | ||
it('responds with exception status and complete message', () => { | ||
const filter = new ApplicationExceptionFilter(buildConfigServiceMock(Env.DEVELOPMENT)) | ||
const [host, statusMock, jsonMock] = buildArgumentsHostMock() | ||
|
||
filter.catch(exception, host) | ||
|
||
expect(statusMock).toHaveBeenCalledWith(exception.getStatus()) | ||
expect(jsonMock).toHaveBeenCalledWith({ | ||
statusCode: exception.getStatus(), | ||
message: exception.message, | ||
context: exception.context, | ||
stack: exception.stack | ||
}) | ||
}) | ||
}) | ||
}) | ||
}) |
52 changes: 52 additions & 0 deletions
52
apps/policy-engine/src/shared/filter/application-exception.filter.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import { ArgumentsHost, Catch, ExceptionFilter, LogLevel, Logger } from '@nestjs/common' | ||
import { ConfigService } from '@nestjs/config' | ||
import { Response } from 'express' | ||
import { Config, Env } from '../../policy-engine.config' | ||
import { ApplicationException } from '../../shared/exception/application.exception' | ||
|
||
@Catch(ApplicationException) | ||
export class ApplicationExceptionFilter implements ExceptionFilter { | ||
private logger = new Logger(ApplicationExceptionFilter.name) | ||
|
||
constructor(private configService: ConfigService<Config, true>) {} | ||
|
||
catch(exception: ApplicationException, host: ArgumentsHost) { | ||
const ctx = host.switchToHttp() | ||
const response = ctx.getResponse<Response>() | ||
const status = exception.getStatus() | ||
const isProduction = this.configService.get('env') === Env.PRODUCTION | ||
|
||
this.log(exception) | ||
|
||
response.status(status).json( | ||
isProduction | ||
? { | ||
statusCode: status, | ||
message: exception.message, | ||
context: exception.context | ||
} | ||
: { | ||
statusCode: status, | ||
message: exception.message, | ||
context: exception.context, | ||
stack: exception.stack, | ||
...(exception.origin && { origin: exception.origin }) | ||
} | ||
) | ||
} | ||
|
||
// TODO (@wcalderipe, 16/01/24): Unit test the logging logic. For that, we | ||
// must inject the logger in the constructor via dependency injection. | ||
private log(exception: ApplicationException) { | ||
const level: LogLevel = exception.getStatus() >= 500 ? 'error' : 'warn' | ||
|
||
if (this.logger[level]) { | ||
this.logger[level](exception.message, { | ||
status: exception.getStatus(), | ||
context: exception.context, | ||
stacktrace: exception.stack, | ||
origin: exception.origin | ||
}) | ||
} | ||
} | ||
} |
41 changes: 41 additions & 0 deletions
41
apps/policy-engine/src/shared/filter/http-exception.filter.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import { ArgumentsHost, Catch, ExceptionFilter, HttpException, Logger } from '@nestjs/common' | ||
import { ConfigService } from '@nestjs/config' | ||
import { Response } from 'express' | ||
import { Config, Env } from '../../policy-engine.config' | ||
|
||
@Catch(HttpException) | ||
export class HttpExceptionFilter implements ExceptionFilter { | ||
private logger = new Logger(HttpExceptionFilter.name) | ||
|
||
constructor(private configService: ConfigService<Config, true>) {} | ||
|
||
catch(exception: HttpException, host: ArgumentsHost) { | ||
const ctx = host.switchToHttp() | ||
const response = ctx.getResponse<Response>() | ||
const status = exception.getStatus() | ||
|
||
const isProduction = this.configService.get('env') === Env.PRODUCTION | ||
|
||
this.logger.error({ | ||
message: exception.message, | ||
stack: exception.stack, | ||
response: exception.getResponse(), | ||
statusCode: status | ||
}) | ||
|
||
response.status(status).json( | ||
isProduction | ||
? { | ||
statusCode: status, | ||
message: exception.message, | ||
response: exception.getResponse() | ||
} | ||
: { | ||
statusCode: status, | ||
message: exception.message, | ||
response: exception.getResponse(), | ||
stack: exception.stack | ||
} | ||
) | ||
} | ||
} |