From 97602c3d62ecc60e48627be9772aa29e0acf54b8 Mon Sep 17 00:00:00 2001 From: Marius Date: Sun, 25 Apr 2021 23:17:42 +0200 Subject: [PATCH] filestore: Handle os.ErrNotExist not in core handler --- pkg/filestore/filestore.go | 8 ++++++++ pkg/filestore/filestore_test.go | 15 +++++++++++++-- pkg/handler/head_test.go | 3 +-- pkg/handler/patch_test.go | 3 +-- pkg/handler/unrouted_handler.go | 6 ------ 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/pkg/filestore/filestore.go b/pkg/filestore/filestore.go index c2273b1..131b90c 100644 --- a/pkg/filestore/filestore.go +++ b/pkg/filestore/filestore.go @@ -89,6 +89,10 @@ func (store FileStore) GetUpload(ctx context.Context, id string) (handler.Upload info := handler.FileInfo{} data, err := ioutil.ReadFile(store.infoPath(id)) if err != nil { + if os.IsNotExist(err) { + // Interpret os.ErrNotExist as 404 Not Found + err = handler.ErrNotFound + } return nil, err } if err := json.Unmarshal(data, &info); err != nil { @@ -99,6 +103,10 @@ func (store FileStore) GetUpload(ctx context.Context, id string) (handler.Upload infoPath := store.infoPath(id) stat, err := os.Stat(binPath) if err != nil { + if os.IsNotExist(err) { + // Interpret os.ErrNotExist as 404 Not Found + err = handler.ErrNotFound + } return nil, err } diff --git a/pkg/filestore/filestore_test.go b/pkg/filestore/filestore_test.go index 6bbb4ee..3b871dd 100644 --- a/pkg/filestore/filestore_test.go +++ b/pkg/filestore/filestore_test.go @@ -4,7 +4,6 @@ import ( "context" "io" "io/ioutil" - "os" "path/filepath" "strings" "testing" @@ -74,7 +73,7 @@ func TestFilestore(t *testing.T) { // Test if upload is deleted upload, err = store.GetUpload(ctx, info.ID) a.Equal(nil, upload) - a.True(os.IsNotExist(err)) + a.Equal(handler.ErrNotFound, err) } func TestMissingPath(t *testing.T) { @@ -89,6 +88,18 @@ func TestMissingPath(t *testing.T) { a.Equal(nil, upload) } +func TestNotFound(t *testing.T) { + a := assert.New(t) + + store := FileStore{"./path"} + ctx := context.Background() + + upload, err := store.GetUpload(ctx, "upload-that-does-not-exist") + a.Error(err) + a.Equal(handler.ErrNotFound, err) + a.Equal(nil, upload) +} + func TestConcatUploads(t *testing.T) { a := assert.New(t) diff --git a/pkg/handler/head_test.go b/pkg/handler/head_test.go index ab5af62..59da085 100644 --- a/pkg/handler/head_test.go +++ b/pkg/handler/head_test.go @@ -3,7 +3,6 @@ package handler_test import ( "context" "net/http" - "os" "testing" "github.com/golang/mock/gomock" @@ -64,7 +63,7 @@ func TestHead(t *testing.T) { }) SubTest(t, "UploadNotFoundFail", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { - store.EXPECT().GetUpload(context.Background(), "no").Return(nil, os.ErrNotExist) + store.EXPECT().GetUpload(context.Background(), "no").Return(nil, ErrNotFound) handler, _ := NewHandler(Config{ StoreComposer: composer, diff --git a/pkg/handler/patch_test.go b/pkg/handler/patch_test.go index 19eba85..79a3fe9 100644 --- a/pkg/handler/patch_test.go +++ b/pkg/handler/patch_test.go @@ -5,7 +5,6 @@ import ( "io" "io/ioutil" "net/http" - "os" "strings" "testing" "time" @@ -141,7 +140,7 @@ func TestPatch(t *testing.T) { }) SubTest(t, "UploadNotFoundFail", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { - store.EXPECT().GetUpload(context.Background(), "no").Return(nil, os.ErrNotExist) + store.EXPECT().GetUpload(context.Background(), "no").Return(nil, ErrNotFound) handler, _ := NewHandler(Config{ StoreComposer: composer, diff --git a/pkg/handler/unrouted_handler.go b/pkg/handler/unrouted_handler.go index 3ff4b15..81f6c64 100644 --- a/pkg/handler/unrouted_handler.go +++ b/pkg/handler/unrouted_handler.go @@ -9,7 +9,6 @@ import ( "math" "net" "net/http" - "os" "regexp" "strconv" "strings" @@ -896,11 +895,6 @@ func (handler *UnroutedHandler) terminateUpload(ctx context.Context, upload Uplo // Send the error in the response body. The status code will be looked up in // ErrStatusCodes. If none is found 500 Internal Error will be used. func (handler *UnroutedHandler) sendError(w http.ResponseWriter, r *http.Request, err error) { - // Interpret os.ErrNotExist as 404 Not Found - if os.IsNotExist(err) { - err = ErrNotFound - } - // Errors for read timeouts contain too much information which is not // necessary for us and makes grouping for the metrics harder. The error // message looks like: read tcp 127.0.0.1:1080->127.0.0.1:53673: i/o timeout