From 34190011b2f774bc9a3613850a7be9d67c8ecd7e Mon Sep 17 00:00:00 2001
From: Jaime Martinez <jmartinez@gitlab.com>
Date: Tue, 7 Dec 2021 12:10:14 +1100
Subject: refactor: handle error outside Root caller

and pretend to serve content even if the request has been cancelled
to prevent making extra operations if this happens early in the
request chain. For example, if we know the request was cancelled
there is no point in checking the redirects or the custom
404 file handler.
---
 internal/serving/disk/reader.go | 48 +++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

(limited to 'internal/serving')

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
+	}
+}
-- 
cgit v1.2.3