vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

Properly return 404 when the file does not exist #966

Merged
konrad merged 2 commits from LordGaav/frontend:feature/do-not-return-200-on-every-request into main 2021-11-10 21:38:08 +00:00
Conversation 1 Commits 2 Files Changed 1 +7 -2
1 changed files with 7 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
nginx.conf
View File

@ -69,10 +69,15 @@ http {
ssl_certificate /etc/nginx/ssl/dummy.crt;
ssl_certificate_key /etc/nginx/ssl/dummy.key;
location ~* .(txt|webmanifest|css|js|mjs|map|svg|jpg|jpeg|png|ico|ttf|woff|woff2|wav)$ {
root /usr/share/nginx/html;
try_files $uri $uri/ =404;
konrad commented 2021-11-09 23:02:35 +00:00
Review
Copy Link

Unfortunately this breaks accessing something like /login (or password reset routes, share links etc) directly without going through /.

Unfortunately this breaks accessing something like `/login` (or password reset routes, share links etc) directly without going through `/`.
LordGaav commented 2021-11-10 20:57:37 +00:00
Review
Copy Link

I see, you're right. Unfortunately, when serving with Nginx, the problem that I mentioned in the opening exists (serving 200 OK when actually it should be 404 Not Found).

Is there a way to generate static HTML files for the routes that we need?

I see, you're right. Unfortunately, when serving with Nginx, the problem that I mentioned in the opening exists (serving 200 OK when actually it should be 404 Not Found). Is there a way to generate static HTML files for the routes that we need?
konrad commented 2021-11-10 20:59:57 +00:00
Review
Copy Link

Is there a way to generate static HTML files for the routes that we need?

Maybe, kind of. You could theoretically parse route definitions at router/index.js to figure out a regex map of all routes that are actually required and then put that in the nginx config. But then we might as well think about SSR...

> Is there a way to generate static HTML files for the routes that we need? Maybe, kind of. You could theoretically parse route definitions at `router/index.js` to figure out a regex map of all routes that are actually required and then put that in the nginx config. But then we might as well think about [SSR](https://vitejs.dev/guide/ssr.html)...
LordGaav commented 2021-11-10 21:13:13 +00:00
Review
Copy Link

Maybe we're going about this wrong; we want to avoid returning 200 OK on .js files because that breaks the service worker, any other route doesn't really matter.

Maybe we're going about this wrong; we want to avoid returning 200 OK on .js files because that breaks the service worker, any other route doesn't really matter.
konrad commented 2021-11-10 21:19:01 +00:00
Review
Copy Link

mhhhhh nginx can do regex, I'm sure we could modify the location block to do the try_files only work on files that are not ending on .js. Then add another one to keep serving .js files.

mhhhhh nginx can do regex, I'm sure we could modify the `location` block to do the `try_files` only work on files that are not ending on `.js`. Then add another one to keep serving `.js` files.
LordGaav commented 2021-11-10 21:33:45 +00:00
Review
Copy Link

Indeed, see my latest commit. I applied this to my own installation, and everything still seems to work.

Indeed, see my latest commit. I applied this to my own installation, and everything still seems to work.
konrad commented 2021-11-10 21:34:46 +00:00
Review
Copy Link

Looks good.

Looks good.
dpschen commented 2021-11-10 21:52:15 +00:00
Review
Copy Link

Seems error-prone to list the files individually. E.g. mp3 is missing.

Seems error-prone to list the files individually. E.g. `mp3` is missing.
LordGaav commented 2021-11-10 22:26:13 +00:00
Review
Copy Link

I did not actually find any mp3 files in the deployed Docker container, this list is all the extensions that I did find.

I did not actually find any mp3 files in the deployed Docker container, this list is all the extensions that I did find.
konrad commented 2021-11-10 22:29:09 +00:00
Review
Copy Link

@dpschen do you know a way to generate that list?

@dpschen do you know a way to generate that list?
dpschen commented 2021-11-10 23:42:18 +00:00
Review
Copy Link

I know that it's possible to prerender which also generates the routes that are static: https://vite-plugin-ssr.com/pre-rendering
Not sure if that helps though.

Regarding the list: I found this after a built in the dist folder using this:

css
html
ico
jpg
js
map
mjs
mp3
png
svg
txt
webmanifest
woff2
I know that it's possible to prerender which also generates the routes that are static: https://vite-plugin-ssr.com/pre-rendering Not sure if that helps though. Regarding the list: I found this after a built in the dist folder using [this](https://stackoverflow.com/a/1842270/15522256): ``` css html ico jpg js map mjs mp3 png svg txt webmanifest woff2 ```
}
location / {
root /usr/share/nginx/html;
try_files $uri $uri/ /;
index index.html index.htm;
try_files $uri $uri/ /index.html;
index index.html;
}
error_page 500 502 503 504 /50x.html;