hoangvvo / next-connect

The TypeScript-ready, minimal router and middleware layer for Next.js, Micro, Vercel, or Node.js http/http2
https://www.npmjs.com/package/next-connect
MIT License
1.63k stars 65 forks source link

How to use with custom handlers #133

Closed jzzfs closed 3 years ago

jzzfs commented 3 years ago

Thanks for the cool pkg, @hoangvvo!

I wanna use withSentry. It's typically used as a handler's wrapper:

import { withSentry } from '@sentry/nextjs';

const handler = async (req, res) => {
  ...
}

export default withSentry(handler);

But I need it in my nc chain:

...
const withCorsAndFirebase = nc();
withCorsAndFirebase
  .use(withCors)
  .use(withFirebase)
  .use(async (req, res, next) => {
    withSentry(???);           <---
    next();
  });
...

What should I pass to the function withSentry? Thanks!

hoangvvo commented 3 years ago

Hi, I believe it would be withSentry(withCorsAndFirebase) since nc() returns a function (req, res) => void

jzzfs commented 3 years ago

Thanks but once I wrap nc in withSentry, the result is not chainable anymore because withSentry returns a void.

In other words, withSentry is often used right before exporting like so:

async function handler(req, res) {
  res.status(200).json({ name: 'John Doe' })
}

export default withSentry(handler)

That said, when I want to use .METHOD(pattern, ...fns), I'm stuck with imperatively wrapping each http method:

// file: pages/api/user
import nc from "next-connect";
import { withSentry } from "@sentry/nextjs";

import { withCorsAndFirebase } from "../../../utils/api/utils/middleware";

const handler = nc().use(withCorsAndFirebase);

const forGet = async (req: NextApiRequest, res: NextApiResponse) => { ... };
const forPut = async (req: NextApiRequest, res: NextApiResponse) => { ... };

handler.get(withSentry(forGet))
       .put(withSentry(forPut))

// export default withSentry(handler)  <-- this is not enough -- got to use `withSentry` for each http method

export default handler

I wonder whether this can be abstracted away on the nc declaration level...

I mean, withSentry itself is a wrapper around (req, res):

export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => {
  return async (req, res) => {
    try {
      return await handler(req, res);      // <----
    } catch (e) {
      withScope(scope => {
        scope.addEventProcessor(event => {
          addExceptionMechanism(event, {
            handled: false,
          });
          return parseRequest(event, req);
        });
        captureException(e);
      });
      throw e;
    } finally {
      await flush(2000);
    }
  };
};
hoangvvo commented 3 years ago

@jzzfs Sorry for the late response. You can always do all the chaining before you wrap withSentry around it.

const handler = nc().get().post().put()

export default withSentry(handler)
jzzfs commented 3 years ago

Good idea, will do. Thank you!

IRediTOTO commented 2 years ago

I have a base Next-connect like this

export const base= () => {
    return nc<NextApiRequest, NextApiResponse>({
        onError: (err, req, res, next) => {
            return res.status(500).json({ success: false })
        },
        onNoMatch: (req, res) => {
            return res.status(404).json({ success: false })
        },
    })
}

And I am using like this

const handler = base()

handler.get(.... )

export default withSentry(handler)

But not work. I tried the default API of Nextjs, it ran fine.

I can use like this

export const base= () => { return nc<NextApiRequest, NextApiResponse>({ onError: (err, req, res, next) => { Sentry.captureException(err) // But this look like not a right way, right? Did you use Sentry before? return res.status(500).json({ success: false }) }, onNoMatch: (req, res) => { return res.status(404).json({ success: false }) }, }) }

hoangvvo commented 2 years ago

I understood the issue. withSentry is triggered if handler() throws an error. However, in reality, it never does so withSentry() will not work. The usage of Sentry.captureException in onError will work! @IRediTOTO

kachkaev commented 1 year ago

Looks like it is necessary to call Sentry.flush after Sentry.captureException, according to with-sentry Next.js example.

Here is what I ended up with:

    onError: async (error, request, response) => {
      // Source: https://github.com/vercel/next.js/blob/cd3e054f14ce38f4ff57c727a997da2a6e1d05dd/examples/with-sentry/pages/api/test4.js
      Sentry.captureException(error);
      await Sentry.flush(2000);

      // Default next-connect behavior
      // https://github.com/hoangvvo/next-connect/blob/e5ac7faf380a73114a2b6b07b0a43a01f8c1be36/src/node.ts#L125-L127
      response.statusCode = 500;
      // eslint-disable-next-line no-console
      console.error(error);
      response.end("Internal Server Error");
    },