Returning redirect or rewrite response doesn't stop subsequent middleware from executing

[marko-hologram]

First of all I gotta say I absolutely love this library. It has helped me clean up middleware in projects a lot. I'm definitely going to be using it all the time.

Describe the bug

Returning a NextResponse.redirect() or NextResponse.rewrite() from middleware doesn't stop subsequent middleware from executing when in a middleware chain

To Reproduce

Clone this reproduction repo: https://github.com/marko-hologram/nemo-middleware-chain-bug

npm install

npm run dev

Steps to reproduce the behavior:

1. Go to http://localhost:3000
2. Click on "Admin" link
3. Observe that two messages are logged into the terminal where the app is running, even though second middleware shouldn't even run because first one returns a redirect response
4. Click on "Dashboard" link
5. Observe that two messages are logged into the terminal where the app is running, even though second middleware shouldn't even run because first one returns a rewrite response

Expected behavior

If NextResponse.redirect() or NextResponse.rewrite() is returned from a middleware in a middleware chain, all other middleware after it should not be executed.

There might be a case that I'm missing where it makes sense to have additional middleware run after a rewrite, but I'm quite sure after a redirect, no additional middleware should run. I did find this older issue (https://github.com/z4nr34l/nemo/issues/17) where apparently the redirect issue was fixed, but the bug might have been re-introduced after that.

Ideally, it would be great if maybe we could return some special type of response that would break the middleware chain because middleware chain doesn't need to always just break on redirect or rewrite, but maybe based on some custom condition. For example, in the app I'm working on, there is some special case on a route where only admins get some special treatment that needs to happen in middleware. So there I just need to check if user is admin and if it is, I want to execute additional middleware.

I know I could do each middleware on its own and do all checks there, but maybe I want to have smaller middleware that can be composed like this:

// Rest omitted for brevity
"/admin/*": [isUserAdminMiddleware, anotherAdminSpecialMiddleware],
"/admin-only": [isUserAdminMiddleware, additionalAdminOnlyMiddleware]

Unfortunately, with the current implementation there is no baked in solution to skip anotherAdminSpecialMiddleware or additionalAdminOnlyMiddleware if isUserAdminMiddleware doesn't satisfy some check. But this is a bit offtopic here.

Screenshots

Here is a screenshot of few console logs from my minimal reproduction code. "This is an additional middleware for admin routes" gets logged even though it shouldn't.

Desktop (please complete the following information):

• OS: Windows
• Browser Chrome, Firefox
• Version Latest

Additional context

EDIT: Ok I noticed my createSkippableMiddleware function isn't actually working because middleware doesn't properly execute one after another, but rather executes async and depends on race conditions. Will fix it at some point, but the idea still stands I guess.

I did find a workaround by using context and setting a value on it that I use to check if middleware should run and then in middleware that I want to specify "hey, after this middleware don't run anything else" I do call a function to set that flag in context. Then those middleware that can be skippable I just create using my createSkippableMiddleware function that just returns the response if that context key is set and doesn't actually run that middleware's code.

const MIDDLEWARE_SKIP_KEY = "__skipMiddlewareExecution";

/**
 * Create middleware that is skipped if some middleware before it in the middleware chain sets a flag to tell future middleware to skip execution.
 *
 * Useful for better middleware composition so that each middleware can do smaller focused checks and so that same middleware can be used in multiple middleware chains.
 */
const createSkippableMiddleware = (middlewareFunction: MiddlewareFunction) => {
  return async ({ context, response, event, request }: MiddlewareFunctionProps) => {
    if (context.get(MIDDLEWARE_SKIP_KEY)) {
      return response;
    }

    return middlewareFunction({ context, response, event, request });
  };
};

const stopFutureMiddlewareExecution = (context: Map<string, unknown>) => {
  context.set(MIDDLEWARE_SKIP_KEY, true);
};

I did leave this custom code in my reproduction repo just to showcase how this works. You can clearly see that "This is an additional middleware that won't be logged after the first middleware" message isn't logged when you click on "Skippable Middleware" link on homepage.

[linear[bot]]

NEMO-25 Returning redirect or rewrite response doesn't stop subsequent middleware from executing

[thenbe]

I'm quite sure after a redirect, no additional middleware should run.

This is what I was expecting as well.

For example, you might want to run a middleware very early in order to redirect users who are heading to old routes (routes which no longer exist), to a different route. Once that middleware runs and attempts to redirect, you no longer want to run any other middleware for that request.