Grouped routes error handlers

[sher]

Given a simple app with grouped routes:

const main = new Hono();

const api = new Hono();

api.notFound((c) => {
  return c.text('Custom 404 Message from API', 404);
});

main.route('/api/*, api);

main.get('*', serveStatic({ path: './index.html', manifest }))

What I am trying to achieve:

• serve /api/* requests from api module
• if requested URL matches /api/* pattern, but endpoint handler is missing, respond with api.notFound handler
• otherwise, all other GET requests serve a static index.html

But what really is happening is that /api/missing-endpoint URL is also serving main.get('*', ...) handler, which is a static index.html

[yusukebe]

Hi @sher

It can't pass the custom not found handler to main with the app.route() function. But you can specify a fallback handler like the following. Try to use it for implementing custom not found:

const main = new Hono()
const api = new Hono()

api.get('/', (c) => c.json('/'))
api.get('/abc', (c) => c.json('abc'))
api.get('*', (c) => c.text('Custom 404 Message from API', 404))

main.route('/api', api)

[sher]

Thanks @yusukebe

I didn't explain the full context. Hono app is deployed and serving rest api on prefix /api and
all other paths should be serving static assets. And the last * wildcard should serve only index.html file.
My problem was to return a 404 error from /api if the endpoint/path was not found, so I went another way.

app.route('/api', api);
app.get('/assets/*', serveStatic({ root: './', manifest }));
app.get('/favicon.ico', serveStatic({ path: './favicon.ico', manifest }));
app.get('*', async function (c, next) {
  if (c.req.path.startsWith('/api/')) {
    return c.json(undefined, 404);
  }
  return serveStatic({ path: './index.html', manifest })(c, next);
});

[yusukebe]

@sher thanks for the explanation.

How about this?

app.route('/api', api)
app.all('/api/*', (c) => c.json({ message: 'Not Found' }, 404))
app.get('*', serveStatic({ path: './index.html', manifest }))

[jordanbtucker]

IMHO, this should just work:

import { serve } from "@hono/node-server";
import { Hono } from "hono";

const app = new Hono();
app.notFound((c) => c.text("App Not Found", 404));

const sub = new Hono();
sub.notFound((c) => c.text("Sub Not Found", 404));

app.route("/sub", sub);

serve({ fetch: app.fetch });

However, I get App Not Found instead of Sub Not Found no matter the path.

[NicoPlyley]

Hi @jordanbtucker,

Are you making a suggestion or trying to solve this?

There is an answer mentioned above. Right now in Hono, it is not possible to use the notFound method on any sub-app. This is because the route method requires each route to have a method, path, and handler. So to do this, you can provide all three by doing a catch-all at the bottom of your routes.

// All previous routes

subApp.all('/*', (c) => {
  return c.text('Sub app not found', 404)
})

This will have the same action as the notFound method.

If are are suggesting this as a feature, I'm not sure it would be entirely possible without rewriting the base Hono class which I imagine would introduce breaking changes.

[jordanbtucker]

The problem with using a catch all is that you can't just perform a return c.notFound(), which is the ergonomic way to do it.

sub.get("/:id", (c) => {
  const item = await db.findByID(c.req.param("id"));
  if (item == null) {
    return c.notFound();
  }
  return c.json(item);
});

A catch all doesn't help here because I need to return or throw a 404 if the database item doesn't exist. Plus, what if I want my catch all to do something other than return a 404?

Sure, I could do return next() and let the catch all handle it, but that makes the code harder to reason about. Being able to specify that all notFound() calls on this route group should return the JSON { message: "Not Found" } with a status of 404 is more ergonomic than having to call return c.json({ message: "Not Found" }, 404) several times in your route group, especially when it's always going to return JSON.

I do understand that this would be a breaking change, so I suppose I'm suggesting that this be fixed in v5. I think that having notFound work on route groups would follow the principle of least surprise and the benefits are clear.

[NicoPlyley]

If you were to use another catch-all a notFound would not fire. Throwing an error will go to onError which may help you in this case. What I do when no database results are found is this:

if (!user) {
  throw new ErrorReponse('User not found', 404); // ErrorResponse is a custom class that extends Error
}

You can check out my library hono-error-handler and tweak it to your needs if needed. That is how I handle all errors and 404's that I throw.

With that being said, I don't think having a notFound for each sub-app is a bad idea. But in order to be implemented now I think it would essentially be a catch-all running in the background

Update:
I did not see you mention c.notFound originally which absolutely makes sense, I did not think of that.