fix: return 404 not 500 when stored file gone from disk

frontend/src/i18n/lang/en.json

@@ -1516,6 +1516,7 @@
     "4021": "This user is already assigned to that task.",
     "4022": "Please provide what the reminder date is relative to.",
     "4023": "Cannot create a task relation cycle.",
+    "4034": "The file does not exist.",
     "6001": "The team name cannot be empty.",
     "6002": "The team does not exist.",
     "6004": "The team already has access to that project.",

pkg/files/dump.go

@@ -17,9 +17,7 @@
 package files
 
 import (
-	"errors"
 	"io"
-	gofs "io/fs"
 )
 
 // Dump dumps all saved files
@@ -35,8 +33,7 @@ func Dump() (allFiles map[int64]io.ReadCloser, err error) {
 	for _, file := range files {
 		err = file.LoadFileByID()
 		if err != nil {
-			var pathError *gofs.PathError
-			if errors.As(err, &pathError) {
+			if IsErrFileDoesNotExist(err) {
 				continue
 			}
 			return

pkg/files/error.go

@@ -39,6 +39,18 @@ func IsErrFileDoesNotExist(err error) bool {
 	return ok
 }
 
+// ErrCodeFileDoesNotExist holds the unique world-error code of this error
+const ErrCodeFileDoesNotExist = 4034
+
+// HTTPError holds the http error description
+func (err ErrFileDoesNotExist) HTTPError() web.HTTPError {
+	return web.HTTPError{
+		HTTPCode: http.StatusNotFound,
+		Code:     ErrCodeFileDoesNotExist,
+		Message:  "The file does not exist.",
+	}
+}
+
 // ErrFileIsTooLarge defines an error where a file is larger than the configured limit
 type ErrFileIsTooLarge struct {
 	Size uint64

pkg/files/files.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"math"
 	"os"
 	"strconv"
@@ -68,7 +69,14 @@ func DeleteBlob(id int64) error {
 // LoadFileByID returns a file by its ID
 func (f *File) LoadFileByID() (err error) {
 	f.File, err = storage.Open(f.fileID())
-	return
+	if err != nil {
+		// A db row without its blob is a broken install, not a server error.
+		if errors.Is(err, fs.ErrNotExist) {
+			return ErrFileDoesNotExist{FileID: f.ID}
+		}
+		return fmt.Errorf("failed to open file %d: %w", f.ID, err)
+	}
+	return nil
 }
 
 // LoadFileMetaByID loads the file metadata using the caller's session — an engine

pkg/files/files_test.go

@@ -21,7 +21,7 @@ import (
 	"image"
 	"image/png"
 	"io"
-	"os"
+	"net/http"
 	"testing"
 
 	"code.vikunja.io/api/pkg/config"
@@ -180,7 +180,8 @@ func TestFile_LoadFileByID(t *testing.T) {
 		f := &File{ID: 9999}
 		err := f.LoadFileByID()
 		require.Error(t, err)
-		assert.True(t, os.IsNotExist(err))
+		assert.True(t, IsErrFileDoesNotExist(err))
+		assert.Equal(t, http.StatusNotFound, err.(ErrFileDoesNotExist).HTTPError().HTTPCode)
 	})
 }
 

pkg/files/s3_test.go

@@ -185,7 +185,7 @@ func TestFileStorageIntegration(t *testing.T) {
 		nonExistentFile := &File{ID: 999999}
 		err := nonExistentFile.LoadFileByID()
 		require.Error(t, err, "Loading non-existent file should error")
-		assert.True(t, os.IsNotExist(err), "Error should indicate file does not exist")
+		assert.True(t, IsErrFileDoesNotExist(err), "Error should be ErrFileDoesNotExist")
 
 		// Try to load metadata for non-existent file
 		s := db.NewSession()

pkg/models/export.go

@@ -19,10 +19,8 @@ package models
 import (
 	"archive/zip"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
-	"io/fs"
 	"os"
 	"time"
 
@@ -343,8 +341,7 @@ func exportTaskAttachments(s *xorm.Session, wr *zip.Writer, taskIDs []int64) (er
 	for _, ta := range tas {
 		err = ta.File.LoadFileByID()
 		if err != nil {
-			var pathError *fs.PathError
-			if errors.As(err, &pathError) {
+			if files.IsErrFileDoesNotExist(err) {
 				continue
 			}
 			return err
@@ -392,8 +389,7 @@ func exportProjectBackgrounds(s *xorm.Session, u *user.User, wr *zip.Writer) (er
 		}
 		err = bgFile.LoadFileByID()
 		if err != nil {
-			var pathError *fs.PathError
-			if errors.As(err, &pathError) {
+			if files.IsErrFileDoesNotExist(err) {
 				continue
 			}
 			return err
@@ -426,7 +422,7 @@ func GetUserDataExportFile(s *xorm.Session, u *user.User) (*files.File, error) {
 // session first; the caller must close the reader.
 func OpenUserDataExportFile(exportFile *files.File) error {
 	if err := exportFile.LoadFileByID(); err != nil {
-		if os.IsNotExist(err) {
+		if files.IsErrFileDoesNotExist(err) {
 			return ErrUserDataExportDoesNotExist{}
 		}
 		return err

pkg/modules/avatar/upload/upload_test.go

@@ -21,6 +21,7 @@ import (
 	"strconv"
 	"testing"
 
+	"code.vikunja.io/api/pkg/files"
 	"code.vikunja.io/api/pkg/log"
 	"code.vikunja.io/api/pkg/modules/keyvalue"
 	"code.vikunja.io/api/pkg/user"
@@ -107,4 +108,18 @@ func TestGetAvatar(t *testing.T) {
 		assert.Equal(t, []byte("fake_image_data"), avatar)
 		assert.Equal(t, "image/png", mimeType)
 	})
+
+	t.Run("avatar file missing from storage", func(t *testing.T) {
+		files.InitTestFileHandler()
+
+		provider := &Provider{}
+		testUser := &user.User{
+			ID:           999999,
+			AvatarFileID: 424242,
+		}
+
+		_, _, err := provider.GetAvatar(testUser, 32)
+		require.Error(t, err)
+		assert.True(t, files.IsErrFileDoesNotExist(err))
+	})
 }