diff --git a/cmd/auth_token.go b/cmd/auth_token.go index cfc131de6..0c8d52ca7 100644 --- a/cmd/auth_token.go +++ b/cmd/auth_token.go @@ -3,7 +3,6 @@ package cmd import ( - "context" "io" "github.com/featurebasedb/featurebase/v3/ctl" @@ -18,9 +17,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 f1754173a..4f8a9e071 100644 --- a/cmd/backup.go +++ b/cmd/backup.go @@ -3,7 +3,6 @@ package cmd import ( - "context" "io" "github.com/featurebasedb/featurebase/v3/ctl" @@ -18,9 +17,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 d96a6fd0d..654d09af3 100644 --- a/cmd/backup_tar.go +++ b/cmd/backup_tar.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/featurebasedb/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 ce77a0862..d09a07660 100644 --- a/cmd/chksum.go +++ b/cmd/chksum.go @@ -3,7 +3,6 @@ package cmd import ( - "context" "io" "github.com/featurebasedb/featurebase/v3/ctl" @@ -19,9 +18,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 40aba36c6..9c7681f24 100644 --- a/cmd/cli.go +++ b/cmd/cli.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/featurebasedb/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 0f4cd8e79..c72d58e8c 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -24,7 +24,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 bbbbf215a..e43518962 100644 --- a/cmd/export.go +++ b/cmd/export.go @@ -3,7 +3,6 @@ package cmd import ( - "context" "io" "github.com/spf13/cobra" @@ -28,9 +27,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 851a189b8..86b9e01a7 100644 --- a/cmd/generate_config.go +++ b/cmd/generate_config.go @@ -3,7 +3,6 @@ package cmd import ( - "context" "io" "github.com/spf13/cobra" @@ -20,9 +19,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 725c3a3b8..a8c1a24c5 100644 --- a/cmd/import.go +++ b/cmd/import.go @@ -61,7 +61,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 85f5152fa..1a7285606 100644 --- a/cmd/keygen.go +++ b/cmd/keygen.go @@ -3,7 +3,6 @@ package cmd import ( - "context" "io" "github.com/featurebasedb/featurebase/v3/ctl" @@ -18,9 +17,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 7d755dc50..733070325 100644 --- a/cmd/rbf.go +++ b/cmd/rbf.go @@ -3,7 +3,6 @@ package cmd import ( - "context" "errors" "fmt" "io" @@ -45,9 +44,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 } @@ -79,9 +76,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 } @@ -103,9 +98,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() @@ -140,9 +133,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 55fcff5fe..1011afd25 100644 --- a/cmd/restore.go +++ b/cmd/restore.go @@ -3,7 +3,6 @@ package cmd import ( - "context" "io" "github.com/featurebasedb/featurebase/v3/ctl" @@ -18,9 +17,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 e27c5f3bc..5c3de02bb 100644 --- a/cmd/restore_tar.go +++ b/cmd/restore_tar.go @@ -2,7 +2,6 @@ package cmd import ( - "context" "io" "github.com/featurebasedb/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 837cd84b9..656649a9e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -3,16 +3,49 @@ package cmd import ( + "context" + "errors" "fmt" "io" "strings" - pilosa "github.com/featurebasedb/featurebase/v3" + 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", @@ -49,6 +82,7 @@ at https://docs.featurebase.com/. 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 ea7447def..a3a75ed7c 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -59,8 +59,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 ad2f74e24..d01c0c453 100644 --- a/ctl/backup.go +++ b/ctl/backup.go @@ -6,18 +6,19 @@ import ( "context" "crypto/tls" "encoding/json" + "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "time" - pilosa "github.com/featurebasedb/featurebase/v3" - "github.com/featurebasedb/featurebase/v3/authn" - "github.com/featurebasedb/featurebase/v3/disco" - "github.com/featurebasedb/featurebase/v3/encoding/proto" - "github.com/featurebasedb/featurebase/v3/server" - "github.com/pkg/errors" + pilosa "github.com/molecula/featurebase/v3" + "github.com/molecula/featurebase/v3/authn" + "github.com/molecula/featurebase/v3/disco" + "github.com/molecula/featurebase/v3/encoding/proto" + "github.com/molecula/featurebase/v3/server" "github.com/ricochet2200/go-disk-usage/du" "golang.org/x/sync/errgroup" ) @@ -81,19 +82,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 } @@ -137,7 +138,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 fad3c1a13..961aebb24 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 9a448db78..754efaa03 100644 --- a/ctl/export.go +++ b/ctl/export.go @@ -4,11 +4,12 @@ package ctl import ( "context" + "fmt" "io" "os" - pilosa "github.com/featurebasedb/featurebase/v3" - "github.com/featurebasedb/featurebase/v3/server" + pilosa "github.com/molecula/featurebase/v3" + "github.com/molecula/featurebase/v3/server" "github.com/pkg/errors" ) @@ -43,9 +44,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 79f7e9fa7..09404c5e8 100644 --- a/ctl/export_test.go +++ b/ctl/export_test.go @@ -9,8 +9,8 @@ import ( "strings" "testing" - pilosa "github.com/featurebasedb/featurebase/v3" - "github.com/featurebasedb/featurebase/v3/test" + pilosa "github.com/molecula/featurebase/v3" + "github.com/molecula/featurebase/v3/test" ) func TestExportCommand_Validation(t *testing.T) { @@ -20,14 +20,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 a3b2923c8..ccd808f5c 100644 --- a/ctl/import.go +++ b/ctl/import.go @@ -80,11 +80,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 dc492f0a2..712a42c3b 100644 --- a/ctl/import_test.go +++ b/ctl/import_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -26,25 +27,47 @@ import ( "github.com/golang-jwt/jwt" ) +// 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 3c118f7f6..651e14370 100644 --- a/ctl/restore.go +++ b/ctl/restore.go @@ -78,9 +78,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 3f60a1154..a4fc1cef3 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 a85d4cb60..07d716971 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