From 957cfa4f22c7e67c48fc571c29a3df8c7722fc04 Mon Sep 17 00:00:00 2001 From: Michael Snowden Date: Wed, 21 Dec 2022 13:00:26 -0800 Subject: [PATCH] Fix errcheck in ./common/archiver/ (#3744) --- common/archiver/filestore/historyArchiver_test.go | 4 +++- common/archiver/filestore/util.go | 13 ++++++------- .../archiver/filestore/visibilityArchiver_test.go | 4 +++- common/archiver/gcloud/connector/client.go | 14 ++++++++------ common/archiver/gcloud/connector/client_test.go | 10 ++++++++-- common/archiver/gcloud/historyArchiver.go | 4 +++- common/archiver/s3store/historyArchiver_test.go | 4 +++- 7 files changed, 34 insertions(+), 19 deletions(-) diff --git a/common/archiver/filestore/historyArchiver_test.go b/common/archiver/filestore/historyArchiver_test.go index 5d741a3fc7e..ac9211df68a 100644 --- a/common/archiver/filestore/historyArchiver_test.go +++ b/common/archiver/filestore/historyArchiver_test.go @@ -92,7 +92,9 @@ func (s *historyArchiverSuite) SetupSuite() { } func (s *historyArchiverSuite) TearDownSuite() { - os.RemoveAll(s.testGetDirectory) + if err := os.RemoveAll(s.testGetDirectory); err != nil { + s.Fail("Failed to remove test directory %v: %v", s.testGetDirectory, err) + } } func (s *historyArchiverSuite) SetupTest() { diff --git a/common/archiver/filestore/util.go b/common/archiver/filestore/util.go index 7624f38e9ac..289575eac4c 100644 --- a/common/archiver/filestore/util.go +++ b/common/archiver/filestore/util.go @@ -36,6 +36,7 @@ import ( "github.com/dgryski/go-farm" "github.com/gogo/protobuf/proto" historypb "go.temporal.io/api/history/v1" + "go.uber.org/multierr" archiverspb "go.temporal.io/server/api/archiver/v1" "go.temporal.io/server/common/archiver" @@ -110,7 +111,7 @@ func readFile(filepath string) ([]byte, error) { return os.ReadFile(filepath) } -func listFiles(dirPath string) ([]string, error) { +func listFiles(dirPath string) (fileNames []string, err error) { if info, err := os.Stat(dirPath); err != nil { return nil, err } else if !info.IsDir() { @@ -121,12 +122,10 @@ func listFiles(dirPath string) ([]string, error) { if err != nil { return nil, err } - fileNames, err := f.Readdirnames(-1) - f.Close() - if err != nil { - return nil, err - } - return fileNames, nil + defer func() { + err = multierr.Combine(err, f.Close()) + }() + return f.Readdirnames(-1) } func listFilesByPrefix(dirPath string, prefix string) ([]string, error) { diff --git a/common/archiver/filestore/visibilityArchiver_test.go b/common/archiver/filestore/visibilityArchiver_test.go index 2511cfd9c81..fc3dbee4253 100644 --- a/common/archiver/filestore/visibilityArchiver_test.go +++ b/common/archiver/filestore/visibilityArchiver_test.go @@ -83,7 +83,9 @@ func (s *visibilityArchiverSuite) SetupSuite() { } func (s *visibilityArchiverSuite) TearDownSuite() { - os.RemoveAll(s.testQueryDirectory) + if err := os.RemoveAll(s.testQueryDirectory); err != nil { + s.Fail("Failed to remove test query directory %v: %v", s.testQueryDirectory, err) + } } func (s *visibilityArchiverSuite) SetupTest() { diff --git a/common/archiver/gcloud/connector/client.go b/common/archiver/gcloud/connector/client.go index 06ce3bf1b26..79a556f840a 100644 --- a/common/archiver/gcloud/connector/client.go +++ b/common/archiver/gcloud/connector/client.go @@ -34,6 +34,7 @@ import ( "os" "cloud.google.com/go/storage" + "go.uber.org/multierr" "google.golang.org/api/iterator" "go.temporal.io/server/common/archiver" @@ -125,15 +126,16 @@ func (s *storageWrapper) Exist(ctx context.Context, URI archiver.URI, fileName s } // Get retrieve a file -func (s *storageWrapper) Get(ctx context.Context, URI archiver.URI, fileName string) ([]byte, error) { +func (s *storageWrapper) Get(ctx context.Context, URI archiver.URI, fileName string) (fileContent []byte, err error) { bucket := s.client.Bucket(URI.Hostname()) reader, err := bucket.Object(formatSinkPath(URI.Path()) + "/" + fileName).NewReader(ctx) - if err == nil { - defer reader.Close() - return io.ReadAll(reader) + if err != nil { + return nil, err } - - return nil, err + defer func() { + err = multierr.Combine(err, reader.Close()) + }() + return io.ReadAll(reader) } // Query, retieves file names by provided storage query diff --git a/common/archiver/gcloud/connector/client_test.go b/common/archiver/gcloud/connector/client_test.go index f2626e85120..dead1a77ac4 100644 --- a/common/archiver/gcloud/connector/client_test.go +++ b/common/archiver/gcloud/connector/client_test.go @@ -49,13 +49,19 @@ func (s *clientSuite) SetupTest() { s.controller = gomock.NewController(s.T()) file, _ := json.MarshalIndent(&fakeData{data: "example"}, "", " ") - os.MkdirAll("/tmp/temporal_archival/development", os.ModePerm) + path := "/tmp/temporal_archival/development" + if err := os.MkdirAll(path, os.ModePerm); err != nil { + s.FailNowf("Failed to create directory %s: %v", path, err) + } s.Require().NoError(os.WriteFile("/tmp/temporal_archival/development/myfile.history", file, 0644)) } func (s *clientSuite) TearDownTest() { s.controller.Finish() - os.Remove("/tmp/temporal_archival/development/myfile.history") + name := "/tmp/temporal_archival/development/myfile.history" + if err := os.Remove(name); err != nil { + s.Failf("Failed to remove file %s: %v", name, err) + } } func TestClientSuite(t *testing.T) { diff --git a/common/archiver/gcloud/historyArchiver.go b/common/archiver/gcloud/historyArchiver.go index e7ba3a3844c..2f105b0173e 100644 --- a/common/archiver/gcloud/historyArchiver.go +++ b/common/archiver/gcloud/historyArchiver.go @@ -192,7 +192,9 @@ func (h *historyArchiver) Archive(ctx context.Context, URI archiver.URI, request totalUploadSize = totalUploadSize + int64(binary.Size(encodedHistoryPart)) } - saveHistoryIteratorState(ctx, featureCatalog, historyIterator, part, &progress) + if err := saveHistoryIteratorState(ctx, featureCatalog, historyIterator, part, &progress); err != nil { + return err + } } handler.Counter(metrics.HistoryArchiverTotalUploadSize.GetMetricName()).Record(totalUploadSize) diff --git a/common/archiver/s3store/historyArchiver_test.go b/common/archiver/s3store/historyArchiver_test.go index 8fb1468e59f..ab8a0ae70be 100644 --- a/common/archiver/s3store/historyArchiver_test.go +++ b/common/archiver/s3store/historyArchiver_test.go @@ -114,7 +114,9 @@ func setupFsEmulation(s3cli *mocks.MockS3API) { putObjectFn := func(_ aws.Context, input *s3.PutObjectInput, _ ...request.Option) (*s3.PutObjectOutput, error) { buf := new(bytes.Buffer) - buf.ReadFrom(input.Body) + if _, err := buf.ReadFrom(input.Body); err != nil { + return nil, err + } fs[*input.Bucket+*input.Key] = buf.Bytes() return &s3.PutObjectOutput{}, nil }