Skip to content

Commit 957cfa4

Browse files
Fix errcheck in ./common/archiver/ (#3744)
1 parent 1454f84 commit 957cfa4

File tree

7 files changed

+34
-19
lines changed

7 files changed

+34
-19
lines changed

common/archiver/filestore/historyArchiver_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ func (s *historyArchiverSuite) SetupSuite() {
9292
}
9393

9494
func (s *historyArchiverSuite) TearDownSuite() {
95-
os.RemoveAll(s.testGetDirectory)
95+
if err := os.RemoveAll(s.testGetDirectory); err != nil {
96+
s.Fail("Failed to remove test directory %v: %v", s.testGetDirectory, err)
97+
}
9698
}
9799

98100
func (s *historyArchiverSuite) SetupTest() {

common/archiver/filestore/util.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"github.com/dgryski/go-farm"
3737
"github.com/gogo/protobuf/proto"
3838
historypb "go.temporal.io/api/history/v1"
39+
"go.uber.org/multierr"
3940

4041
archiverspb "go.temporal.io/server/api/archiver/v1"
4142
"go.temporal.io/server/common/archiver"
@@ -110,7 +111,7 @@ func readFile(filepath string) ([]byte, error) {
110111
return os.ReadFile(filepath)
111112
}
112113

113-
func listFiles(dirPath string) ([]string, error) {
114+
func listFiles(dirPath string) (fileNames []string, err error) {
114115
if info, err := os.Stat(dirPath); err != nil {
115116
return nil, err
116117
} else if !info.IsDir() {
@@ -121,12 +122,10 @@ func listFiles(dirPath string) ([]string, error) {
121122
if err != nil {
122123
return nil, err
123124
}
124-
fileNames, err := f.Readdirnames(-1)
125-
f.Close()
126-
if err != nil {
127-
return nil, err
128-
}
129-
return fileNames, nil
125+
defer func() {
126+
err = multierr.Combine(err, f.Close())
127+
}()
128+
return f.Readdirnames(-1)
130129
}
131130

132131
func listFilesByPrefix(dirPath string, prefix string) ([]string, error) {

common/archiver/filestore/visibilityArchiver_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ func (s *visibilityArchiverSuite) SetupSuite() {
8383
}
8484

8585
func (s *visibilityArchiverSuite) TearDownSuite() {
86-
os.RemoveAll(s.testQueryDirectory)
86+
if err := os.RemoveAll(s.testQueryDirectory); err != nil {
87+
s.Fail("Failed to remove test query directory %v: %v", s.testQueryDirectory, err)
88+
}
8789
}
8890

8991
func (s *visibilityArchiverSuite) SetupTest() {

common/archiver/gcloud/connector/client.go

+8-6
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"os"
3535

3636
"cloud.google.com/go/storage"
37+
"go.uber.org/multierr"
3738
"google.golang.org/api/iterator"
3839

3940
"go.temporal.io/server/common/archiver"
@@ -125,15 +126,16 @@ func (s *storageWrapper) Exist(ctx context.Context, URI archiver.URI, fileName s
125126
}
126127

127128
// Get retrieve a file
128-
func (s *storageWrapper) Get(ctx context.Context, URI archiver.URI, fileName string) ([]byte, error) {
129+
func (s *storageWrapper) Get(ctx context.Context, URI archiver.URI, fileName string) (fileContent []byte, err error) {
129130
bucket := s.client.Bucket(URI.Hostname())
130131
reader, err := bucket.Object(formatSinkPath(URI.Path()) + "/" + fileName).NewReader(ctx)
131-
if err == nil {
132-
defer reader.Close()
133-
return io.ReadAll(reader)
132+
if err != nil {
133+
return nil, err
134134
}
135-
136-
return nil, err
135+
defer func() {
136+
err = multierr.Combine(err, reader.Close())
137+
}()
138+
return io.ReadAll(reader)
137139
}
138140

139141
// Query, retieves file names by provided storage query

common/archiver/gcloud/connector/client_test.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,19 @@ func (s *clientSuite) SetupTest() {
4949
s.controller = gomock.NewController(s.T())
5050
file, _ := json.MarshalIndent(&fakeData{data: "example"}, "", " ")
5151

52-
os.MkdirAll("/tmp/temporal_archival/development", os.ModePerm)
52+
path := "/tmp/temporal_archival/development"
53+
if err := os.MkdirAll(path, os.ModePerm); err != nil {
54+
s.FailNowf("Failed to create directory %s: %v", path, err)
55+
}
5356
s.Require().NoError(os.WriteFile("/tmp/temporal_archival/development/myfile.history", file, 0644))
5457
}
5558

5659
func (s *clientSuite) TearDownTest() {
5760
s.controller.Finish()
58-
os.Remove("/tmp/temporal_archival/development/myfile.history")
61+
name := "/tmp/temporal_archival/development/myfile.history"
62+
if err := os.Remove(name); err != nil {
63+
s.Failf("Failed to remove file %s: %v", name, err)
64+
}
5965
}
6066

6167
func TestClientSuite(t *testing.T) {

common/archiver/gcloud/historyArchiver.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,9 @@ func (h *historyArchiver) Archive(ctx context.Context, URI archiver.URI, request
192192
totalUploadSize = totalUploadSize + int64(binary.Size(encodedHistoryPart))
193193
}
194194

195-
saveHistoryIteratorState(ctx, featureCatalog, historyIterator, part, &progress)
195+
if err := saveHistoryIteratorState(ctx, featureCatalog, historyIterator, part, &progress); err != nil {
196+
return err
197+
}
196198
}
197199

198200
handler.Counter(metrics.HistoryArchiverTotalUploadSize.GetMetricName()).Record(totalUploadSize)

common/archiver/s3store/historyArchiver_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,9 @@ func setupFsEmulation(s3cli *mocks.MockS3API) {
114114

115115
putObjectFn := func(_ aws.Context, input *s3.PutObjectInput, _ ...request.Option) (*s3.PutObjectOutput, error) {
116116
buf := new(bytes.Buffer)
117-
buf.ReadFrom(input.Body)
117+
if _, err := buf.ReadFrom(input.Body); err != nil {
118+
return nil, err
119+
}
118120
fs[*input.Bucket+*input.Key] = buf.Bytes()
119121
return &s3.PutObjectOutput{}, nil
120122
}

0 commit comments

Comments
 (0)