diff --git a/go.mod b/go.mod index d00f7f3..3acb01e 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( github.com/spf13/cobra v1.10.1 github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 + go.uber.org/dig v1.19.0 go.uber.org/fx v1.24.0 golang.org/x/sync v0.18.0 golang.org/x/sys v0.38.0 @@ -256,7 +257,6 @@ require ( go.opentelemetry.io/otel/metric v1.38.0 // indirect go.opentelemetry.io/otel/trace v1.38.0 // indirect go.shabbyrobe.org/gocovmerge v0.0.0-20230507111327-fa4f82cfbf4d // indirect - go.uber.org/dig v1.19.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect diff --git a/internal/cli/app.go b/internal/cli/app.go index 889327f..5b4fb50 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -8,12 +8,12 @@ import ( "errors" "fmt" "path/filepath" - "strings" "sync" "time" "github.com/adrg/xdg" "github.com/spf13/cobra" + "go.uber.org/dig" "go.uber.org/fx" "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/database" @@ -146,16 +146,12 @@ func (e *startupError) Error() string { // could not build arguments for function "Y" (file:line): failed to build U: // received non-nil error from function "Z" (file:line): // -// Users care about the real error, not the DI plumbing. We strip everything -// up through the last "): " (which is always the close-paren of an fx -// function-location annotation followed by the wrapped error). +// Users care about the real error, not the DI plumbing. dig.RootCause, +// from the library fx is built on, unwraps those layers and returns the +// real error whole. Cutting the text at "): " instead would also cut a +// real error that contains it, such as the blob_size_limit one. func cleanStartupError(err error) error { - msg := err.Error() - if idx := strings.LastIndex(msg, "): "); idx >= 0 { - msg = msg[idx+3:] - } - - return &startupError{msg: msg} + return &startupError{msg: dig.RootCause(err).Error()} } // RunApp starts the fx application, blocks until it is asked to stop, and diff --git a/internal/cli/app_test.go b/internal/cli/app_test.go index 102bfae..c1de5a0 100644 --- a/internal/cli/app_test.go +++ b/internal/cli/app_test.go @@ -2,58 +2,65 @@ package cli //nolint:testpackage // needs access to unexported cleanStartupError import ( "errors" + "os" "path/filepath" "testing" + "go.uber.org/fx" + "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/pidlock" ) +// TestCleanStartupError starts a real fx app whose config has a +// blob_size_limit below the minimum. fx puts annotations naming the +// functions it was building in front of the config error; only those +// may be removed, although the config error itself contains "): ". func TestCleanStartupError(t *testing.T) { t.Parallel() - tests := []struct { - name string - in string - want string - }{ - { - name: "real fx error chain", - in: `could not build arguments for function ` + - `"sneak.berlin/go/vaultik/internal/cli".newSnapshotCreateCommand.func1.1 ` + - `(/Users/user/dev/vaultik/internal/cli/snapshot.go:71): ` + - `failed to build *vaultik.Vaultik: ` + - `could not build arguments for function ` + - `"sneak.berlin/go/vaultik/internal/vaultik".New ` + - `(/Users/user/dev/vaultik/internal/vaultik/vaultik.go:59): ` + - `failed to build storage.Storer: ` + - `received non-nil error from function ` + - `"sneak.berlin/go/vaultik/internal/storage".NewStorer ` + - `(/Users/user/dev/vaultik/internal/storage/module.go:23): ` + - `creating base path: mkdir /Volumes/BACKUPS: permission denied`, - want: `creating base path: mkdir /Volumes/BACKUPS: permission denied`, - }, - { - name: "no fx wrapping", - in: "plain error", - want: "plain error", - }, - { - name: "single fx wrapping", - in: `received non-nil error from function "foo" (file.go:1): underlying problem`, - want: "underlying problem", - }, + path := filepath.Join(t.TempDir(), "config.yml") + + err := os.WriteFile(path, []byte("snapshots:\n"+ + " test:\n"+ + " paths: [/tmp/vaultik-test-source]\n"+ + "storage_url: file:///tmp/vaultik-test-storage\n"+ + "chunk_size: 10MB\n"+ + "blob_size_limit: 30MB\n"), 0o600) + if err != nil { + t.Fatalf("write config: %v", err) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() + app := fx.New( + fx.NopLogger, + fx.Supply(config.Path(path)), + config.Module, + fx.Invoke(func(*config.Config) {}), + ) - //nolint:err113 // test constructs errors from table input - got := cleanStartupError(errors.New(tt.in)).Error() - if got != tt.want { - t.Errorf("got %q, want %q", got, tt.want) - } - }) + err = app.Start(t.Context()) + if err == nil { + t.Fatal("app started; want a blob_size_limit error") + } + + want := "failed to load config: invalid config: blob_size_limit must " + + "be at least the largest chunk the chunker can emit (chunk_size " + + "times the FastCDC size spread): need at least 40000000 bytes" + + got := cleanStartupError(err).Error() + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +// TestCleanStartupErrorNotFromFx checks that an error fx did not wrap is +// shown unchanged. +func TestCleanStartupErrorNotFromFx(t *testing.T) { + t.Parallel() + + //nolint:err113 // a one-off error standing in for a startup failure + got := cleanStartupError(errors.New("plain error")).Error() + if got != "plain error" { + t.Errorf("got %q, want %q", got, "plain error") } }