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
feat: add mux middlewares and inject http pattern #4290
base: main
Are you sure you want to change the base?
Conversation
Hi, thanks for your PR. However, please open an issue to discuss the problem you're having before submitting a PR. I'm not convinced this is functionality we want, so please open a PR to discuss your specific problem first. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'm happy to go ahead with this feature. I would request that you update the comment on the generated direct-to-implementation registration functions in template.go:
grpc-gateway/protoc-gen-grpc-gateway/internal/gengateway/template.go
Lines 704 to 706 in 33ca56e
// Note: the gRPC framework executes interceptors within the gRPC handler. If the passed in "{{$svc.InstanceName}}Client" | |
// doesn't go through the normal gRPC flow (creating a gRPC client etc.) then it will be up to the passed in | |
// "{{$svc.InstanceName}}Client" to call the correct interceptors. |
to nudge people towards the use of this middleware functionality as an alternative to gRPC interceptors.
Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Looks like we need to regenerate the files after the update? |
It is not possible to regenerate files. Inside |
References to other Issues or PRs
Issue #4319
Have you read the Contributing Guidelines?
Yes
Brief description of what is fixed or changed
Added Middleware for ServerMux
Added the option of passing a HTTP Pattern to the request context
Other comments
Our team encountered a problem that middleware can only be applied to the Mux "http.Handler", which is unsafe for our application. Middleware is triggered regardless of whether a route exists or not, which leads to unnecessary triggering of middlewares. Also in http.Handler middleware there is no way except "defer" to get http pattern, so we added an option to pass HttpPattern in the request context before triggering middlewares.