Fix the Nginx issue when refresh the page #857
Conversation
|
@rk3rn3r can you have a look |
nginx.config/default.conf
Outdated
| location / { | ||
| proxy_buffering off; | ||
| root /usr/share/nginx/html; | ||
| try_files $uri $uri/ $uri/index.html /index.html =404; |
There was a problem hiding this comment.
@indraraj how should 404s be handled?
There was a problem hiding this comment.
Will remove it since you don't have 404 html file in the dist dir.
There was a problem hiding this comment.
its a single page application we don't need a separate 404 html, 404 scenarios are already handled in UI via routes
There was a problem hiding this comment.
@yahyaAlsaidi I know there is the original 50x.html file. But I am not sure about 404. I need to check.
There was a problem hiding this comment.
No, I delete it this part when merging the two commits together, you don't have to check it there is not 404 file there. Sometimes some bundler include that and some not so you don't have it, so there is no reason to add it.
There was a problem hiding this comment.
Ah yes, I double-checked. There's indeed no 404.html file and the UI frontend is supposed to handle 404s.
One thing that was coming to my mind and I am interested to hear your opinion!
What do you think about using a different directory inside the container to host the UI?
Like /srv/ or a new directory like dbz-ui instead of /usr/share/nginx/html? And then it makes really sense to create a new config file in conf.d/ dir for that endpoint with your config? That way we done interfere with the default config but we need to try if this will be working. wdyt?
There was a problem hiding this comment.
Well, simply including your custom configuration in the default file might not resolve the issue due to potential conflicts. Some opt for a different approach by organizing configurations into sites-available and sites-enabled directories. Configurations are placed in sites-available and then symlinked in sites-enabled, which can be included in nginx.conf. However, this method can add complexity, especially if accidental symlink or backup files are created. You can find more discussion on this topic here. While separation of concerns is important, sometimes simplicity is preferable. Also here kind of example to do something similar to that.
There was a problem hiding this comment.
You should read the second answer in the first link (Stackoverflow). I think that one is very accurate Best Practice Is conf.d. and You should be using /etc/nginx/conf.d, as that's a standard convention, and should work anywhere. It think this reply is really on point. In a dockerized environment it does not make sense to use sites-available/sites-enable. I think this pattern only makes sense when you have one big server hosting multiple different websites/customers etc.
So that leaves the question if we should overwrite the upstream conf.d/default.conf or add an additional conf.d/debezium-ui.conf? I lean towards the second options if default.conf and debezium-ui.conf dont't interfere and can work together without harm. Maybe we only need to add the try_files line inside the correct structure in debezium-ui.conf? Maybe the rest is inherited and merged from default.conf?
There was a problem hiding this comment.
Yes I read that part but my concern is overwrite the same location in nginx will cause some conflicts and confusion. But, you could give it a try.
|
First, @yahyaAlsaidi thx a lot for raising this issue. I left one bigger comment though and I am looking forward to your feedback. Also, like mentioned at the end of my bigger comment, please file a Jira for this change in the Debezium issue tracker / Jira and use this to prefix your commit. And please squash that merge into your actual change-commit. Thx! |
|
Sorry @yahyaAlsaidi for the intensive discussion, but you raised a very good issue and that lead to some things that we should clarify when moving forward. |
| @@ -1,3 +1,3 @@ | |||
| { | |||
| "KAFKA_CONNECT_CLUSTERS": "http://localhost:8083" | |||
| "KAFKA_CONNECT_CLUSTERS": "http://localhost:8083/" | |||
There was a problem hiding this comment.
@indraraj Is this config change needed?
Issue:
When users refresh the page in Nginx, they encounter a "route not found" error due to the default file being served. This occurs because Nginx is configured to serve the default file instead of routing requests to the correct endpoints.
Solution:
To address this issue, I removed the default file from the Nginx configuration and added a custom file to handle route navigation. This ensures that requests are properly routed to the correct endpoints, preventing the "route not found" error on page refresh.