diff --git a/cmd/auth_token.go b/cmd/auth_token.go index 894c0507e..5b4ac0eb2 100644 --- a/cmd/auth_token.go +++ b/cmd/auth_token.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/molecula/featurebase/v3/ctl" @@ -17,9 +16,7 @@ func newAuthTokenCommand(stdin io.Reader, stdout io.Writer, stderr io.Writer) *c Long: ` Retrieves an auth-token for use in authenticating with FeatureBase from the configured identity provider. `, - RunE: func(c *cobra.Command, args []string) error { - return cmd.Run(context.Background()) - }, + RunE: usageErrorWrapper(cmd), } flags := ccmd.Flags() diff --git a/cmd/backup.go b/cmd/backup.go index 97243d9a0..275b62d2d 100644 --- a/cmd/backup.go +++ b/cmd/backup.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/molecula/featurebase/v3/ctl" @@ -17,9 +16,7 @@ func newBackupCommand(stdin io.Reader, stdout io.Writer, stderr io.Writer) *cobr Long: ` Backs up a FeatureBase server to a local, tar-formatted snapshot file. `, - RunE: func(c *cobra.Command, args []string) error { - return cmd.Run(context.Background()) - }, + RunE: usageErrorWrapper(cmd), } flags := ccmd.Flags() diff --git a/cmd/backup_tar.go b/cmd/backup_tar.go index e480a487c..1c53b5d14 100644 --- a/cmd/backup_tar.go +++ b/cmd/backup_tar.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/molecula/featurebase/v3/ctl" @@ -17,9 +16,7 @@ func newBackupTarCommand(stdin io.Reader, stdout io.Writer, stderr io.Writer) *c Long: ` Backs up a FeatureBase server to a local, tar-formatted snapshot file. `, - RunE: func(c *cobra.Command, args []string) error { - return cmd.Run(context.Background()) - }, + RunE: usageErrorWrapper(cmd), } flags := ccmd.Flags() diff --git a/cmd/chksum.go b/cmd/chksum.go index 54a8643f9..78896027a 100644 --- a/cmd/chksum.go +++ b/cmd/chksum.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/molecula/featurebase/v3/ctl" @@ -18,9 +17,7 @@ func newChkSumCommand(stdin io.Reader, stdout io.Writer, stderr io.Writer) *cobr Generates a digital signature of all the data associated with a provided FeatureBase server WARNING: could be slow if high cardinality fields exist `, - RunE: func(c *cobra.Command, args []string) error { - return cmd.Run(context.Background()) - }, + RunE: usageErrorWrapper(cmd), } flags := ccmd.Flags() diff --git a/cmd/cli.go b/cmd/cli.go index 88b279508..412290770 100644 --- a/cmd/cli.go +++ b/cmd/cli.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/molecula/featurebase/v3/ctl" @@ -18,9 +17,7 @@ func newCLICommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command { Use: "cli", Short: "Query FB with SQL3 from the command line", Long: ``, - RunE: func(cmd *cobra.Command, args []string) error { - return cli.Run(context.Background()) - }, + RunE: usageErrorWrapper(cli), } flags := cliCmd.Flags() diff --git a/cmd/config.go b/cmd/config.go index fc3e11b24..e3167657d 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -23,7 +23,7 @@ func newConfigCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command RunE: func(cmd *cobra.Command, args []string) error { conf.Config = Server.Config - return conf.Run(context.Background()) + return considerUsageError(cmd, conf.Run(context.Background())) }, } diff --git a/cmd/dax.go b/cmd/dax.go index 84b8bed2a..957db266a 100644 --- a/cmd/dax.go +++ b/cmd/dax.go @@ -19,7 +19,7 @@ func newDAXCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command { Long: ``, RunE: func(cmd *cobra.Command, args []string) error { if err := server.Start(); err != nil { - return errors.Wrap(err, "running server") + return considerUsageError(cmd, errors.Wrap(err, "running server")) } return errors.Wrap(server.Wait(), "waiting on Server") }, diff --git a/cmd/export.go b/cmd/export.go index aae8f13d9..ec9fbdc4b 100644 --- a/cmd/export.go +++ b/cmd/export.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/spf13/cobra" @@ -27,9 +26,7 @@ The format of the CSV file is: The file does not contain any headers. `, - RunE: func(cmd *cobra.Command, args []string) error { - return Exporter.Run(context.Background()) - }, + RunE: usageErrorWrapper(Exporter), } flags := exportCmd.Flags() diff --git a/cmd/generate_config.go b/cmd/generate_config.go index ce671f886..cd5400cd9 100644 --- a/cmd/generate_config.go +++ b/cmd/generate_config.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/spf13/cobra" @@ -19,9 +18,7 @@ func newGenerateConfigCommand(stdin io.Reader, stdout io.Writer, stderr io.Write Short: "Print the default configuration.", Long: `generate-config prints the default configuration to stdout `, - RunE: func(cmd *cobra.Command, args []string) error { - return generateConf.Run(context.Background()) - }, + RunE: usageErrorWrapper(generateConf), } return confCmd diff --git a/cmd/import.go b/cmd/import.go index fe69c1cb3..067faa97b 100644 --- a/cmd/import.go +++ b/cmd/import.go @@ -60,7 +60,7 @@ omitted. If it is present then its format should be YYYY-MM-DDTHH:MM. `, RunE: func(cmd *cobra.Command, args []string) error { Importer.Paths = args - return Importer.Run(context.Background()) + return considerUsageError(cmd, Importer.Run(context.Background())) }, } diff --git a/cmd/keygen.go b/cmd/keygen.go index 8bf9166f0..a93d2e69c 100644 --- a/cmd/keygen.go +++ b/cmd/keygen.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/molecula/featurebase/v3/ctl" @@ -17,9 +16,7 @@ func newKeygenCommand(stdin io.Reader, stdout io.Writer, stderr io.Writer) *cobr Long: ` Generate secret key to configure FeatureBase for Authentication. `, - RunE: func(c *cobra.Command, args []string) error { - return cmd.Run(context.Background()) - }, + RunE: usageErrorWrapper(cmd), } flags := ccmd.Flags() diff --git a/cmd/rbf.go b/cmd/rbf.go index b7c1925c4..ec4207511 100644 --- a/cmd/rbf.go +++ b/cmd/rbf.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "errors" "fmt" "io" @@ -44,9 +43,7 @@ Executes a consistency check on an RBF data directory. c.Path = args[0] return nil }, - RunE: func(cmd *cobra.Command, args []string) error { - return c.Run(context.Background()) - }, + RunE: usageErrorWrapper(c), } return cmd } @@ -78,9 +75,7 @@ Dumps the raw hex data for one or more RBF pages. return nil }, - RunE: func(cmd *cobra.Command, args []string) error { - return c.Run(context.Background()) - }, + RunE: usageErrorWrapper(c), } return cmd } @@ -102,9 +97,7 @@ Prints a line for every page in the database with its type/status. c.Path = args[0] return nil }, - RunE: func(cmd *cobra.Command, args []string) error { - return c.Run(context.Background()) - }, + RunE: usageErrorWrapper(c), } flags := cmd.Flags() @@ -139,9 +132,7 @@ Prints the header & cell data for one or more pages. return nil }, - RunE: func(cmd *cobra.Command, args []string) error { - return c.Run(context.Background()) - }, + RunE: usageErrorWrapper(c), } return cmd } diff --git a/cmd/restore.go b/cmd/restore.go index f9f8c32db..3f1f7b5ec 100644 --- a/cmd/restore.go +++ b/cmd/restore.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/molecula/featurebase/v3/ctl" @@ -17,9 +16,7 @@ func newRestoreCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command Long: ` The Restore command will take a backup archive and restore it to a new, clean cluster. `, - RunE: func(c *cobra.Command, args []string) error { - return cmd.Run(context.Background()) - }, + RunE: usageErrorWrapper(cmd), } flags := restoreCmd.Flags() flags.StringVarP(&cmd.Path, "source", "s", "", "backup file; specify '-' to restore from stdin tar stream") diff --git a/cmd/restore_tar.go b/cmd/restore_tar.go index 563487429..fd65f7f95 100644 --- a/cmd/restore_tar.go +++ b/cmd/restore_tar.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/molecula/featurebase/v3/ctl" @@ -17,9 +16,7 @@ func newRestoreTarCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Comm Long: ` The Restore command will take a tar-formatted backup archive and restore it to a new, clean cluster. `, - RunE: func(c *cobra.Command, args []string) error { - return cmd.Run(context.Background()) - }, + RunE: usageErrorWrapper(cmd), } flags := restoreCmd.Flags() flags.StringVarP(&cmd.Path, "source", "s", "", "backup file; specify '-' to restore from stdin tar stream") diff --git a/cmd/root.go b/cmd/root.go index 08caf6880..8536e1733 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -2,16 +2,49 @@ package cmd import ( + "context" + "errors" "fmt" "io" "strings" pilosa "github.com/molecula/featurebase/v3" + "github.com/molecula/featurebase/v3/ctl" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" ) +// runner represents a thing, like any of the NewFooCommand we produce +// in ctl/*.go, which has a Run(context) error method, which is used +// to implement a command. We use this so we can just specify the +// object, rather than its run method, in calling usageErrorWrapper. +type runner interface { + Run(context.Context) error +} + +// usageErrorWrapper takes a thing with a Run(context) error, and produces +// a func(*cobra.Command, []string) error from it which will run that +// command, and then set Cobra's SilenceUsage flag unless the returned +// error errors.Is() a ctl.UsageError. +func usageErrorWrapper(inner runner) func(*cobra.Command, []string) error { + return func(c *cobra.Command, args []string) error { + return considerUsageError(c, inner.Run(context.Background())) + } +} + +// considerUsageError sets a command to silence usage errors if +// the given error is not a ctl.UsageError, then returns the +// unmodified error. It's here to let us write one-liner Run +// wrappers. +func considerUsageError(cmd *cobra.Command, err error) error { + cmd.SilenceErrors = true + if !errors.Is(err, ctl.UsageError) { + cmd.SilenceUsage = true + } + return err +} + func NewRootCommand(stdin io.Reader, stdout, stderr io.Writer) *cobra.Command { rc := &cobra.Command{ Use: "featurebase", @@ -48,6 +81,7 @@ at https://docs.molecula.cloud/. return nil }, + SilenceErrors: true, } rc.PersistentFlags().Bool("dry-run", false, "stop before executing") _ = rc.PersistentFlags().MarkHidden("dry-run") diff --git a/cmd/server.go b/cmd/server.go index 9b0d762c1..a0eafb745 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -58,8 +58,11 @@ on the configured port.`, RunE: func(cmd *cobra.Command, args []string) error { // Start & run the server. if err := Server.Start(); err != nil { - return errors.Wrap(err, "running server") + return considerUsageError(cmd, errors.Wrap(err, "running server")) } + // anything past here is definitely not a usage error + cmd.SilenceErrors = true + cmd.SilenceUsage = true if Server.Config.DataDog.Enable { opts := make([]profiler.ProfileType, 0) if Server.Config.DataDog.CPUProfile { diff --git a/ctl/backup.go b/ctl/backup.go index b97e98a0c..506d1452d 100644 --- a/ctl/backup.go +++ b/ctl/backup.go @@ -5,8 +5,10 @@ import ( "context" "crypto/tls" "encoding/json" + "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "time" @@ -16,7 +18,6 @@ import ( "github.com/molecula/featurebase/v3/disco" "github.com/molecula/featurebase/v3/encoding/proto" "github.com/molecula/featurebase/v3/server" - "github.com/pkg/errors" "github.com/ricochet2200/go-disk-usage/du" "golang.org/x/sync/errgroup" ) @@ -80,19 +81,19 @@ func (cmd *BackupCommand) Run(ctx context.Context) (err error) { logger := cmd.Logger() close, err := startProfilingServer(cmd.Pprof, logger) if err != nil { - return errors.Wrap(err, "starting profiling server") + return fmt.Errorf("starting profiling server: %w", err) } defer close() // Validate arguments. if cmd.OutputDir == "" { - return fmt.Errorf("-o flag required") + return fmt.Errorf("%w: -o flag required", UsageError) } else if cmd.Concurrency <= 0 { - return fmt.Errorf("concurrency must be at least one") + return fmt.Errorf("%w: concurrency must be at least one", UsageError) } if cmd.HeaderTimeoutStr != "" { if dur, err := time.ParseDuration(cmd.HeaderTimeoutStr); err != nil { - return fmt.Errorf("could not parse '%s' as a duration: %v", cmd.HeaderTimeoutStr, err) + return fmt.Errorf("%w: could not parse '%s' as a duration: %v", UsageError, cmd.HeaderTimeoutStr, err) } else { cmd.HeaderTimeout = dur } @@ -136,7 +137,7 @@ func (cmd *BackupCommand) Run(ctx context.Context) (err error) { schema := &pilosa.Schema{Indexes: indexes} // Ensure output directory doesn't exist; then create output directory. - if _, err := os.Stat(cmd.OutputDir); !os.IsNotExist(err) { + if _, err := os.Stat(cmd.OutputDir); !errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("output directory already exists") } else if err := os.MkdirAll(cmd.OutputDir, 0o750); err != nil { return err diff --git a/ctl/backup_tar.go b/ctl/backup_tar.go index 295b50127..879045dbf 100644 --- a/ctl/backup_tar.go +++ b/ctl/backup_tar.go @@ -76,13 +76,13 @@ func (cmd *BackupTarCommand) Run(ctx context.Context) (err error) { // Validate arguments. if cmd.OutputPath == "" { - return fmt.Errorf("-o flag required") + return fmt.Errorf("%w: -o flag required", UsageError) } useStdout := cmd.OutputPath == "-" if cmd.HeaderTimeoutStr != "" { if dur, err := time.ParseDuration(cmd.HeaderTimeoutStr); err != nil { - return fmt.Errorf("could not parse '%s' as a duration: %v", cmd.HeaderTimeoutStr, err) + return fmt.Errorf("%w: could not parse '%s' as a duration: %v", UsageError, cmd.HeaderTimeoutStr, err) } else { cmd.HeaderTimeout = dur } diff --git a/ctl/export.go b/ctl/export.go index 43c988693..b665bb23b 100644 --- a/ctl/export.go +++ b/ctl/export.go @@ -3,10 +3,11 @@ package ctl import ( "context" + "fmt" "io" "os" - "github.com/molecula/featurebase/v3" + pilosa "github.com/molecula/featurebase/v3" "github.com/molecula/featurebase/v3/server" "github.com/pkg/errors" ) @@ -42,9 +43,9 @@ func (cmd *ExportCommand) Run(ctx context.Context) error { // Validate arguments. if cmd.Index == "" { - return pilosa.ErrIndexRequired + return fmt.Errorf("%w: %v", UsageError, pilosa.ErrIndexRequired) } else if cmd.Field == "" { - return pilosa.ErrFieldRequired + return fmt.Errorf("%w: %v", UsageError, pilosa.ErrFieldRequired) } // Use output file, if specified. diff --git a/ctl/export_test.go b/ctl/export_test.go index 8dbdf08c3..6b4432caf 100644 --- a/ctl/export_test.go +++ b/ctl/export_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - "github.com/molecula/featurebase/v3" + pilosa "github.com/molecula/featurebase/v3" "github.com/molecula/featurebase/v3/test" ) @@ -19,14 +19,14 @@ func TestExportCommand_Validation(t *testing.T) { cm := NewExportCommand(stdin, stdout, stderr) err := cm.Run(context.Background()) - if err != pilosa.ErrIndexRequired { - t.Fatalf("Command not working, expect: %s, actual: '%s'", pilosa.ErrIndexRequired, err) + if !errContains(err, pilosa.ErrIndexRequired) { + t.Fatalf("wrong error, expected %q, got: '%s'", pilosa.ErrIndexRequired, err) } cm.Index = "i" err = cm.Run(context.Background()) - if err != pilosa.ErrFieldRequired { - t.Fatalf("Command not working, expect: %s, actual: '%s'", pilosa.ErrFieldRequired, err) + if !errContains(err, pilosa.ErrFieldRequired) { + t.Fatalf("wrong error, expected %q, got: '%s'", pilosa.ErrFieldRequired, err) } } diff --git a/ctl/import.go b/ctl/import.go index 3818c8f4b..62ba4605b 100644 --- a/ctl/import.go +++ b/ctl/import.go @@ -79,11 +79,11 @@ func (cmd *ImportCommand) Run(ctx context.Context) error { // Validate arguments. // Index and field are validated early before the files are parsed. if cmd.Index == "" { - return pilosa.ErrIndexRequired + return fmt.Errorf("%w: %v", UsageError, pilosa.ErrIndexRequired) } else if cmd.Field == "" { - return pilosa.ErrFieldRequired + return fmt.Errorf("%w: %v", UsageError, pilosa.ErrFieldRequired) } else if len(cmd.Paths) == 0 { - return errors.New("path required") + return fmt.Errorf("%w: path required", UsageError) } // Create a client to the server. client, err := commandClient(cmd) diff --git a/ctl/import_test.go b/ctl/import_test.go index e80576707..e30cb7960 100644 --- a/ctl/import_test.go +++ b/ctl/import_test.go @@ -6,6 +6,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -25,25 +26,47 @@ import ( "github.com/molecula/featurebase/v3/testhook" ) +// errContains reports whether the first error is or +// contains the second, either via errors.Is, or by +// containing its Error() string. a nil error contains +// a nil error, but does not contain any non-nil error, +// and a non-nil error does not contain a nil error. +func errContains(err error, expected error) bool { + if err == nil { + if expected == nil { + return true + } + return false + } + if expected == nil { + return false + } + if errors.Is(err, expected) { + return true + } + e1, e2 := err.Error(), expected.Error() + return strings.Contains(e1, e2) +} func TestImportCommand_Validation(t *testing.T) { buf := bytes.Buffer{} stdin, stdout, stderr := GetIO(buf) cm := NewImportCommand(stdin, stdout, stderr) err := cm.Run(context.Background()) - if err != pilosa.ErrIndexRequired { - t.Fatalf("Command not working, expect: %s, actual: '%s'", pilosa.ErrIndexRequired, err) + if !errContains(err, pilosa.ErrIndexRequired) { + t.Fatalf("wrong error: expected %q, got: '%v'", pilosa.ErrIndexRequired, err) } cm.Index = "i" err = cm.Run(context.Background()) - if err != pilosa.ErrFieldRequired { - t.Fatalf("Command not working, expect: %s, actual: '%s'", pilosa.ErrFieldRequired, err) + if !errContains(err, pilosa.ErrFieldRequired) { + t.Fatalf("wrong error: expected %q, got: '%v'", pilosa.ErrFieldRequired, err) } cm.Field = "f" err = cm.Run(context.Background()) - if err.Error() != "path required" { - t.Fatalf("Command not working, expect: %s, actual: '%s'", "path required", err) + pathRequired := errors.New("path required") + if !errContains(err, pathRequired) { + t.Fatalf("wrong error: expected %q, got: '%v'", pathRequired, err) } } diff --git a/ctl/restore.go b/ctl/restore.go index 904f71c5c..be6ee1ce1 100644 --- a/ctl/restore.go +++ b/ctl/restore.go @@ -77,9 +77,9 @@ func (cmd *RestoreCommand) Run(ctx context.Context) (err error) { // Validate arguments. if cmd.Path == "" { - return fmt.Errorf("-s flag required") + return fmt.Errorf("%w: -s flag required", UsageError) } else if cmd.Concurrency <= 0 { - return fmt.Errorf("concurrency must be at least one") + return fmt.Errorf("%w: concurrency must be at least one", UsageError) } // Parse TLS configuration for node-specific clients. diff --git a/ctl/restore_tar.go b/ctl/restore_tar.go index 7df8f4f34..6b2651e32 100644 --- a/ctl/restore_tar.go +++ b/ctl/restore_tar.go @@ -69,7 +69,7 @@ func (cmd *RestoreTarCommand) Run(ctx context.Context) (err error) { // Validate arguments. if cmd.Path == "" { - return fmt.Errorf("-s flag required") + return fmt.Errorf("%w: -s flag required", UsageError) } useStdin := cmd.Path == "-" diff --git a/ctl/util.go b/ctl/util.go index 4ee0762b0..62368b40e 100644 --- a/ctl/util.go +++ b/ctl/util.go @@ -13,6 +13,14 @@ import ( "github.com/pkg/errors" ) +type ctlUsageError struct{} + +func (c ctlUsageError) Error() string { + return "usage error" +} + +var UsageError ctlUsageError + // startProfilingServer starts a server which handles /debug/pprof and // /debug/fgprof for use in utilities we might want to profile but // wouldn't otherwise be running an http server. Caller should call