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

Improve error messages when server misconfigured #1520

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

jameshiew
Copy link
Contributor

Motivation

Relates to #1435

This PR is trying out a way to improve the error message shown when something is misconfigured.

Currently running the server binary (cargo run in the svix-server/ folder) panics when some needed config is missing.

e.g. listen_address not set

thread 'main' panicked at svix-server/src/cfg.rs:550:10:
Error loading configuration: Error { tag: Tag::Default, profile: Some(Profile(Uncased { string: "default" })), metadata: None, path: [], kind: MissingField("listen_address"), prev: None }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

e.g. jwt_secret not set - in this case the error message doesn't indicate the missing field due to an issue with it coming from an inlined struct inside ConfigurationInner - SergioBenitez/Figment#80

thread 'main' panicked at svix-server/src/cfg.rs:550:10:
Error loading configuration: Error { tag: Tag::Default, profile: Some(Profile(Uncased { string: "default" })), metadata: None, path: [], kind: Message("data did not match any variant of untagged enum JwtSigningConfig"), prev: None }

Solution

If cfg::load() errors out, terminate with exit code 1 and print the formatted error to stderr. I tried switching to using anyhow::Result for main() and cfg::load() to make propagating errors with human friendly context easier.

Error: failed to extract configuration

Caused by:
    missing field `listen_address`

Check upfront that jwt_secret is set and override the error message to be more helpful due to SergioBenitez/Figment#80

Error: failed to extract configuration

Caused by:
    missing field `jwt_secret`

- add better error message for missing JWT secret
- print configuration errors in friendlier format and exit with code 1
@jameshiew jameshiew requested a review from a team as a code owner November 16, 2024 19:25
server/svix-server/src/cfg.rs Outdated Show resolved Hide resolved
server/svix-server/src/cfg.rs Outdated Show resolved Hide resolved
server/svix-server/src/main.rs Outdated Show resolved Hide resolved
@jameshiew
Copy link
Contributor Author

Thanks for reviewing! I've updated to bail! with the wait_for_dsn err

 > SVIX_JWT_SECRET='a' SVIX_REDIS_DSN='redis://redis:6379' cargo run -- --wait-for 1
   Compiling svix-server v1.40.0 (/opt/workspace/3p/svix-webhooks/server/svix-server)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 6.00s
     Running `/opt/workspace/caches/cargo/debug/svix-server --wait-for 1`
2024-11-18T18:21:00.138658Z ERROR svix_server: Waiting for service PostgreSQL host=pgbouncer port=5432 timed out
Error: Waiting for service PostgreSQL host=pgbouncer port=5432 timed out

Copy link
Member

@svix-jplatte svix-jplatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@svix-onelson svix-onelson merged commit 38130cc into svix:main Nov 19, 2024
6 checks passed
@jameshiew jameshiew deleted the improve-cfg-err-msg branch November 24, 2024 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants