diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-08-11 09:49:55 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-09-23 10:02:55 +0300 |
commit | 0e459e2167a73235f40f91d747039d7d9f2b0d33 (patch) | |
tree | 5a42cb8b1f7b15ee2b1149bef1b5eaaf7697d3a0 | |
parent | f1a28cb3f75d406ff9380729d2ffa902a5487f8d (diff) |
safe: Check error code when closing writers
Check error codes when closing a `safe.LockingFileWriter` or
`safe.Writer` and drop the corresponding exclude from our linting rules.
-rw-r--r-- | .golangci.yml | 2 | ||||
-rw-r--r-- | internal/cache/diskcache.go | 10 | ||||
-rw-r--r-- | internal/cache/keyer.go | 10 | ||||
-rw-r--r-- | internal/gitaly/service/repository/apply_gitattributes.go | 10 | ||||
-rw-r--r-- | internal/gitaly/storage/metadata.go | 11 |
5 files changed, 33 insertions, 10 deletions
diff --git a/.golangci.yml b/.golangci.yml index 36368721c..7b874bd4a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -61,8 +61,6 @@ linters-settings: - (*gitlab.com/gitlab-org/gitaly/v15/client.Pool).Close - (*gitlab.com/gitlab-org/gitaly/v15/client.SidechannelWaiter).Close - (*gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook.SidechannelWaiter).Close - - (*gitlab.com/gitlab-org/gitaly/v15/internal/safe.FileWriter).Close - - (*gitlab.com/gitlab-org/gitaly/v15/internal/safe.LockingFileWriter).Close - (*gitlab.com/gitlab-org/gitaly/v15/internal/sidechannel.ServerConn).Close - (*gitlab.com/gitlab-org/gitaly/v15/internal/sidechannel.Waiter).Close - (*gitlab.com/gitlab-org/gitaly/v15/internal/streamcache.pipe).Close diff --git a/internal/cache/diskcache.go b/internal/cache/diskcache.go index 192afde07..60408f4af 100644 --- a/internal/cache/diskcache.go +++ b/internal/cache/diskcache.go @@ -272,7 +272,7 @@ func (irc instrumentedReadCloser) Read(p []byte) (n int, err error) { // PutStream will store a stream in a repo-namespace keyed by the digest of the // request protobuf message. -func (c *DiskCache) PutStream(ctx context.Context, repo *gitalypb.Repository, req proto.Message, src io.Reader) error { +func (c *DiskCache) PutStream(ctx context.Context, repo *gitalypb.Repository, req proto.Message, src io.Reader) (returnedErr error) { reqPath, err := c.KeyPath(ctx, repo, req) if err != nil { return err @@ -298,7 +298,13 @@ func (c *DiskCache) PutStream(ctx context.Context, repo *gitalypb.Repository, re if err != nil { return err } - defer sf.Close() + defer func() { + if err := sf.Close(); err != nil && returnedErr == nil { + if !errors.Is(err, safe.ErrAlreadyDone) { + returnedErr = err + } + } + }() n, err = io.Copy(sf, src) if err != nil { diff --git a/internal/cache/keyer.go b/internal/cache/keyer.go index b89b240db..519edbd38 100644 --- a/internal/cache/keyer.go +++ b/internal/cache/keyer.go @@ -56,7 +56,7 @@ func newLeaseKeyer(locator storage.Locator, countErr func(error) error) leaseKey } } -func (keyer leaseKeyer) updateLatest(ctx context.Context, repo *gitalypb.Repository) (string, error) { +func (keyer leaseKeyer) updateLatest(ctx context.Context, repo *gitalypb.Repository) (_ string, returnedErr error) { repoStatePath, err := keyer.getRepoStatePath(repo) if err != nil { return "", err @@ -71,7 +71,13 @@ func (keyer leaseKeyer) updateLatest(ctx context.Context, repo *gitalypb.Reposit if err != nil { return "", err } - defer latest.Close() + defer func() { + if err := latest.Close(); err != nil && returnedErr == nil { + if !errors.Is(err, safe.ErrAlreadyDone) { + returnedErr = err + } + } + }() nextGenID := uuid.New().String() if nextGenID == "" { diff --git a/internal/gitaly/service/repository/apply_gitattributes.go b/internal/gitaly/service/repository/apply_gitattributes.go index 460a86ba5..700b2b6c2 100644 --- a/internal/gitaly/service/repository/apply_gitattributes.go +++ b/internal/gitaly/service/repository/apply_gitattributes.go @@ -22,7 +22,7 @@ import ( const attributesFileMode os.FileMode = 0o644 -func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, objectReader catfile.ObjectReader, repoPath string, revision []byte) error { +func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, objectReader catfile.ObjectReader, repoPath string, revision []byte) (returnedErr error) { infoPath := filepath.Join(repoPath, "info") attributesPath := filepath.Join(infoPath, "attributes") @@ -85,7 +85,13 @@ func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, o if err != nil { return fmt.Errorf("creating gitattributes writer: %w", err) } - defer writer.Close() + defer func() { + if err := writer.Close(); err != nil && returnedErr == nil { + if !errors.Is(err, safe.ErrAlreadyDone) { + returnedErr = err + } + } + }() if _, err := io.Copy(writer, blobObj); err != nil { return err diff --git a/internal/gitaly/storage/metadata.go b/internal/gitaly/storage/metadata.go index 1af49493c..f5d8e51e9 100644 --- a/internal/gitaly/storage/metadata.go +++ b/internal/gitaly/storage/metadata.go @@ -2,6 +2,7 @@ package storage import ( "encoding/json" + "errors" "os" "path/filepath" @@ -21,7 +22,7 @@ type Metadata struct { } // WriteMetadataFile marshals and writes a metadata file -func WriteMetadataFile(storagePath string) error { +func WriteMetadataFile(storagePath string) (returnedErr error) { path := filepath.Join(storagePath, metadataFilename) if _, err := os.Stat(path); !os.IsNotExist(err) { @@ -32,7 +33,13 @@ func WriteMetadataFile(storagePath string) error { if err != nil { return err } - defer fw.Close() + defer func() { + if err := fw.Close(); err != nil && returnedErr == nil { + if !errors.Is(err, safe.ErrAlreadyDone) { + returnedErr = err + } + } + }() if err = json.NewEncoder(fw).Encode(&Metadata{ GitalyFilesystemID: uuid.New().String(), |