Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to add a custom middleware after nextjs RequestHandler #6290

Closed
batanun opened this issue Feb 13, 2019 · 4 comments
Closed

Comments

@batanun
Copy link

batanun commented Feb 13, 2019

Feature request

Description of the problem at hand

Our website is quite complex, with many URL's that require custom logic. But there are also a lot of URL's that should be handled by the nextjs RequestHandler (like requests to a specific react page, babel content, static content, etc).
But there is no way to define a simple URL-pattern for either of these cases. Ie, we can't do like this:

  server.get(someRegexForCustomUrl, (req, res) => {
    //Custom logic...
  })

  server.get('*', (req, res) => {
    return handle(req, res)
  })

...because there is no regex that can match all cases of custom URLs, and nothing else (ie not accidently catch stuff that nextjs should handle).

And we can't do like this either:

  server.get(someRegexForNextJs, (req, res) => {
    return handle(req, res)
  })

  server.get('*', (req, res) => {
    //Custom logic...
  })

...because I can't find any such regex for nextjs, or any kind of documented complete list of all URL patterns that nextjs should handle.

Current workaround

Our current workaround uses a list of URL patterns that we know that nextjs should handle. This list we had to construct manually, by surfing on the website, looking at the incoming requests and write down all the new patterns we find. But it doesn't feel like a future proof way. I mean, what if our random surfing on the website failed to trigger some special case request? Or what if we add a new plugin or something, that starts to making requests for URL's that doesn't match any of our previous patterns?

A possible solution

This problem could be solved quite easily, if nextjs supported a way to add a custom middleware after nextjs RequestHandler. Ie a middleware that get's triggered if the handle method can't handle the request. Basically that would leave all the 404-handling to us, but that we can handle.

Example code:

const standardMiddleware = function (req, res, next) {
  return handleWithout404(req, res, next)
}

server.get('*', [standardNextMiddleware, customMiddleware])

In the example code above, I use the fictional method handleWithout404 that is a twin method of the normal handle method. The only difference compared to the normal handle method is that this method calls next() if it can't find any page/resource/whatever when it otherwise would trigger a 404. The method name is just an example. And maybe instead of a new method it could be implemented by an extra parameter to the normal handle method.

Alternative solutions

One other way to solve this could be if nextjs provided us with some method that tests the request. If nextjs detects that it should handle the request, then it returns true, otherwise false. But in my opinion this would be an inferior solution, since it would most likely mean that nextjs would have to perform that check twice for normal nextjs requests (unless it added it's result to a cache, but that could cause other problems). Also, the middleware solution above seems more of a standard way of handling this.

@timneutkens
Copy link
Member

  const TOP_PATHS = new Set(
    readdirSync(join(__dirname, '..', 'pages'))
      .map(f => f.replace(/\.js$/, ''))
      .concat(['', 'static', '_next'])
  )
  server.use(function topRoutesMiddleware(req, res, goNext) {
    const topPath = req.path.split('/', 2)[1]

    if (TOP_PATHS.has(topPath)) {
      const { path: pathname, query } = req
      handle(req, res, { pathname, query })
      return
    }

    goNext()
  })

This is how we handle this on zeit.co in the custom server.

@batanun
Copy link
Author

batanun commented Mar 6, 2019

ok. Well, even though I can't recreate the situation now, in the beginning of our project we noticed a few special cases in a localhost dev environment not included in your solution:

  • Requests starting with /babel
  • Requests starting with /eval%
  • Requests starting with /code
  • Requests for js-files were the path didn't start with /_next, /static or /babel

So it seems that your solution isn't really foolproof. Which is why I think it would make much more sence if Nextjs would provide a way to use next as a true middleware, split up into one middleware for normal handling, and one middleware for 404. Then we could add our custom middleware in between those two.
Or... If it would be difficult to split the nextjs logic into two parts, then it instead could provide a way to inject our custom middleware that would be called before the 404 is rendered.

@osym
Copy link

osym commented May 15, 2019

Hey,
Maybe I missed it but is there a more reliable solution for this problem apart from the one mentioned here #6290 (comment)?

@sesteva
Copy link

sesteva commented Jun 10, 2019

We have a different scenario where getting a handleWithout404 sort of handle would actually solve it.
We have a Prometheus client where we report performance metrics.
We need to run some logic after every route is handled.
Unfortunately, "handle" does not call next and therefore our code is never invoked.

Customer Server Example

  // Runs before each requests
  server.use((req, res, next) => {
    res.locals.startEpoch = Date.now();
    next();
  });

  server.get("/", (req, res) => {
    return handle(req, res, "/", req.query);
  });

  server.get("*", (req, res) => {
    return handle(req, res);
  });

  // Runs after each requests - **THIS NEVER GETS INVOKED**
  server.use((req, res, next) => {
    const responseTimeInMs = Date.now() - res.locals.startEpoch;
    httpRequestDurationMicroseconds
      .labels(req.method, req.route.path, res.statusCode)
      .observe(responseTimeInMs);

    next();
  });

@timneutkens What is your suggestion? Shall I open a new ticket?

@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants