From b231ed1fc61b307c47daa00e95d0959dab448179 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Wed, 11 Nov 2026 18:31:38 +0530 Subject: [PATCH] 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) + } +}