diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-24 16:16:55 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-30 09:09:53 +0300 |
commit | 96bbfa25468d5c2655c4f5fc609a72192f6d0873 (patch) | |
tree | 8f1df0f68f72c5c8a6e377c0f3fbc9e9a165ab9c | |
parent | 86b607c1ffb7a7457ad567a68637603e09333c38 (diff) |
gitaly-lfs-smudge: Simplify error handling and logging
The way `smudge()` handles errors is a bit convoluted right now: in many
paths it will both log the error and return it to the caller, where the
caller should then use it as an indicator to decide whether to exit with
an error code or whether it should exit successfully.
Refactor the code so that we only log any such errors in our `main()`
function to simplify this logic and make it more Go-like. Also, now that
logging is mostly done in the `main()` function, move the initialization
of the logging system close to it.
-rw-r--r-- | cmd/gitaly-lfs-smudge/lfs_smudge.go | 32 | ||||
-rw-r--r-- | cmd/gitaly-lfs-smudge/main.go | 24 |
2 files changed, 28 insertions, 28 deletions
diff --git a/cmd/gitaly-lfs-smudge/lfs_smudge.go b/cmd/gitaly-lfs-smudge/lfs_smudge.go index 7d956aea7..75166b001 100644 --- a/cmd/gitaly-lfs-smudge/lfs_smudge.go +++ b/cmd/gitaly-lfs-smudge/lfs_smudge.go @@ -7,13 +7,11 @@ import ( "fmt" "io" "net/url" - "path/filepath" "github.com/git-lfs/git-lfs/lfs" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/prometheus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitlab" - gitalylog "gitlab.com/gitlab-org/gitaly/v15/internal/log" "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/labkit/tracing" ) @@ -22,22 +20,7 @@ type configProvider interface { Get(key string) string } -func initLogging(p configProvider) (io.Closer, error) { - path := p.Get(gitalylog.GitalyLogDirEnvKey) - if path == "" { - return log.Initialize(log.WithWriter(io.Discard)) - } - - filepath := filepath.Join(path, "gitaly_lfs_smudge.log") - - return log.Initialize( - log.WithFormatter("json"), - log.WithLogLevel("info"), - log.WithOutputName(filepath), - ) -} - -func smudge(to io.Writer, from io.Reader, cfgProvider configProvider) error { +func smudge(to io.Writer, from io.Reader, cfgProvider configProvider) (returnedErr error) { // Since the environment is sanitized at the moment, we're only // using this to extract the correlation ID. The finished() call // to clean up the tracing will be a NOP here. @@ -46,19 +29,16 @@ func smudge(to io.Writer, from io.Reader, cfgProvider configProvider) error { output, err := handleSmudge(ctx, to, from, cfgProvider) if err != nil { - log.WithError(err).Error(err) - return err + return fmt.Errorf("smudging contents: %w", err) } defer func() { - if err := output.Close(); err != nil { - log.ContextLogger(ctx).WithError(err).Error("closing LFS object: %w", err) + if err := output.Close(); err != nil && returnedErr == nil { + returnedErr = fmt.Errorf("closing LFS object: %w", err) } }() - _, copyErr := io.Copy(to, output) - if copyErr != nil { - log.WithError(err).Error(copyErr) - return copyErr + if _, err := io.Copy(to, output); err != nil { + return fmt.Errorf("writing smudged contents: %w", err) } return nil diff --git a/cmd/gitaly-lfs-smudge/main.go b/cmd/gitaly-lfs-smudge/main.go index 314105b50..2d39f2619 100644 --- a/cmd/gitaly-lfs-smudge/main.go +++ b/cmd/gitaly-lfs-smudge/main.go @@ -2,7 +2,12 @@ package main import ( "fmt" + "io" "os" + "path/filepath" + + gitalylog "gitlab.com/gitlab-org/gitaly/v15/internal/log" + "gitlab.com/gitlab-org/labkit/log" ) type envConfig struct{} @@ -36,8 +41,23 @@ func main() { } defer closer.Close() - err = smudge(os.Stdout, os.Stdin, &envConfig{}) - if err != nil { + if err := smudge(os.Stdout, os.Stdin, &envConfig{}); err != nil { + log.WithError(err).Error(err) os.Exit(1) } } + +func initLogging(p configProvider) (io.Closer, error) { + path := p.Get(gitalylog.GitalyLogDirEnvKey) + if path == "" { + return log.Initialize(log.WithWriter(io.Discard)) + } + + filepath := filepath.Join(path, "gitaly_lfs_smudge.log") + + return log.Initialize( + log.WithFormatter("json"), + log.WithLogLevel("info"), + log.WithOutputName(filepath), + ) +} |