Properly handle io.copy errors

Changelog: added

6 files changed, 64 insertions, 8 deletions

diff --git a/internal/feature/feature.go b/internal/feature/feature.go
index c98fe85d..a012fc49 100644
--- a/internal/feature/feature.go
+++ b/internal/feature/feature.go
@@ -37,6 +37,11 @@ var RedirectsPlaceholders = Feature{
 	EnvVariable: "FF_ENABLE_PLACEHOLDERS",
 }
 
+// HandleReadErrors reports vfs.ReadErrors to sentry and enable error handling
+var HandleReadErrors = Feature{
+	EnvVariable: "FF_HANDLE_READ_ERRORS",
+}
+
 // Enabled reads the environment variable responsible for the feature flag
 // if FF is disabled by default, the environment variable needs to be "true" to explicitly enable it
 // if FF is enabled by default, variable needs to be "false" to explicitly disable it
diff --git a/internal/httprange/http_reader.go b/internal/httprange/http_reader.go
index 8e85a6d3..fbad326d 100644
--- a/internal/httprange/http_reader.go
+++ b/internal/httprange/http_reader.go
@@ -190,7 +190,7 @@ func (r *Reader) Read(buf []byte) (int, error) {
 	}
 
 	if err := r.ensureResponse(); err != nil {
-		return 0, err
+		return 0, vfs.NewReadError(err)
 	}
 
 	n, err := r.res.Body.Read(buf)
diff --git a/internal/httprange/http_reader_test.go b/internal/httprange/http_reader_test.go
index 01edb2e5..55258b4e 100644
--- a/internal/httprange/http_reader_test.go
+++ b/internal/httprange/http_reader_test.go
@@ -7,6 +7,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/vfs"
 )
 
 func TestSeekAndRead(t *testing.T) {
@@ -73,7 +75,7 @@ func TestSeekAndRead(t *testing.T) {
 			readSize:        testDataLen,
 			seekOffset:      int64(testDataLen),
 			seekWhence:      io.SeekStart,
-			expectedReadErr: ErrInvalidRange,
+			expectedReadErr: vfs.NewReadError(ErrInvalidRange),
 		},
 		// io.SeekCurrent ...
 		"read_all_from_seek_current": {
@@ -170,7 +172,7 @@ func TestSeekAndRead(t *testing.T) {
 		"invalid_range_reading_from_end": {
 			readSize:        testDataLen / 3,
 			seekWhence:      io.SeekEnd,
-			expectedReadErr: ErrInvalidRange,
+			expectedReadErr: vfs.NewReadError(ErrInvalidRange),
 		},
 	}
 	for name, tt := range tests {
diff --git a/internal/vfs/errors.go b/internal/vfs/errors.go
new file mode 100644
index 00000000..6fa2f29b
--- /dev/null
+++ b/internal/vfs/errors.go
@@ -0,0 +1,33 @@
+package vfs
+
+import "fmt"
+
+const message = "An error occurred while trying to read vfs.File content"
+
+type ReadError struct {
+	wrapped error
+}
+
+func NewReadError(wrapped error) *ReadError {
+	return &ReadError{
+		wrapped: wrapped,
+	}
+}
+
+func (r *ReadError) Error() string {
+	if r.wrapped == nil {
+		return message
+	}
+
+	return fmt.Sprintf("%s: %s", message, r.wrapped.Error())
+}
+
+func (r *ReadError) Unwrap() error {
+	return r.wrapped
+}
+
+func (r *ReadError) Is(target error) bool {
+	// nolint: errorlint // implementing type equality for errors.Is
+	_, ok := target.(*ReadError)
+	return ok
+}
diff --git a/internal/vfs/serving/serving.go b/internal/vfs/serving/serving.go
index 4d8b7d90..c5a7d011 100644
--- a/internal/vfs/serving/serving.go
+++ b/internal/vfs/serving/serving.go
@@ -5,6 +5,7 @@
 package serving
 
 import (
+	"context"
 	"errors"
 	"io"
 	"net/http"
@@ -13,6 +14,7 @@ import (
 	"time"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/errortracking"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/feature"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/logging"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/vfs"
@@ -47,10 +49,23 @@ func serveContent(w http.ResponseWriter, r *http.Request, modtime time.Time, con
 		return
 	}
 
-	w.WriteHeader(code)
+	if r.Method == http.MethodHead {
+		w.WriteHeader(code)
+		return
+	}
 
-	if r.Method != http.MethodHead {
-		io.Copy(w, content)
+	if _, err := io.Copy(w, content); err != nil {
+		if errors.Is(err, &vfs.ReadError{}) {
+			if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
+				return
+			}
+			logging.LogRequest(r).WithField("handle_read_errors", feature.HandleReadErrors.Enabled()).WithError(err).Error("error reading content")
+			if feature.HandleReadErrors.Enabled() {
+				errortracking.CaptureErrWithReqAndStackTrace(err, r)
+				logging.LogRequest(r).WithError(err).Error("error reading content")
+				httperrors.Serve500(w)
+			}
+		}
 	}
 }
 
diff --git a/internal/vfs/zip/archive_test.go b/internal/vfs/zip/archive_test.go
index 782c43b1..731f1608 100644
--- a/internal/vfs/zip/archive_test.go
+++ b/internal/vfs/zip/archive_test.go
@@ -23,6 +23,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/config"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httprange"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/vfs"
 )
 
 var (
@@ -143,7 +144,7 @@ func TestOpenCached(t *testing.T) {
 			filePath:         "index.html",
 			expectedRequests: 1,
 			// we receive an error on `read` as `open` offset is already cached
-			expectedReadErr:       httprange.ErrRangeRequestsNotSupported,
+			expectedReadErr:       vfs.NewReadError(httprange.ErrRangeRequestsNotSupported),
 			expectedArchiveStatus: archiveCorrupted,
 		},
 		{
@@ -161,7 +162,7 @@ func TestOpenCached(t *testing.T) {
 			filePath:         "subdir/hello.html",
 			expectedRequests: 1,
 			// we receive an error on `read` as `open` offset is already cached
-			expectedOpenErr:       httprange.ErrRangeRequestsNotSupported,
+			expectedOpenErr:       vfs.NewReadError(httprange.ErrRangeRequestsNotSupported),
 			expectedArchiveStatus: archiveCorrupted,
 		},
 	}