diff options
| -rw-r--r-- | internal/serving/disk/reader.go | 48 | ||||
| -rw-r--r-- | internal/vfs/zip/vfs.go | 18 |
2 files changed, 35 insertions, 31 deletions
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index 258df3bc..2438daf7 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -16,6 +16,7 @@ import ( "gitlab.com/gitlab-org/labkit/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" + "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/redirects" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" "gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk/symlink" @@ -41,11 +42,8 @@ func (reader *Reader) serveRedirectsStatus(h serving.Handler, redirects *redirec func (reader *Reader) tryRedirects(h serving.Handler) bool { ctx := h.Request.Context() root, err := reader.vfs.Root(ctx, h.LookupPath.Path, h.LookupPath.SHA256) - if errors.Is(err, fs.ErrNotExist) { - return false - } else if err != nil { - httperrors.Serve500WithRequest(h.Writer, h.Request, "vfs.Root", err) - return true + if err != nil { + return handleRootError(err, h) } r := redirects.ParseRedirects(ctx, root) @@ -74,12 +72,8 @@ func (reader *Reader) tryFile(h serving.Handler) bool { ctx := h.Request.Context() root, err := reader.vfs.Root(ctx, h.LookupPath.Path, h.LookupPath.SHA256) - if errors.Is(err, fs.ErrNotExist) { - return false - } else if err != nil { - httperrors.Serve500WithRequest(h.Writer, h.Request, - "vfs.Root", err) - return true + if err != nil { + return handleRootError(err, h) } fullPath, err := reader.resolvePath(ctx, root, h.SubPath) @@ -137,11 +131,8 @@ func (reader *Reader) tryNotFound(h serving.Handler) bool { ctx := h.Request.Context() root, err := reader.vfs.Root(ctx, h.LookupPath.Path, h.LookupPath.SHA256) - if errors.Is(err, fs.ErrNotExist) { - return false - } else if err != nil { - httperrors.Serve500WithRequest(h.Writer, h.Request, "vfs.Root", err) - return true + if err != nil { + return handleRootError(err, h) } page404, err := reader.resolvePath(ctx, root, "404.html") @@ -153,6 +144,12 @@ func (reader *Reader) tryNotFound(h serving.Handler) bool { err = reader.serveCustomFile(ctx, h.Writer, h.Request, http.StatusNotFound, root, page404) if err != nil { + // Handle context.Canceled error as not exist https://gitlab.com/gitlab-org/gitlab-pages/-/issues/669 + if errors.Is(err, context.Canceled) { + logging.LogRequest(h.Request).WithError(err).Warn("user cancelled request") + return false + } + httperrors.Serve500WithRequest(h.Writer, h.Request, "serveCustomFile", err) return true } @@ -251,11 +248,6 @@ func (reader *Reader) serveCustomFile(ctx context.Context, w http.ResponseWriter // Open and serve content of file file, err := root.Open(ctx, fullPath) if err != nil { - // Handle context.Canceled error as not exist https://gitlab.com/gitlab-org/gitlab-pages/-/issues/669 - if errors.Is(err, context.Canceled) { - return fs.ErrNotExist - } - return err } defer file.Close() @@ -283,3 +275,17 @@ func (reader *Reader) serveCustomFile(ctx context.Context, w http.ResponseWriter return nil } + +func handleRootError(err error, h serving.Handler) bool { + switch err { + case fs.ErrNotExist: + return false + case context.Canceled: + // Handle context.Canceled error as not found exist https://gitlab.com/gitlab-org/gitlab-pages/-/issues/669 + //httperrors.Serve404(h.Writer) + return true + default: + httperrors.Serve500WithRequest(h.Writer, h.Request, "vfs.Root", err) + return true + } +} diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index cc0ff290..4ad1075f 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -179,16 +179,14 @@ func (zfs *zipVFS) Root(ctx context.Context, path string, cacheKey string) (vfs. // we do it in loop to not use any additional locks for { root, err := zfs.findOrOpenArchive(ctx, cacheKey, path) - switch err { - case context.Canceled: - // treat user-initiated cancellations as not found and - // do not report a 500 https://gitlab.com/gitlab-org/gitlab-pages/-/issues/669 - fallthrough - case httprange.ErrNotFound: - // If archive is not found, return a known `vfs` error - return nil, fs.ErrNotExist - case errAlreadyCached: - continue + if err != nil { + switch err { + case httprange.ErrNotFound: + // If archive is not found, return a known `vfs` error + return nil, fs.ErrNotExist + case errAlreadyCached: + continue + } } return root, err |