storj / gateway-st

Single-tenant, S3-compatible server to interact with the Storj network
Apache License 2.0
71 stars 19 forks source link

Friendly error messages for bad configs #40

Closed wthorp closed 2 years ago

wthorp commented 2 years ago

Currently a bad config results in a noisy stack trace. Validate configs up front with friendlier error messages.

% gateway run
2021-11-01T16:20:08.530-0400    DEBUG   process/tracing.go:72   Anonymized tracing disabled
2021-11-01T16:20:08.530-0400    DEBUG   process/debug.go:37 debug server listening on 127.0.0.1:58073
2021-11-01T16:20:08.530-0400    DEBUG   process/metrics.go:79   Telemetry disabled
2021-11-01T16:20:08.530-0400    INFO    st-gateway/main.go:151  Starting Storj DCS S3 Gateway

2021-11-01T16:20:08.530-0400    INFO    st-gateway/main.go:152  Endpoint: 127.0.0.1:7777

2021-11-01T16:20:08.530-0400    INFO    st-gateway/main.go:153  Access key: insecure-dev-access-key

2021-11-01T16:20:08.530-0400    INFO    st-gateway/main.go:154  Secret key: insecure-dev-secret-key

2021-11-01T16:20:08.530-0400    WARN    st-gateway/main.go:158  Failed to contact Satellite. Perhaps your configuration is invalid?
main.cmdRun
    /Users/bill/storj/st-gateway/main.go:158
storj.io/private/process.cleanup.func1.4
    /Users/bill/go/pkg/mod/storj.io/private@v0.0.0-20210810102517-434aeab3f17d/process/exec_conf.go:363
storj.io/private/process.cleanup.func1
    /Users/bill/go/pkg/mod/storj.io/private@v0.0.0-20210810102517-434aeab3f17d/process/exec_conf.go:381
github.com/spf13/cobra.(*Command).execute
    /Users/bill/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:852
github.com/spf13/cobra.(*Command).ExecuteC
    /Users/bill/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:960
github.com/spf13/cobra.(*Command).Execute
    /Users/bill/go/pkg/mod/github.com/spf13/cobra@v1.1.3/command.go:897
storj.io/private/process.ExecWithCustomConfig
    /Users/bill/go/pkg/mod/storj.io/private@v0.0.0-20210810102517-434aeab3f17d/process/exec_conf.go:88
storj.io/private/process.Exec
    /Users/bill/go/pkg/mod/storj.io/private@v0.0.0-20210810102517-434aeab3f17d/process/exec_conf.go:65
main.main
    /Users/bill/storj/st-gateway/main.go:434
runtime.main
    /usr/local/go/src/runtime/proc.go:255
2021-11-01T16:20:08.530-0400    DEBUG   process/exec_conf.go:394    Unrecoverable error {"error": "gateway setup error: uplink: invalid access grant format", "errorVerbose": "gateway setup error: uplink: invalid access grant format\n\tstorj.io/uplink.ParseAccess:98\n\tmain.AccessConfig.GetAccess:48\n\tmain.checkCfg:175\n\tmain.cmdRun:156\n\tstorj.io/private/process.cleanup.func1.4:363\n\tstorj.io/private/process.cleanup.func1:381\n\tgithub.com/spf13/cobra.(*Command).execute:852\n\tgithub.com/spf13/cobra.(*Command).ExecuteC:960\n\tgithub.com/spf13/cobra.(*Command).Execute:897\n\tstorj.io/private/process.ExecWithCustomConfig:88\n\tstorj.io/private/process.Exec:65\n\tmain.main:434\n\truntime.main:255"}
halkyon commented 2 years ago

On a release binary, this behaves quite differently, which sets the --defaults flag to release by default and supresses a whole bunch of backtraces and verbose output. This is done by the storj/private process package automatically.

See the difference if it's set correctly:

gateway run --defaults release
2021-11-02T19:02:29.369+1300    INFO    Telemetry enabled   {"instance ID": "54:05:db:37:35:0f"}
2021-11-02T19:02:29.369+1300    INFO    Starting Storj DCS S3 Gateway
2021-11-02T19:02:29.369+1300    INFO    Endpoint: 127.0.0.1:7777
2021-11-02T19:02:29.369+1300    INFO    Access key: insecure-dev-access-key
2021-11-02T19:02:29.369+1300    INFO    Secret key: insecure-dev-secret-key

Error: gateway setup error: uplink: invalid access grant format

Given this, I'm not sure if we should change anything here for dev mode, as it's supposed to be more verbose by design? The error message could perhaps be a bit more helpful, and maybe we could remove the backtrace for errors in this validation context.

amwolff commented 2 years ago

Should we limit the stack trace depth?

halkyon commented 2 years ago

I think we could do that. I'm having a look at the zap API to see if there's any way to control showing a backtrace on a per-log event basis. I think it's only enable or disable for the entire logger instance at creation, not so much at runtime. Perhaps we could create another logger specifically for validation that has the backtrace disabled or limited (?)