feat(router): Rework middlewares so they work as an onion

[javorszky]

Closes #79

• replaces the Before and After middlewares with a single Middleware type that takes a vk.HandlerFunc and returns a vk.HandlerFunc
• Reworks existing middlewares into this pattern
• Adds a test to make sure that the functionality works both on individual handlers, and on the global server as well
• Adds tests that show how to order the middlewares and what the execution order is like.

Reworked the vektor middlewares. Previously vektor's middlewares were "beads on a string". This change turns them into "layers of an onion". I expect a bunch of our services might need some changing, but generally this will enable us to do proper logging / error handling / panic recovery / metrics / tracing because we can start a trace and immediately defer stop them because the actual handler is encapsulated.

In the following case the middlewares are going to be ran in the following order:

g := vk.Group("/b").Middleware(
    midWare1,
    midWare2,
    midWareAfter1,
    midWareAfter2,
)
g.GET("/central", vk.WrapMiddleware([]vk.Middleware{
    handlerMid1,
    handlerMid2,
    handlerAfter1,
    handlerAfter2,
), actualHandler)

Request
    |
    v
G() midWare1
    |
    v
G() midWare2
    |
    v
handlerMid1
    |
    v
handlerMid2
    |
    v
(((--- Actual Handler ---)))
    |
    v
handlerAfter2
    |
    v
handlerAfter1
    |
    v
G() midWareAfter2
    |
    v
G() midWareAfter1
    |
    v
Response

The later a middleware it is in the []vk.Middleware{} list, the closer it is to the handler.

Before type middleware

The middlewares usually look like this (putting the wrapped handler on the tail):

middleware(responsewriter, request) {
    // do stuff in middleware

   return wrappedHandler(responsewriter, request)
}

After type mw

The after type middlewares look like this, putting the wrapped handler on the leading edge:

middleware(responsewriter, request) {
    res, err := wrappedHandler(responsewriter, request)

    // do stuff in the middleware
}

Surround type mw

For logging, tracing, metrics, panic recovery middlewares, they will look like this:

middleware(responsewriter, request) {
    // do stuff here like start a span, start a logging context, set attributes on metrics, whatever

    res, err := wrappedHandler(responsewriter, request) // do the actual work here

    // do more stuff here, like finish span, check for errors, check for panics, whatever
}

[cohix]

From a first glance this seems very reasonable, my question is about the PR description where you have the handlerAfter2, handlerAfter1, G() midWareAfter2, G() midWareAfter1 running after the actual handler, and It's unclear to me how the router makes that distinction since they're all passed in as a single []Middleware.

Does the "before/after" depend on the specific implementation of the middleware functions themselves? I.e. in order to determine if some logic was running before or after the actual handler, would I need to look at each individual middleware function?

[javorszky]

Does the "before/after" depend on the specific implementation of the middleware functions themselves?

Yes. I did write about this in the Before type middleware / After type / Surround type middlewares section (message recently updated to include headings) of the PR message 🙂 . The only thing the order within the slice controls is its proximity to the handler.

vk.WrapMiddleware([]vk.Middleware{
    before1,
    after1,
    after2,
    before2,
    before3,
    after3,
}, actualHandler)

would look like this

(before1 ( ( (before2 (before3 ( (actualHandler) after3) ) ) after2) after1) )

not sure if this is a better visualisation

[danielledeleo]

These changes look reasonable, and it is indeed quite elegant once you wrap your head around it. I think the weakness of this technique is that it's difficult to see at glance the order in which middleware functions are called and are returned. I'm wondering if we could keep the same WrapMiddleware signature but make it more explicit with (for example) WrapBefore and WrapAfter methods that otherwise together have the same behaviour?

Aside: I'd like to hear more about how you settled on this design.

[javorszky]

if we could keep the same WrapMiddleware signature but make it more explicit with (for example) WrapBefore and WrapAfter methods that otherwise together have the same behaviour?

I would advise against it. That separation is one of the reasons I wanted to rewrite this. Whether a middleware runs before, or after, or both, compared to the wrapped handler is up to how the handler is written, ie where the wrappedHandler(request, ctx) call is in relation to the other code in the middleware handler.

The individual middlewares should be responsible for deciding whether they need to run before, after, or on both sides of the wrapped handler based on what their purpose is. For example, trying to put a cors middleware in the after hanlders list would make no sense. Having a wrapbefore and wrapafter method means we now have two separate ways of specifying what's what, and the order in which the middlewares in each are defined would still matter, so that wouldn't solve the initial issue.

I think this is a case where we'll get good at it the more we use it, and both public documentation, and inline documentation at important places will help.

How did I settle on this

Ardan Labs blueprint

It's a slightly modified copy of how it happens in the ardanlabs/service blueprint repository:

• handler wrapping: https://github.com/ardanlabs/service/blob/master/foundation/web/middleware.go
• middleware implementations: metrics: https://github.com/ardanlabs/service/blob/master/business/web/v1/mid/metrics.go (see same folder for the others)
• handler declarations: https://github.com/ardanlabs/service/blob/master/app/services/sales-api/handlers/v1/v1.go#L38

Bill also gives a really deep justification and reasoning on why that pattern should be used in the course.

Echo framework

Echo, another web framework I started out coding in Go, has a similar onion architecture: https://echo.labstack.com/cookbook/middleware/. In there the c.next call is the wrapped handler.

Typeform

At Typeform the service I built was also similarly layered, though that one was "take a template, and process each different type of tag from data from different other services in layers", which made a lot of sense and was very customizable and flexible.

Template -> substitute usernames, pass it on -> substitute answers, pass it on -> substitute data from the form owner, pass it on -> initialize the whole thing, and then back again.

At any point if there was an error up the chain, we could short circuit and not do further work that wasn't needed.

Laravel

Laravel's middleware follows the same pattern: https://laravel.com/docs/9.x/middleware

Meta stuff

It's essentially the chain of responsibility pattern: https://refactoring.guru/design-patterns/chain-of-responsibility as well as the proxy pattern: https://refactoring.guru/design-patterns/proxy as well as the decorator https://refactoring.guru/design-patterns/decorator

The good thing is is that in any of these can decide to not call the next / wrapped handler, for example auth fails, cors fails, or something else.

https://www.youtube.com/watch?v=MBTgNEeJEWQ

[javorszky]

Closed because #101 is better