deno_std
deno_std copied to clipboard
feature(http/file_server): add 404 page support
Adds fallback logic for a custom 404 page as discussed and fixes #3533.
The PR modifies the serveDirResponse logic. Instead of immediately throwing an error when a specified file in the URL is not found, it now performs the following checks before throwing an error:
- If
fallbackis set to false, an error is thrown. - It checks if a custom file is specified. If yes, it uses that file; otherwise, it defaults to
404.html. - It retrieves the file, serves it and if it is not found, an error is thrown.
Noting here that it also solves https://github.com/denoland/deno_std/issues/3420.
There seems to be a lot of formatting changes which makes this pretty hard to review (did you format with prettier?). Are you able to revert this?
From what I've been able to see, it looks great! Nice work.
Apologies for that! It's my first contribution to the codebase, so it's a one-time occurrence. (i hope)
You were right, the formatting problem was indeed caused by using Prettier. I've switched to deno fmt instead, and now deno fmt --check doesn't raise any complaints. However, I have noticed some occasional formatting changes where parentheses are removed or line breaks are altered. This might be due to changes in deno fmt itself?
Thank you for bringing this to my attention!
deno fmt allows more formatting variations than prettier. So it doesn't format back to the original state unfortunately.. Probably it's better to revert back to main, and copy paste the relevant parts..
@mooxl
I think hard coded 404.html is enough for a first pass. I'm skeptical if we need to support custom 404 file path as there's no such request from the users
@mooxl I think hard coded
404.htmlis enough for a first pass. I'm skeptical if we need to support custom 404 file path as there's no such request from the users
Hey @kt3k, Thank you for the prompt response! I believe we should add support for a custom 404 file to address issue #3420. I have made the necessary formatting changes in the updated PR.
I think the case #3420 can be solved by copying index.html to 404.html.
Also file_server is historically not intended to support all use cases, but it's more for showcasing how a simple file server can be written in Deno. So it's also important file_server.ts can be read without much difficulties by curious users. With that policy in mind, we should be very careful about adding new features to file_server.ts
Hi @kt3k, I get your point about simplicity in file_server.ts. Let's stick to the default 404.html. I'll adjust the PR accordingly. Thanks for your guidance.
@kt3k could you take another look?
@mooxl Are you able to push this forward? I'm willing to pick it up if not.
Hey @lino-levan, I'm currently preparing for my exams, so I don't have time to work on this. You can go ahead with it. :)
Hey guys, are we still happy to push this forward?
Personally a bit busy atm, hoping either you our Max can push it forward.
@kt3k, how would you like to proceed with this one?
There doesn't seem to be much demand for this feature and this PR has gone stale. I suggest we close without merging. WDYT, @kt3k?
I'm personally in favor of this.
I'm closing this without merging, as this PR has gone stale. We'd be happy to look at a new PR if anyone is willing to finish this. Thank you, either way, @mooxl!