Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Chain multiple exception filters execution #3252

Closed
ghost opened this issue Oct 24, 2019 · 5 comments
Closed

Chain multiple exception filters execution #3252

ghost opened this issue Oct 24, 2019 · 5 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@ghost
Copy link

ghost commented Oct 24, 2019

Feature Request

Is your feature request related to a problem? Please describe.

The issue is that I am not able to execute two exception filters as the filter themselves are actually single pointcut for the chain.

Describe the solution you'd like

The first option would be to have Filter with a similar syntax to Interceptor such that developers can decide if the Filter is a pointcut or not.
A second option would be to apply a Pipe or an Interceptor at different levels, such as to catch errors from Guard.

What is the motivation/use case for changing the behavior?

A request comes into the following API

req -> TraceMiddleware -> AuthGuard -> LoggingInterceptor -> GQLResolver w/ TransformExceptionFilter(HttpException)

Each step can potentially throw an Exception, not necessarely an HttpException.
The Exception transformation is done in a Filter in order to be able to catch errors coming from any step.

As a new feature, I would like to have a logger to log every exception that is happening, potentially with the ExecutionContext from which is thrown, without having to change my existing filter to comply with SRP and prevent mixing up logging with transformation.
Possible scenarios:

  • A Middleware would not have the ExecutionContext.
  • An ExceptionLoggingInterceptor would not be called to log if the failure point is TraceMiddleware or AuthGuard.
  • A second Filter would not be run together with the TransformExceptionFilter.
@ghost ghost added needs triage This issue has not been looked into type: enhancement 🐺 labels Oct 24, 2019
@kamilmysliwiec
Copy link
Member

Exception filters were designed to be a pointcut. If you want to log every exception and at the same time, have multiple filters registered, wouldn't it be more predictable to use composition/inheritance instead? In case you want to make it even more dynamic, you could create a MethodDecorator that would wrap the actual function.

@ghost
Copy link
Author

ghost commented Oct 28, 2019

Okay, but we can potentially have a chain of inheritance of which is hard to maintain at some later point, so we were wondering if there is a possibility to easy that out.

@kamilmysliwiec
Copy link
Member

You can use composition in this case.

@ghost
Copy link
Author

ghost commented Oct 30, 2019

Okay, thanks for the feedback. We will look into which solutions could be the best way to achieve it.

@ghost ghost closed this as completed Oct 30, 2019
@lock
Copy link

lock bot commented Jan 28, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 28, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

1 participant