Should handler.Middleware be able to prevent the actual interaction handler from execution?

[jkdlgy]

Currently, the handler.Mux executes middleware before and independently from the resolved handler (https://github.com/disgoorg/disgo/blob/master/handler/mux.go#L92). In other words, middlewares can't stop the execution chain completely and prevent the resolved handler from running.
Allowing middlewares to prevent the resolved handler from running would be useful for different in-middleware checks which include rate-limiting user interactions, checking if user has access to the command or if the command is allowed to be executed in guild/channel, etc. This pattern can be seen in usual http request router, which have various auth checks inside middleware, each able to abort the request and return certain response (401/403/redirect/etc).
While It is indeed possible to abort interaction in middleware by sending interaction response, which, in turn, will "prevent" the final handler func from sending the response again (any attempt to respond to said interaction further will result in "interaction already acknowledged" discord API error), the handler will still, well, run.
Besides, it's also possible to move such checks into handlers, or wrap handler functions into decorator functions which would do the same. The former introduces a huge amount of boilerplate, though, while the latter is almost exactly what middlewares are actually meant to do, at least in my opinion.
I see two ways of doing this:

1. Change the middleware func return type to error and check if the whole middleware chain resulted in error before executing the handler. Something along the lines of:

type Handler func(e *events.InteractionCreate) error
type Middleware func(next Handler) Handler

func exampleMiddleware(next handler.Handler) handler.Handler {
	return func(event *events.InteractionCreate) error {
		if some_check {
			_ = event.Respond(
				discord.InteractionResponseTypeCreateMessage,
				discord.NewMessageCreateBuilder().
					SetContent("Some check failed, you are not allowed to use this").
					SetEphemeral(true).
					Build(),
			)
			return errors.New("user failed some check")
		}

		return next(event)
	}
}

func (r *Mux) Handle(path string, variables map[string]string, e *events.InteractionCreate) error {
	path = parseVariables(path, r.pattern, variables)
	middlewares := func(event *events.InteractionCreate) error {
                return nil
        }
	for i := len(r.middlewares) - 1; i >= 0; i-- {
		middlewares = r.middlewares[i](middlewares)
	}
	if err := middlewares(e); err != nil {
                return errors.Wrap(err, "middleware chain resulted in error")
        }

	for _, route := range r.routes {
		if route.Match(path, e.Type()) {
			return route.Handle(path, variables, e)
		}
	}
	if r.notFoundHandler != nil {
		return r.notFoundHandler(e)
	}
	return nil
}

I don't like this option because it would be difficult to understand exactly what the error returned from the middleware means.

2. Again, change the middleware func return type to error, and wrap the final handler func into the middleware chain so that the middlewares are able to simply not call it at all:

type Handler func(e *events.InteractionCreate) error
type Middleware func(next Handler) Handler

func exampleMiddleware(next handler.Handler) handler.Handler {
	return func(event *events.InteractionCreate) error {
		if some_check {
			return event.Respond(
				discord.InteractionResponseTypeCreateMessage,
				discord.NewMessageCreateBuilder().
					SetContent("Some check failed, you are not allowed to use this").
					SetEphemeral(true).
					Build(),
			)
		}

		return next(event)
	}
}

func (r *Mux) Handle(path string, variables map[string]string, e *events.InteractionCreate) error {
	handlerChain := func(event *events.InteractionCreate) error {
		path = parseVariables(path, r.pattern, variables)

		for _, route := range r.routes {
			if route.Match(path, e.Type()) {
				return route.Handle(path, variables, e)
			}
		}
		if r.notFoundHandler != nil {
			return r.notFoundHandler(e)
		}
		return nil
	}

	for i := len(r.middlewares) - 1; i >= 0; i-- {
		handlerChain = r.middlewares[i](handlerChain)
	}

	return handlerChain(e)
}

Http routers usually work this way. Furthermore, this implementation would allow to do some complex actions inside the middleware. For example, make a middleware which automatically defers interaction if the handler takes too much time to execute, like in Arikawa lib: (this is just an example, I do understand the same deferring logic can't be applied to disgo because the interaction response is sent via method call from inside the handler, not returned from it).
Personally, I've implemented the second option in my custom router. To be precise, I exactly copied the code from your handler, only changing the middleware part.
I'd like to know your opinion on this topic: whether you'd like to change the implementation of the default handler.Mux or leave it to end users to decide if they want/need to implement it themselves.

[topi314]

Hey, I completely missed that when implementing the router, middlewares should definitely be able to cancel handlers. I'm gonna look into your 2 options within the next days and get back to you with a proposal/implementation

If you have further input or want to talk more about it let me know here or in our discord

[topi314]

Sorry for the delay here,
I like your second approach
I'm not sure if I like to have an error return in the middleware func, but it's prob fine.
I added it in cb10239 and will be in the next release

[topi314]

released in https://github.com/disgoorg/disgo/releases/tag/v0.16.0