Allow turning off console.warn when 404 on Bun serveStatic adapter

[Th1nkK1D]

What is the feature you are proposing?

First of all, I'm willing to open the PR for this since I already modified it on my project but I need some thought.

The current Bun serveStatic adapter will console.warn when the requested file is not found.

hono/src/adapter/bun/serve-static.ts

Line 52 in 0f33cf8

console.warn(`Static file: ${path} is not found`)

However, this is not a behavior I want in the production. I want to propose an option to turn off the warning which might be done in 3 ways:

1. Add a boolean, eg. showNotFoundWarning to the ServeStaticOptions to toggle it on /off.
2. Add onNotFound to the ServeStaticOptions as a handler when the 404 error is occurred. The default value can be the console.warn to keep the original behavior.

onNotFound: (path: string) => void | Promise<void> =
    (path) => console.warn(`Static file: ${path} is not found`) 

3. Just remove it, since Hono already provides app.notFound which anyone can use it handle 404 in the way that they prefer, including console.warn.

Please let me know what you think :)

[yusukebe]

Hi @Th1nkK1D,

Indeed, you might be right. The purpose of these warnings is to aid in debugging, letting users know which path serveStatic is searching.

Static file: ${path} is not found

I think option 2 is the best choice because it's flexible and maintains the ability for debugging. If we go with this, the other serveStatic implementations, such as for Cloudflare and Deno, should also be updated accordingly.

[Th1nkK1D]

Hi! Thank you for the fast response.

I think option 2 is the best choice because it's flexible and maintains the ability for debugging.

Then do you think the default value of onNotFound should be the console.warn, or just the undefined value that does nothing? I just feel like putting onNotFound: () => {} to disable the warning is a bit unintuitive. In contrast, defaulting to undefined is like, if we are not specific about what to do when an error occurs, then do nothing. I prefer this way but it breaks the old behavior.

If we go with this, the other serveStatic implementations, such as for Cloudflare and Deno, should also be updated accordingly.

I can update those too.

[yusukebe]

If we implement onNotFound, I also prefer that it does nothing by default. As you mentioned, this changes the old behavior, but it doesn't break the procedure; it only stops showing the warning. If users want to display the warning, they can simply add a callback. Therefore, I think we can introduce this change as a minor version, like 3.12.0.

PR is welcome!

[Th1nkK1D]

@yusukebe the PR #1825 is ready for a review.

[sor4chi]

I thought it would be useful if the callback function of onNotFound could take a context as an argument for requirements such as sending logs externally.

like this:

onNotFound: (c, path) => {
  c.env.DB.exec ...
}

[yusukebe]

@sor4chi

That's a good idea. I think the following API is better:

onNotFound: (path, c) => {
  c.env.DB.exec ...
}

@Th1nkK1D What do you think about passing the context to the method?

[yusukebe]

Anyway, I've approved it once and I will merge #1825 into the "next" branch. If you want to introduce it, please create another PR.

[Th1nkK1D]

Sorry, I just came back from the holiday. @sor4chi That's a good idea and thank you for opening a PR for that ;)

[yusukebe]

This is fixed!!!!