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

Custom middleware after nextjs RequestHandler #7665

Closed
sesteva opened this issue Jun 25, 2019 · 7 comments
Closed

Custom middleware after nextjs RequestHandler #7665

sesteva opened this issue Jun 25, 2019 · 7 comments

Comments

@sesteva
Copy link

sesteva commented Jun 25, 2019

Feature request

Related to #6290
We need to run some logic after every route is handled.

Is your feature request related to a problem? Please describe.

We have a Prometheus client where we report performance metrics.
"handleRequest" does not call next and therefore our code is never invoked.

Custom Server Example

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

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

  server.get("*", (req, res) => {
    return handleRequest(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();
  });

Describe the solution you'd like

handleRequest should call next
This would solve #7499 as well allowing people to add middlewares

Describe alternatives you've considered

I've not found a workaround

@Janpot
Copy link
Contributor

Janpot commented Jun 25, 2019

Maybe, instead of a middleware, you can use the res.on('finish') event for this in your main request handler?

@timneutkens
Copy link
Member

Next.js does not provide a middleware, it ends the request when you call handle. So like @Janpot is saying you'll probably want to add on finish or similar.

@sesteva
Copy link
Author

sesteva commented Jul 2, 2019

@timneutkens could you expand why adding a Middleware concept would be a bad idea for this framework ?

@sesteva
Copy link
Author

sesteva commented Aug 15, 2019

@timneutkens NextJS is great, I think it could be Amazing if I knew what is the correct way to open a dialogue about features such as middlewares or at least understanding why it does not fit the ZEIT's vision.

In the meantime, this is how to work around it as @Janpot kindly advised.

const handle = app.getRequestHandler();
const metricsInterval = Prometheus.collectDefaultMetrics();
const httpRequestDurationMicroseconds = new Prometheus.Histogram({
  name: "http_request_duration_ms",
  help: "Duration of HTTP requests in ms",
  labelNames: ["method", "route", "code"],
  buckets: [0.1, 5, 15, 50, 100, 200, 300, 400, 500] // buckets for response time from 0.1ms to 500ms
});

const onFinish = (req, res) => {
  res.on("finish", () => {
    const responseTimeInMs = Date.now() - res.locals.startEpoch;
    httpRequestDurationMicroseconds
      .labels(req.method, req.route.path, res.statusCode)
      .observe(responseTimeInMs);
  });

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

  server.get("/_next/*", (req, res) => {
    onFinish(req, res);
    /* serving _next static content using next.js handler */
    handle(req, res);
  });

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

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

@timneutkens
Copy link
Member

There already was an open RFC before this issue was posted: #7208

@code-press
Copy link

@timneutkens @sesteva would this help? https://github.com/oslabs-beta/connext-js

I'd still like to see middleware for my pages, but this for API Routes seems feasible

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
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

5 participants