Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

soroban-rpc: improve missing command line arguments message #880

Merged
merged 5 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions cmd/soroban-rpc/internal/config/config_option.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,28 @@ type ConfigOptions []*ConfigOption

// Validate all the config options.
func (options ConfigOptions) Validate() error {
var missingOptions []errMissingRequiredOption
for _, option := range options {
if option.Validate != nil {
err := option.Validate(option)
if err != nil {
return errors.Wrap(err, fmt.Sprintf("Invalid config value for %s", option.Name))
if err == nil {
continue
}
if missingOption, ok := err.(errMissingRequiredOption); ok {
missingOptions = append(missingOptions, missingOption)
continue
}
return errors.Wrap(err, fmt.Sprintf("Invalid config value for %s", option.Name))
}
}
if len(missingOptions) > 0 {
// we had one or more missing options, combine these all into a single error.
errString := "The following required configuration parameters are missing:"
for _, missingOpt := range missingOptions {
errString += "\n*\t" + missingOpt.strErr
errString += "\n \t" + missingOpt.usage
}
return &errMissingRequiredOption{strErr: errString}
}
return nil
}
Expand Down
3 changes: 1 addition & 2 deletions cmd/soroban-rpc/internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/stellar/go/network"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -48,7 +47,7 @@ func TestConfigLoadDefaults(t *testing.T) {
require.NoError(t, cfg.loadDefaults())

// Check that the defaults are set
assert.Equal(t, network.FutureNetworkPassphrase, cfg.NetworkPassphrase)
assert.Equal(t, defaultHTTPEndpoint, cfg.Endpoint)
assert.Equal(t, uint(runtime.NumCPU()), cfg.PreflightWorkerCount)
}

Expand Down
35 changes: 26 additions & 9 deletions cmd/soroban-rpc/internal/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"github.com/stellar/soroban-tools/cmd/soroban-rpc/internal/ledgerbucketwindow"
)

const defaultHTTPEndpoint = "localhost:8000"

func (cfg *Config) options() ConfigOptions {
if cfg.optionsCache != nil {
return *cfg.optionsCache
Expand All @@ -41,7 +43,7 @@ func (cfg *Config) options() ConfigOptions {
Name: "endpoint",
Usage: "Endpoint to listen and serve on",
ConfigKey: &cfg.Endpoint,
DefaultValue: "localhost:8000",
DefaultValue: defaultHTTPEndpoint,
},
{
Name: "admin-endpoint",
Expand Down Expand Up @@ -189,11 +191,10 @@ func (cfg *Config) options() ConfigOptions {
ConfigKey: &cfg.FriendbotURL,
},
{
Name: "network-passphrase",
Usage: "Network passphrase of the Stellar network transactions should be signed for",
ConfigKey: &cfg.NetworkPassphrase,
DefaultValue: network.FutureNetworkPassphrase,
Validate: required,
Name: "network-passphrase",
Usage: "Network passphrase of the Stellar network transactions should be signed for. Commonly used values are \"" + network.FutureNetworkPassphrase + "\", \"" + network.TestNetworkPassphrase + "\" and \"" + network.PublicNetworkPassphrase + "\"",
ConfigKey: &cfg.NetworkPassphrase,
Validate: required,
},
{
Name: "db-path",
Expand Down Expand Up @@ -399,9 +400,25 @@ func (cfg *Config) options() ConfigOptions {
return *cfg.optionsCache
}

type errMissingRequiredOption struct {
strErr string
usage string
}

func (e errMissingRequiredOption) Error() string {
return e.strErr
}

func required(option *ConfigOption) error {
if !reflect.ValueOf(option.ConfigKey).Elem().IsZero() {
return nil
switch reflect.ValueOf(option.ConfigKey).Elem().Kind() {
case reflect.Slice:
if reflect.ValueOf(option.ConfigKey).Elem().Len() > 0 {
return nil
}
default:
if !reflect.ValueOf(option.ConfigKey).Elem().IsZero() {
return nil
}
}

waysToSet := []string{}
Expand All @@ -426,7 +443,7 @@ func required(option *ConfigOption) error {
advice = fmt.Sprintf(" Please %s, %s, or %s.", waysToSet[0], waysToSet[1], waysToSet[2])
}

return fmt.Errorf("%s is required.%s", option.Name, advice)
return errMissingRequiredOption{strErr: fmt.Sprintf("%s is required.%s", option.Name, advice), usage: option.Usage}
}

func positive(option *ConfigOption) error {
Expand Down
2 changes: 1 addition & 1 deletion cmd/soroban-rpc/internal/config/toml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestBasicTomlWriting(t *testing.T) {

// Spot-check that the output looks right. Try to check one value for each
// type of option. (string, duration, uint, etc...)
assert.Contains(t, out, "NETWORK_PASSPHRASE = \"Test SDF Future Network ; October 2022\"")
assert.Contains(t, out, "ENDPOINT = \"localhost:8000\"")
assert.Contains(t, out, "STELLAR_CORE_TIMEOUT = \"2s\"")
assert.Contains(t, out, "STELLAR_CAPTIVE_CORE_HTTP_PORT = 11626")
assert.Contains(t, out, "LOG_LEVEL = \"info\"")
Expand Down