Open ElectricCookie opened 6 months ago
That makes a lot of sense! A goal of the Goa design is to ensure that design code is never used at runtime, the DSL is only used for code generation. This makes it possible to ensure that the generated code follows an expected pattern and that in particular there is clear layer enforcements between transport and non-transport code.
So applying this to middlewares there could be a couple of non-mutually exclusive approaches:
There could be a new "Middleware" DSL that can be applied to non-transport or transport specific DSL, something like:
var _ = Service("calc", func() {
Middleware("ErrorHandler", "The ErrorHandler middleware reports unexpected errors.")
})
This would cause Goa to generate hooks for every service method that the user provides the implementation to (something akin to how the security middlewares work today). the Middleware
DSL could be applied at the API
, Service
or Method
level and would cause hooks to be generated for all the methods in scope. The user provided function signature would be:
func ErrorHandler(goa.Endpoint) goa.Endpoint
Similarly the Middleware
function could be applied at the transport level:
var _ = Service("calc", func() {
HTTP(func() {
Middleware("ErrorHandler", "The ErrorHandler middleware reports unexpected errors.")
})
GRPC(func() {
Middleware("ErrorHandler", "The ErrorHandler middleware reports unexpected errors.")
})
})
In this case the hooks would be transport specific:
func HTTPErrorHandler(http.Handler) http.Handler
func GRPCErrorHandler() grpc.UnaryServerInterceptor // (or grpc.StreamServerInterceptor)
Alternatively (or complementary) there could be a set of "known" middlewares that have specific DSL, or a generic DSL that identifies the middleware:
var _ = Service("calc", func() {
Middleware(ErrorHandler)
})
These specific middleware implementations might have different hook signatures, for example in this case:
func ErrorHandler(err error)
There might be other approaches, this is what comes to mind first :)
This is exactly what I had in mind - and now that you mention it the security middleware is already really close to this!
I like the idea of having several places to put the middleware (API, Service and Method).
While we are dreaming:
(1) I currently have some middleware that adds values to the context, which is sometimes error prone (forgetting the middleware leads to nils..) maybe we could design the middleware function to produce a result-type? This could then be passed to the method in a type safe manner?
var FooExtractor = Middleware("fooExtractor",func(){
// Use a result to signal that the middleware has an output
Result(func(){
// Field numbers are probably not necessary, since the result is not exposed to GRPC directly
Field(0,"extractedFoo",String)
})
Error("noFoo","No foo was found in the payload")
})
Method("foo",func() {
Middleware(FooExtractor)
})
which could result in
func (s *ExampleService) Foo(ctx context.Context, *p example.FooPayload) (res *example.FooResult, err error){
extractedFoo := example.FooExtractorResult(ctx)
}
alternatively, for an even cleaner experience the result of the middleware could be merged into the payload type under the name of the middleware.
func (s *ExampleService) Foo(ctx context.Context, *p example.FooPayload) (res *example.FooResult, err error){
extractedFoo := p.fooExtractor.extractedFoo
}
(2)If we wanted to go even further one could add a mechanism for making parameters in middlewares possible. This would require that a middleware can accept parameters (making it now very close in charachteristics to a method itself)
var FooExtractor = Middleware("fooExtractor",func(){
Payload(func(){
Attribute("fooSeed",String)
Required("fooSeed")
})
// Use a result to signal that the middleware has an output
Result(func(){
// Field numbers are probably not necessary, since the result is not exposed to GRPC directly
Attribute("extractedFoo",String)
})
Error("noFoo","No foo was found in the payload")
})
This would then require a check that any method using the FooExtractor
middleware has an attribute called fooSeed
in its payload and that the types match.
This kind of feature would make many crud application a lot simpler because any endpoint that handles single item operations (GET /item/:id
, PATCH /item/:id
, DELETE /item/:id/
...) could then use a middleware to fetch the item with the specified id using a middleware and provide it to the handler in a very convenient manner.
(3) Since now a middleware shares basically all mechanics of a method we could also use methods directly so instead of
var FooExtractor = Middleware(...)
var FooExtractor = Method(....)
and then implement the Middleware
DSL to allow the user to add any method as middleware in another method definition.
The checks on type compatibility would still need to apply (meaning that the parameters of the middleware have to be a subset of the method using the middleware).
This would fit very nicely in the already existing DSL in my opinion 🥳 curious about your thoughts
This is great, considering only the "transport-agnostic" scenario for a moment the use cases that this would need to support are:
So we would need DSL that make it possible to:
The FooExtractor
example above only supports use cases 1. and 3. Here an inspired but revised proposal that would support all the use cases:
The new RequestInterceptor
DSL defines a middleware that reads and potentially modifies the method payload. It would support the following sub-DSL:
Payload
defines the the interceptor payload, that is what is read from the request payload. It would work mechanically like Body
in HTTP
, you could specify a list of request payload attributes, a single attribute by name or the entire payload type.Result
indicates what the middleware modifies in request payload, and works similarly to the above: the interceptor could modify a single attribute, many attributes or the entire payload.Similarly the new ResponseInterceptor
DSL defines an interceptor that reads and potentially modifies the method result. It would also support a Payload
and Result
sub-DSL where Payload
indicates what to read from the method result and Result
what to send back to the client.
There could be both a RequestInterceptor
and ResponseInterceptor
defined on a method. The request interceptor could add data to the context that the response interceptor could use which allows for use cases where state needs to be communicated (e.g. measuring the time it took to run the method requires capturing time.Now()
in the request interceptor and in the response interceptor then subtracting).
Putting it all together:
var SomeRequestInterceptor = RequestInterceptor("SomeRequestInterceptor", func() {
Payload(func() {
Attribute("a") // Read payload attribute `a` - must be a payload attribute
Attribute("b") // Read payload attribute `b`
})
// OR
// Payload("a") // Read payload attribute `a` and uses that as interceptor argument (instead of object)
// OR
// Payload(PayloadType) // Read entire payload type - must be the same as the method payload type
Result(func() {
Attribute("c") // Modifies payload attribute `c` - must be a payload attribute
})
})
var SomeResponseInterceptor = ResponseInterceptor("SomeResponseInterceptor", func() {
Payload(func() {
Attribute("a") // Read result attribute `a` - must be a result attribute
Attribute("b") // Read result attribute `b`
})
// OR
// Payload("a") // Read result attribute `a` and uses that as interceptor argument (instead of object)
// OR
// Payload(PayloadType) // Read entire result type - must be the same as the method result type
Result(func() {
Attribute("c") // Modifies result attribute `c` - must be a result attribute
})
})
The above would cause the following hooks to be generated:
func SomeRequestInterceptor(ctx context.Context, payload RequestInterceptorPayloadType) (context.Context, RequestInterceptorResultType, error)
Note that the interceptor returns a context that is then passed on to both the method and any response interceptor. If a request interceptor returns an error then the method is not invoked - instead the error is handled by Goa the same way a method error is (that is if it's a designed error then the corresponding status code is returned otherwise a 500 gets returned).
func SomeResponseInterceptor(ctx context.Context, payload ResponseInterceptorPayloadType) ( ResponseInterceptorResultType, error)
The response interceptor has a similar signature, there is just no context returned in this case.
Additionally there can be multiple request and/or response interceptors defined on a single method. They get run in the order in which they are defined.
This is looking really cool! Very good observation that my proposal was only covering a “slice” of what is considered middleware.
What do you think of some mechanism to allow “type safe” interaction with context?
Is there a need for interceptors that can pass errors to the following methods? Maybe one could have an error dsl that is non-critical that will not cancel the method from being called if the middleware fails.
What do you think of some mechanism to allow “type safe” interaction with context?
My knee-jerk reaction would be that this would be out-of-scope of Goa. Different use cases might call for different ways of doing this, in particular the actual data types are probably not generic and trying to build something that can cater to all use cases would add undue complexity (to Goa and to user code). But I could be missing something...
Is there a need for interceptors that can pass errors to the following methods? Maybe one could have an error dsl that is >non-critical that will not cancel the method from being called if the middleware fails.
At the end of the day the middleware is either able to transform the request or response or cannot process the request which would be an exception. That is an expected error case should be communicated via the transform (or non-transform) of the payload. In the extreme case the payload could have a field that indicates the outcome although I would expect the need for this to be fairly rare?
My knee-jerk reaction would be that this would be out-of-scope of Goa. Different use cases might call for different ways of doing this, in particular the actual data types are probably not generic and trying to build something that can cater to all use cases would add undue complexity (to Goa and to user code). But I could be missing something...
I was thinking of something like described above
func (s *ExampleService) Foo(ctx context.Context, *p example.FooPayload) (res *example.FooResult, err error){
extractedFoo := p.fooExtractor.extractedFoo
}
where goa directly injects the result from the middleware into the payload object or
func (s *ExampleService) Foo(ctx context.Context, *p example.FooPayload) (res *example.FooResult, err error){
extractedFoo := example.FooExtractorResult(ctx)
}
where goa generates FooExtractorResult(ctx)
which under the hood calls ctx.Value(...)
with the correct types.
I would prefer the first option since then, a removal of a middleware or a change is directly visible in the implementation function (i.e. the property of the FooPayload does no longer exist)
At the end of the day the middleware is either able to transform the request or response or cannot process the request which would be an exception. That is an expected error case should be communicated via the transform (or non-transform) of the payload. In the extreme case the payload could have a field that indicates the outcome although I would expect the need for this to be fairly rare?
Fair point. I think error DSLs in middleware make sense (e.g. a permission checking middleware will often throw 403s), but maybe non critical errors can be passed via the result (e.g. a cache miss).
I'm really enjoying development with goa. ❤️
I wanted to move some repeating code to a middleware that is used by several methods. I was wondering why the middleware is not part of the design file? Currently the registration in the main.go feels a bit decoupled. Looking at the design or the implementation does not easily reveal which middleware is running for a specific endpoint.
My current workaround is just calling a method at the top of an endpoint, but this feels a bit against the "design first" philosophy.