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

Don't print a stacktrace on config errors #478

Merged
merged 1 commit into from
May 11, 2022

Conversation

julianbrost
Copy link
Contributor

Throwing a stacktrace at a user isn't the nicest thing to do (looks more like the application did something wrong rather than the user did something wrong).

Exit code 2 seems to be what go does when you panic it, but invalid config is somewhat similar to invalid cli options (which also uses exit code 2 at the moment), so I kept that one.

Tests

Before

go run ./cmd/icingadb -c <(printf '{database: {type: invalid}}')
panic: invalid configuration: unknown database type "invalid", must be one of: "mysql", "pgsql"

goroutine 1 [running]:
github.com/icinga/icingadb/pkg/utils.Fatal(...)
	/home/jbrost/dev/icingadb/pkg/utils/utils.go:114
github.com/icinga/icingadb/internal/command.New()
	/home/jbrost/dev/icingadb/internal/command/command.go:41 +0x185
main.run()
	/home/jbrost/dev/icingadb/cmd/icingadb/main.go:40 +0x57
main.main()
	/home/jbrost/dev/icingadb/cmd/icingadb/main.go:36 +0x19
exit status 2

After

$ go run ./cmd/icingadb -c <(printf '{database: {type: invalid}}')
invalid configuration: unknown database type "invalid", must be one of: "mysql", "pgsql"
exit status 2

@cla-bot cla-bot bot added the cla/signed label May 10, 2022
@julianbrost julianbrost marked this pull request as ready for review May 10, 2022 14:48
@Al2Klimov Al2Klimov merged commit 811b69a into master May 11, 2022
@Al2Klimov Al2Klimov deleted the feature/no-stacktrace-on-config-error branch May 11, 2022 08:33
@lippserd lippserd added this to the 1.0.0 milestone May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants