Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

suggestion: use a platform-agnostic filter via HTTP adapter abstraction #7

Closed
micalevisk opened this issue Oct 21, 2023 · 1 comment · Fixed by #8
Closed

suggestion: use a platform-agnostic filter via HTTP adapter abstraction #7

micalevisk opened this issue Oct 21, 2023 · 1 comment · Fixed by #8
Labels
enhancement New feature or request hacktoberfest

Comments

@micalevisk
Copy link
Contributor

micalevisk commented Oct 21, 2023

hi! good lib!

as of now, due to the usage of response.type,response.send and so on, this exception filter only works for http adapters that implements such methods on their response object. Express and Fastify has them but 3rd-party http adapters might not have (because they aren't needed for nestjs core AFIAK)

response
.type(PROBLEM_CONTENT_TYPE)
.status(status)
.send({
...objectExtras,
type: [this.baseUri, type || this.getDefaultType(status)]
.filter(Boolean)
.join('/'),
title,
status,
detail,
});

My suggestion is using the HttpAdapterHost instead like the docs shows here: https://docs.nestjs.com/exception-filters#catch-everything

at same time, as we need to inject that dependency into the HttpExceptionFilter, there's no way to add this feature without introducing a breaking change because now one will need to supply that new parameter when using the app.useGlobalFilters bind approach (like described here)

so yeah, it's up to you 😄

btw I'd like to contribute to this project because of Hacktoberfest :p

@Fcmam5 Fcmam5 added enhancement New feature or request hacktoberfest labels Oct 22, 2023
@Fcmam5
Copy link
Owner

Fcmam5 commented Oct 22, 2023

Brilliant! Yes please go ahead and fix it.

Thank you very much for the references!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants