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.62k stars 65 forks source link

How to set errorHandler globally in v1? #201

Open larsqa opened 1 year ago

larsqa commented 1 year ago

Before v1, i could create a base nc like

export const nc = () =>
    nextConnect({
          onError: (err, req, res) => {
            console.error(err.stack);
            res.status(500).end("Something broke!");
          },
          onNoMatch: (req, res) => {
            res.status(404).end("Page is not found");
          },
    });

And then simply use nc().get(...).

However, now in v1, router expects the errorHandling to be passed into the router.handler() method. Meaning, to achieve the same thing I would have to create an errorHandler object and pass it into the handler. This leads to two imports. Is it possible to set this on a base level?

// Not what i want, but current behavior
const errorHandling = {
          onError: (err, req, res) => {
            console.error(err.stack);
            res.status(500).end("Something broke!");
          },
          onNoMatch: (req, res) => {
            res.status(404).end("Page is not found");
          },
}

...
// another file
import {router, errorHandling} from "./my-next-connect.js"
router.get(...)
export default router.handler(errorHandling)

Edit

Following your documentations, it should be possible to add this options object inside the createRouter() (https://github.com/hoangvvo/next-connect#router--createrouteroptions).

But when looking through the source code, this does not seem to be implemented: https://github.com/hoangvvo/next-connect/blob/main/src/node.ts

hoangvvo commented 1 year ago

That is an error on the doc, which I have removed. Unfortunately this is intended, providing options as part of nextConnect() is a bit misleading since calling .run() does not really respect these option, so I make it explicit that only .handler() will repsect it, thus the fact that it is only there.

Perhaps you can try something like below:

export const nc = () => {
  const router = createRouter();
  const handler = router.handler.bind(router);
  // rewrite router.handler so that when you call it, it is called with the options
  router.handler = () => handler({ ...yourOptions });
  return router;
};
larsqa commented 1 year ago

Isn't

const router = createRouter();
const handler = router.handler.bind(handler);

wrong? Isn't it supposed to be

const router = createRouter();
const handler = router.handler.bind(router);
hussamkhatib commented 1 year ago

How about doing what's specified in

DO NOT reuse the same instance of router like the below pattern: https://github.com/hoangvvo/next-connect#common-errors Where you would clone it.

@hoangvvo I like the way it is done in previous version, Do I need to be worried about anything in previous version

hoangvvo commented 1 year ago

Isn't

const router = createRouter();
const handler = router.handler.bind(handler);

wrong? Isn't it supposed to be

const router = createRouter();
const handler = router.handler.bind(router);

Yup you are correct, I made a typo there.

hoangvvo commented 1 year ago

I can give this more thought, perhaps we can specify it as part of options (preferably nested inside a handler key for clarity) in createRouter function

larsqa commented 1 year ago

Agree with @hussamkhatib, the new approach introduces more friction. I'd like to use it as it was before.

IRediTOTO commented 1 year ago

I like previous features too. I don't dare update Nc newest version :\

raphaelbadia commented 1 year ago

That is an error on the doc, which I have removed. Unfortunately this is intended, providing options as part of nextConnect() is a bit misleading since calling .run() does not really respect these option, so I make it explicit that only .handler() will repsect it, thus the fact that it is only there.

Perhaps you can try something like below:

export const nc = () => {
  const router = createRouter();
  const handler = router.handler.bind(router);
  // rewrite router.handler so that when you call it, it is called with the options
  router.handler = () => handler({ ...yourOptions });
  return router;
};

Tried this, and it works perfectly. Thanks !

nemanjahifolks commented 1 year ago

This is not an elegant solution because now in every api route file I need to export this:

export default handler.handler();

instead of nicer:

export default handler;

or I get this error:

error - Error [TypeError]: resolver is not a function
    at /home/username/Desktop/folks/node_modules/next/dist/server/api-utils/node.js:377:16
    at /home/username/Desktop/folks/node_modules/next/dist/server/lib/trace/tracer.js:92:36
    at NoopContextManager.with (/home/username/Desktop/folks/node_modules/next/dist/compiled/@opentelemetry/api/index.js:1:7057)
    at ContextAPI.with (/home/username/Desktop/folks/node_modules/next/dist/compiled/@opentelemetry/api/index.js:1:516)

How to fix this?