Middleware in design?

[ElectricCookie]

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.

[raphael]

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 :)

[ElectricCookie]

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

[raphael]

This is great, considering only the "transport-agnostic" scenario for a moment the use cases that this would need to support are:

1. A middleware that modifies the request payload
2. A middleware that modifies the response result
3. A middleware that does neither
4. A middleware that does both

So we would need DSL that make it possible to:

• Read request payloads (potentially only reading certain fields)
• Modify request payloads
• Read response results
• Modify response results (potentially only setting certain fields)

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.

[ElectricCookie]

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.

[raphael]

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?

[ElectricCookie]

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).