Codeberg/pages-server
26
387
Fork
You've already forked pages-server
54

FIX blank internal pages (#164) #292

Merged
crapStone merged 5 commits from hoernschen/pages-server:main into main 2024年02月26日 23:21:45 +01:00
Contributor
Copy link

Hello 👋

since it affected my deployment of the pages server I started to look into the problem of the blank pages and think I found a solution for it:

  1. There is no check if the file response is empty, neither in cache retrieval nor in writing of a cache. Also the provided method for checking for empty responses had a bug.
  2. I identified the redirect response to be the issue here. There is a cache write with the full cache key (e. g. rawContent/user/repo|branch|route/index.html) happening in the handling of the redirect response. But the written body here is empty. In the triggered request from the redirect response the server then finds a cache item to the key and serves the empty body. A quick fix is the check for empty file responses mentioned in 1.
  3. The decision to redirect the user comes quite far down in the upstream function. Before that happens a lot of stuff that may not be important since after the redirect response comes a new request anyway. Also, I suspect that this causes the caching problem because there is a request to the forge server and its error handling with some recursions happening before. I propose to move two of the redirects before "Preparing"
  4. The recursion in the upstream function makes it difficult to understand what is actually happening. I added some more logging to have an easier time with that.
  5. I changed the default behaviour to append a trailing slash to the path to true. In my tested scenarios it happened anyway. This way there is no recursion happening before the redirect.

I am not developing in go frequently and rarely contribute to open source -> so feedback of all kind is appreciated

Hello 👋 since it affected my deployment of the pages server I started to look into the problem of the blank pages and think I found a solution for it: 1. There is no check if the file response is empty, neither in cache retrieval nor in writing of a cache. Also the provided method for checking for empty responses had a bug. 2. I identified the redirect response to be the issue here. There is a cache write with the full cache key (e. g. rawContent/user/repo|branch|route/index.html) happening in the handling of the redirect response. But the written body here is empty. In the triggered request from the redirect response the server then finds a cache item to the key and serves the empty body. A quick fix is the check for empty file responses mentioned in 1. 3. The decision to redirect the user comes quite far down in the upstream function. Before that happens a lot of stuff that may not be important since after the redirect response comes a new request anyway. Also, I suspect that this causes the caching problem because there is a request to the forge server and its error handling with some recursions happening before. I propose to move two of the redirects before "Preparing" 4. The recursion in the upstream function makes it difficult to understand what is actually happening. I added some more logging to have an easier time with that. 5. I changed the default behaviour to append a trailing slash to the path to true. In my tested scenarios it happened anyway. This way there is no recursion happening before the redirect. I am not developing in go frequently and rarely contribute to open source -> so feedback of all kind is appreciated
FIX blank internal pages ( #164 )
Some checks failed
ci/woodpecker/pr/woodpecker Pipeline failed
f74f71cac1
FIX linting
Some checks failed
ci/woodpecker/pr/woodpecker Pipeline failed
c1a2b9cfa3
Fix linting
Some checks failed
ci/woodpecker/pr/woodpecker Pipeline failed
5a844330d9
CHANGE redirect and traling slash appending back
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
aeaf084ea1
Author
Contributor
Copy link

I reverted the changes 3 and 5. From the failed integration tests I realized that I didn't took raw resources into consideration.

I reverted the changes 3 and 5. From the failed integration tests I realized that I didn't took raw resources into consideration.
@ -40,3 +40,3 @@
func(fFileResponse)IsEmpty()bool{
returnlen(f.Body)!=0
returnlen(f.Body)==0
Contributor
Copy link

uh that looks like a bug. thanks for that catch!

uh that looks like a bug. thanks for that catch!
@ -26,6 +26,7 @@ func Handler(
dnsLookupCache,canonicalDomainCache,redirectsCachecache.ICache,
)http.HandlerFunc{
returnfunc(whttp.ResponseWriter,req*http.Request){
log.Info().Msg("\n----------------------------------------------------------")

Is this a remnant from debugging?

Is this a remnant from debugging?
Author
Contributor
Copy link

I used this to mark the start of each client request and found it very useful to understand what is happening. For example: After the index page there often comes a request for the favicon.ico. But in that request handling I may not be interested in. This way I can easier get to the interesting part of the log. Maybe it would be better as a debug log?

I used this to mark the start of each client request and found it very useful to understand what is happening. For example: After the index page there often comes a request for the favicon.ico. But in that request handling I may not be interested in. This way I can easier get to the interesting part of the log. Maybe it would be better as a debug log?

I think as debug or trace log it should be fine.

I think as debug or trace log it should be fine.
Author
Contributor
Copy link

I changed it to debug

I changed it to debug
crapStone marked this conversation as resolved
CHANGE logging
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
ci/woodpecker/pull_request_closed/woodpecker Pipeline was successful
44c76dfcaa

Thanks for your contribution!

Thanks for your contribution!
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Codeberg/pages-server!292
Reference in a new issue
Codeberg/pages-server
No description provided.
Delete branch "hoernschen/pages-server:main"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?