From b231ed1fc61b307c47daa00e95d0959dab448179 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Wed, 11 Nov 2026 18:31:38 +0530 Subject: [PATCH 1/4] fix(kvstoreentry): close dir upload files and rewind on retry Close each file opened in ProcessDir and seek to the start before retrying insert after a connection-loss rewind error. Fixes fastly/cli#1942 Signed-off-by: Tyagiquamar Co-authored-by: Cursor --- CHANGELOG.md | 1 + pkg/commands/kvstoreentry/create.go | 23 ++++ .../kvstoreentry/create_processdir_test.go | 128 ++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 pkg/commands/kvstoreentry/create_processdir_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bcdc79b0..ab438b1d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Bug Fixes: +- fix(kvstoreentry): close directory upload files and rewind before retrying ([#1942](https://github.com/fastly/cli/issues/1942)) - fix(compute/serve): provide default `override_host` to Viceroy via temporary manifest and handle IPv6 hostnames ([#1940](https://github.com/fastly/cli/pull/1940)) ### Enhancements: diff --git a/pkg/commands/kvstoreentry/create.go b/pkg/commands/kvstoreentry/create.go index 1b93ce0fa..db1045f86 100644 --- a/pkg/commands/kvstoreentry/create.go +++ b/pkg/commands/kvstoreentry/create.go @@ -290,6 +290,9 @@ func (c *CreateCommand) ProcessDir(in io.Reader, out io.Writer) error { mu.Unlock() return } + defer func() { + _ = f.Close() + }() lr, err := fastly.FileLengthReader(f) if err != nil { @@ -317,6 +320,26 @@ func (c *CreateCommand) ProcessDir(in io.Reader, out io.Writer) error { // NOTE: you can't type assert the error as it's not exported. // https://github.com/golang/go/issues/54173 if strings.Contains(err.Error(), "net/http: cannot rewind body after connection loss") { + if _, seekErr := f.Seek(0, io.SeekStart); seekErr != nil { + mu.Lock() + processingErrors = append(processingErrors, ProcessErr{ + File: filePath, + Err: seekErr, + }) + mu.Unlock() + return + } + lr, rewindErr := fastly.FileLengthReader(f) + if rewindErr != nil { + mu.Lock() + processingErrors = append(processingErrors, ProcessErr{ + File: filePath, + Err: rewindErr, + }) + mu.Unlock() + return + } + opts.file = lr err = insertKey(opts) if err == nil { return diff --git a/pkg/commands/kvstoreentry/create_processdir_test.go b/pkg/commands/kvstoreentry/create_processdir_test.go new file mode 100644 index 000000000..1ea09cfd8 --- /dev/null +++ b/pkg/commands/kvstoreentry/create_processdir_test.go @@ -0,0 +1,128 @@ +package kvstoreentry + +import ( + "context" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/fastly/go-fastly/v19/fastly" + + "github.com/fastly/cli/pkg/argparser" + "github.com/fastly/cli/pkg/global" + "github.com/fastly/cli/pkg/mock" +) + +func TestProcessDirClosesUploadedFiles(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "foo.txt") + if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { + t.Fatal(err) + } + + var capturedBody io.ReadCloser + cmd := &CreateCommand{ + Base: argparser.Base{ + Globals: &global.Data{ + APIClient: &mock.API{ + InsertKVStoreKeyFn: func(_ context.Context, in *fastly.InsertKVStoreKeyInput) error { + if in.Key == "foo.txt" && capturedBody == nil { + if closer, ok := in.Body.(io.ReadCloser); ok { + capturedBody = closer + } + } + return nil + }, + }, + Flags: global.Flags{AutoYes: true, NonInteractive: true}, + }, + }, + dirPath: dir, + } + + if err := cmd.ProcessDir(nil, io.Discard); err != nil { + t.Fatalf("ProcessDir: %v", err) + } + if capturedBody == nil { + t.Fatal("expected InsertKVStoreKey body to be captured") + } + if _, err := capturedBody.Read(make([]byte, 1)); !errors.Is(err, os.ErrClosed) { + t.Fatalf("expected body to be closed after upload, got read err: %v", err) + } +} + +func TestProcessDirRetriesAfterConnectionLossWithRewoundBody(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "foo.txt") + if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { + t.Fatal(err) + } + + calls := 0 + cmd := &CreateCommand{ + Base: argparser.Base{ + Globals: &global.Data{ + APIClient: &mock.API{ + InsertKVStoreKeyFn: func(_ context.Context, in *fastly.InsertKVStoreKeyInput) error { + calls++ + if calls == 1 { + _, _ = io.ReadAll(in.Body) + return errors.New("net/http: cannot rewind body after connection loss") + } + data, err := io.ReadAll(in.Body) + if err != nil { + return err + } + if string(data) != "FOO\n" { + return errors.New("retry body mismatch") + } + return nil + }, + }, + Flags: global.Flags{AutoYes: true, NonInteractive: true}, + }, + }, + dirPath: dir, + } + + if err := cmd.ProcessDir(nil, io.Discard); err != nil { + t.Fatalf("ProcessDir: %v", err) + } + if calls != 2 { + t.Fatalf("expected 2 insert attempts, got %d", calls) + } +} + +func TestProcessDirRetryStillFailsReportsConnectionLoss(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "foo.txt") + if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { + t.Fatal(err) + } + + rewindErr := errors.New("net/http: cannot rewind body after connection loss") + cmd := &CreateCommand{ + Base: argparser.Base{ + Globals: &global.Data{ + APIClient: &mock.API{ + InsertKVStoreKeyFn: func(_ context.Context, _ *fastly.InsertKVStoreKeyInput) error { + return rewindErr + }, + }, + Flags: global.Flags{AutoYes: true, NonInteractive: true}, + }, + }, + dirPath: dir, + } + + err := cmd.ProcessDir(nil, io.Discard) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "failed to process all the provided files") { + t.Fatalf("unexpected error: %v", err) + } +} From 1246ba821e15989eda3f58a96226fb2e13318391 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Thu, 12 Nov 2026 17:37:03 +0530 Subject: [PATCH 2/4] test(kvstoreentry): set dir concurrency in ProcessDir tests ProcessDir tests construct CreateCommand without CLI flag defaults, leaving dirConcurrency at zero and deadlocking the upload semaphores. Use the same concurrency as the --dir-concurrency default and normalize test file LF. Co-authored-by: Cursor --- .../kvstoreentry/create_processdir_test.go | 259 +++++++++--------- 1 file changed, 131 insertions(+), 128 deletions(-) diff --git a/pkg/commands/kvstoreentry/create_processdir_test.go b/pkg/commands/kvstoreentry/create_processdir_test.go index 1ea09cfd8..c38a405c2 100644 --- a/pkg/commands/kvstoreentry/create_processdir_test.go +++ b/pkg/commands/kvstoreentry/create_processdir_test.go @@ -1,128 +1,131 @@ -package kvstoreentry - -import ( - "context" - "errors" - "io" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/fastly/go-fastly/v19/fastly" - - "github.com/fastly/cli/pkg/argparser" - "github.com/fastly/cli/pkg/global" - "github.com/fastly/cli/pkg/mock" -) - -func TestProcessDirClosesUploadedFiles(t *testing.T) { - dir := t.TempDir() - filePath := filepath.Join(dir, "foo.txt") - if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { - t.Fatal(err) - } - - var capturedBody io.ReadCloser - cmd := &CreateCommand{ - Base: argparser.Base{ - Globals: &global.Data{ - APIClient: &mock.API{ - InsertKVStoreKeyFn: func(_ context.Context, in *fastly.InsertKVStoreKeyInput) error { - if in.Key == "foo.txt" && capturedBody == nil { - if closer, ok := in.Body.(io.ReadCloser); ok { - capturedBody = closer - } - } - return nil - }, - }, - Flags: global.Flags{AutoYes: true, NonInteractive: true}, - }, - }, - dirPath: dir, - } - - if err := cmd.ProcessDir(nil, io.Discard); err != nil { - t.Fatalf("ProcessDir: %v", err) - } - if capturedBody == nil { - t.Fatal("expected InsertKVStoreKey body to be captured") - } - if _, err := capturedBody.Read(make([]byte, 1)); !errors.Is(err, os.ErrClosed) { - t.Fatalf("expected body to be closed after upload, got read err: %v", err) - } -} - -func TestProcessDirRetriesAfterConnectionLossWithRewoundBody(t *testing.T) { - dir := t.TempDir() - filePath := filepath.Join(dir, "foo.txt") - if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { - t.Fatal(err) - } - - calls := 0 - cmd := &CreateCommand{ - Base: argparser.Base{ - Globals: &global.Data{ - APIClient: &mock.API{ - InsertKVStoreKeyFn: func(_ context.Context, in *fastly.InsertKVStoreKeyInput) error { - calls++ - if calls == 1 { - _, _ = io.ReadAll(in.Body) - return errors.New("net/http: cannot rewind body after connection loss") - } - data, err := io.ReadAll(in.Body) - if err != nil { - return err - } - if string(data) != "FOO\n" { - return errors.New("retry body mismatch") - } - return nil - }, - }, - Flags: global.Flags{AutoYes: true, NonInteractive: true}, - }, - }, - dirPath: dir, - } - - if err := cmd.ProcessDir(nil, io.Discard); err != nil { - t.Fatalf("ProcessDir: %v", err) - } - if calls != 2 { - t.Fatalf("expected 2 insert attempts, got %d", calls) - } -} - -func TestProcessDirRetryStillFailsReportsConnectionLoss(t *testing.T) { - dir := t.TempDir() - filePath := filepath.Join(dir, "foo.txt") - if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { - t.Fatal(err) - } - - rewindErr := errors.New("net/http: cannot rewind body after connection loss") - cmd := &CreateCommand{ - Base: argparser.Base{ - Globals: &global.Data{ - APIClient: &mock.API{ - InsertKVStoreKeyFn: func(_ context.Context, _ *fastly.InsertKVStoreKeyInput) error { - return rewindErr - }, - }, - Flags: global.Flags{AutoYes: true, NonInteractive: true}, - }, - }, - dirPath: dir, - } - - err := cmd.ProcessDir(nil, io.Discard) - if err == nil { - t.Fatal("expected error") - } - if !strings.Contains(err.Error(), "failed to process all the provided files") { - t.Fatalf("unexpected error: %v", err) - } -} +package kvstoreentry + +import ( + "context" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/fastly/go-fastly/v19/fastly" + + "github.com/fastly/cli/pkg/argparser" + "github.com/fastly/cli/pkg/global" + "github.com/fastly/cli/pkg/mock" +) + +func TestProcessDirClosesUploadedFiles(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "foo.txt") + if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { + t.Fatal(err) + } + + var capturedBody io.ReadCloser + cmd := &CreateCommand{ + Base: argparser.Base{ + Globals: &global.Data{ + APIClient: &mock.API{ + InsertKVStoreKeyFn: func(_ context.Context, in *fastly.InsertKVStoreKeyInput) error { + if in.Key == "foo.txt" && capturedBody == nil { + if closer, ok := in.Body.(io.ReadCloser); ok { + capturedBody = closer + } + } + return nil + }, + }, + Flags: global.Flags{AutoYes: true, NonInteractive: true}, + }, + }, + dirPath: dir, + dirConcurrency: 50, + } + + if err := cmd.ProcessDir(nil, io.Discard); err != nil { + t.Fatalf("ProcessDir: %v", err) + } + if capturedBody == nil { + t.Fatal("expected InsertKVStoreKey body to be captured") + } + if _, err := capturedBody.Read(make([]byte, 1)); !errors.Is(err, os.ErrClosed) { + t.Fatalf("expected body to be closed after upload, got read err: %v", err) + } +} + +func TestProcessDirRetriesAfterConnectionLossWithRewoundBody(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "foo.txt") + if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { + t.Fatal(err) + } + + calls := 0 + cmd := &CreateCommand{ + Base: argparser.Base{ + Globals: &global.Data{ + APIClient: &mock.API{ + InsertKVStoreKeyFn: func(_ context.Context, in *fastly.InsertKVStoreKeyInput) error { + calls++ + if calls == 1 { + _, _ = io.ReadAll(in.Body) + return errors.New("net/http: cannot rewind body after connection loss") + } + data, err := io.ReadAll(in.Body) + if err != nil { + return err + } + if string(data) != "FOO\n" { + return errors.New("retry body mismatch") + } + return nil + }, + }, + Flags: global.Flags{AutoYes: true, NonInteractive: true}, + }, + }, + dirPath: dir, + dirConcurrency: 50, + } + + if err := cmd.ProcessDir(nil, io.Discard); err != nil { + t.Fatalf("ProcessDir: %v", err) + } + if calls != 2 { + t.Fatalf("expected 2 insert attempts, got %d", calls) + } +} + +func TestProcessDirRetryStillFailsReportsConnectionLoss(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "foo.txt") + if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { + t.Fatal(err) + } + + rewindErr := errors.New("net/http: cannot rewind body after connection loss") + cmd := &CreateCommand{ + Base: argparser.Base{ + Globals: &global.Data{ + APIClient: &mock.API{ + InsertKVStoreKeyFn: func(_ context.Context, _ *fastly.InsertKVStoreKeyInput) error { + return rewindErr + }, + }, + Flags: global.Flags{AutoYes: true, NonInteractive: true}, + }, + }, + dirPath: dir, + dirConcurrency: 50, + } + + err := cmd.ProcessDir(nil, io.Discard) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "failed to process all the provided files") { + t.Fatalf("unexpected error: %v", err) + } +} From 4bedf4babd2120104418468c44863bc0d147203b Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Thu, 12 Nov 2026 18:16:44 +0530 Subject: [PATCH 3/4] test(kvstoreentry): assert LengthReader closed after ProcessDir Signed-off-by: Tyagiquamar --- pkg/commands/kvstoreentry/create_processdir_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/commands/kvstoreentry/create_processdir_test.go b/pkg/commands/kvstoreentry/create_processdir_test.go index c38a405c2..108b4ec9b 100644 --- a/pkg/commands/kvstoreentry/create_processdir_test.go +++ b/pkg/commands/kvstoreentry/create_processdir_test.go @@ -23,15 +23,15 @@ func TestProcessDirClosesUploadedFiles(t *testing.T) { t.Fatal(err) } - var capturedBody io.ReadCloser + var capturedBody fastly.LengthReader cmd := &CreateCommand{ Base: argparser.Base{ Globals: &global.Data{ APIClient: &mock.API{ InsertKVStoreKeyFn: func(_ context.Context, in *fastly.InsertKVStoreKeyInput) error { if in.Key == "foo.txt" && capturedBody == nil { - if closer, ok := in.Body.(io.ReadCloser); ok { - capturedBody = closer + if lr, ok := in.Body.(fastly.LengthReader); ok { + capturedBody = lr } } return nil @@ -48,10 +48,10 @@ func TestProcessDirClosesUploadedFiles(t *testing.T) { t.Fatalf("ProcessDir: %v", err) } if capturedBody == nil { - t.Fatal("expected InsertKVStoreKey body to be captured") + t.Fatal("expected InsertKVStoreKey LengthReader body to be captured") } if _, err := capturedBody.Read(make([]byte, 1)); !errors.Is(err, os.ErrClosed) { - t.Fatalf("expected body to be closed after upload, got read err: %v", err) + t.Fatalf("expected upload file to be closed after ProcessDir, got read err: %v", err) } } From 15b37049aadd5a5fdd79b724483e85f1710e30dc Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Fri, 13 Nov 2026 14:33:50 +0530 Subject: [PATCH 4/4] fix(kvstoreentry): seek file on dir upload retry Close opened files after directory upload and rewind with Seek(0) before retrying insert after connection loss, per review feedback. Co-authored-by: Cursor --- pkg/commands/kvstoreentry/create.go | 11 ------ .../kvstoreentry/create_processdir_test.go | 37 +------------------ 2 files changed, 1 insertion(+), 47 deletions(-) diff --git a/pkg/commands/kvstoreentry/create.go b/pkg/commands/kvstoreentry/create.go index db1045f86..9a50b9cc7 100644 --- a/pkg/commands/kvstoreentry/create.go +++ b/pkg/commands/kvstoreentry/create.go @@ -329,17 +329,6 @@ func (c *CreateCommand) ProcessDir(in io.Reader, out io.Writer) error { mu.Unlock() return } - lr, rewindErr := fastly.FileLengthReader(f) - if rewindErr != nil { - mu.Lock() - processingErrors = append(processingErrors, ProcessErr{ - File: filePath, - Err: rewindErr, - }) - mu.Unlock() - return - } - opts.file = lr err = insertKey(opts) if err == nil { return diff --git a/pkg/commands/kvstoreentry/create_processdir_test.go b/pkg/commands/kvstoreentry/create_processdir_test.go index 108b4ec9b..6d9045255 100644 --- a/pkg/commands/kvstoreentry/create_processdir_test.go +++ b/pkg/commands/kvstoreentry/create_processdir_test.go @@ -6,7 +6,6 @@ import ( "io" "os" "path/filepath" - "strings" "testing" "github.com/fastly/go-fastly/v19/fastly" @@ -30,9 +29,7 @@ func TestProcessDirClosesUploadedFiles(t *testing.T) { APIClient: &mock.API{ InsertKVStoreKeyFn: func(_ context.Context, in *fastly.InsertKVStoreKeyInput) error { if in.Key == "foo.txt" && capturedBody == nil { - if lr, ok := in.Body.(fastly.LengthReader); ok { - capturedBody = lr - } + capturedBody = in.Body } return nil }, @@ -97,35 +94,3 @@ func TestProcessDirRetriesAfterConnectionLossWithRewoundBody(t *testing.T) { t.Fatalf("expected 2 insert attempts, got %d", calls) } } - -func TestProcessDirRetryStillFailsReportsConnectionLoss(t *testing.T) { - dir := t.TempDir() - filePath := filepath.Join(dir, "foo.txt") - if err := os.WriteFile(filePath, []byte("FOO\n"), 0o600); err != nil { - t.Fatal(err) - } - - rewindErr := errors.New("net/http: cannot rewind body after connection loss") - cmd := &CreateCommand{ - Base: argparser.Base{ - Globals: &global.Data{ - APIClient: &mock.API{ - InsertKVStoreKeyFn: func(_ context.Context, _ *fastly.InsertKVStoreKeyInput) error { - return rewindErr - }, - }, - Flags: global.Flags{AutoYes: true, NonInteractive: true}, - }, - }, - dirPath: dir, - dirConcurrency: 50, - } - - err := cmd.ProcessDir(nil, io.Discard) - if err == nil { - t.Fatal("expected error") - } - if !strings.Contains(err.Error(), "failed to process all the provided files") { - t.Fatalf("unexpected error: %v", err) - } -}