Detect that route exists, but method not allowed

[1aerostorm]

Handler has:

• options.onError, which handles exceptions (i.e. 500, but not 404/405)
• options.onNoMatch, which handles both 404 (no route) and 405 (route presents, but wrong method), and doesn't provide any convenient way to detect it is 405 or 404.

Probably there is a possibility to develop a middleware which will handle all 404 routes, and throw an exception if it is 405 actually. But it will lay on next-connect internals, so it will be tricky and could break on new next-connect versions. Why not implement such feature in next-connect itself?

[hoangvvo]

One way is to have a .all at the end of the handler.

nc()
  .get(foo)
  .post(bar)
  .all((req, res) => res.status(405).send("Method not allowed"));

[1aerostorm]

@hoangvvo nope. Next-connect is a router. Routers are usually used with routes:

const handler = nc()
    .get('/api/test/foo', (req, res) => {
        res.end('foo!')
    })
    .post('/api/test/bar', (req, res) => {
        res.end('bar!')
    })
    .all((req, res) => {
        res.end('method not allowed');
    });

And .all will handle both 404 and 405, not 405. Only solution is to add .all to each route (by method like .get2 instead of .get, and .get2 will wrap .get+.all);

[hoangvvo]

Oh I see, I thought that you used it in a Next.js API Routes, which should work with my solution above, for example:

// /pages/api/foo.js
export default nc()
  .get(foo)
  .post(bar)
  .all((req, res) => res.status(405).send("Method not allowed"));

// POST /api/foo 200
// GET /api/foo 200
// PUT /api/foo 405  
// POST /api/bar 404

If the code you commented is your use case (which define handlers with bases, I don't think that there is anyway to determine if a request should be 405, unless you explicitly define it like below.

const handler = nc()
    .get('/api/test/foo', (req, res) => {
        res.end('foo!')
    })
    .post('/api/test/bar', (req, res) => {
        res.end('bar!')
    })
    .all('/api/test/bar', (req, res) => { // <- explicitly define the base so we handle non POST request with 405
        res.status(405).end('method not allowed');
    });

[1aerostorm]

@hoangvvo some route handlers are very short and simply, so we shouldn't split them into few files (Next.js API Routes).

I don't think that there is anyway to determine if a request should be 405, unless you explicitly define it like below.

So I propose to add such feature in next next-connect version :)

[hoangvvo]

@hoangvvo some route handlers are very short and simply, so we shouldn't split them into few files (Next.js API Routes).

I don't think that there is anyway to determine if a request should be 405, unless you explicitly define it like below.

So I propose to add such feature in next next-connect version :)

I gave this some thought but I think it is not ideal to implement this behavior for several reasons:

• It adds some additional calculation each time we add a route (since it requires traversing through all route handler every to determine if it should be a 405)
• It cannot reliable detect 405 if among the route handlers is one with a Regex match

I would suggest just manually add .all like I mentioned above.

[1aerostorm]

Currently solved it with:

let handler;

const onNoMatch = makeNoMatch(() => handler);

handler = nc({ onError, onNoMatch, ..., })
    .use(...)
    .get('/api/hello/route1', ...)
    .post('/api/hello/route2', ...);

Where makeNoMatch is util implemented as following:

const onNoMatch = async (req, res, nc) => {
    try {
        if (nc) for (let route of nc.routes) {
            if (route.method.length > 0 && req.url.match(route.pattern)) {
                // return 405 error
                break;
            }
        }
    } catch (err) { // nc.routes is undocumented - it can break
        // log error and return 404. It is better than return default "Internal Server Error", which is not JSON-formatted
    }
    // return 404 error
};

export const makeNoMatch = (handlerGetter) => {
    return async function(req, res) {
        await onNoMatch(req, res, handlerGetter && handlerGetter());
    };
};

So, if call has not returned any response, but at least 1 route with some method exists, it responds with 405.

If we have 2 or more handlers for same route with different methods, it also works.

Looks fully correct.

Yes, it uses "undocumented trick" (nc.routes is not documented), and yes, the iteration costs some CPU time

But it iterates only when 404/405 error happens. And for case it is being used for DDoS attack, we can just add some rate limit into onNoMatch.

So it is more optimal than another way: "automatically\manually adding .all() handlers to all routes", because .all() handlers will present always and cost some CPU time on each request.

I suggest you to add alike implementation into your library.