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

Do not crash on startup when configuration is invalid #2140

Merged
merged 6 commits into from Apr 26, 2021

Conversation

filipw
Copy link
Member

@filipw filipw commented Apr 24, 2021

At the moment when bootstrapping configuration, if anything invalid is found (e.g. JSON structure is bad) we have an unhandled exception and failure to start.

Since the config is really optional I think it is better to swallow the exception, log an error and start with default config. This has been source of several issues already (got closed after the user fixed the config on their side).

since when the config is bootstrapped, the DI container is not yet initialized, logging of the possible error is deferred until the logger is available.

Copy link
Member

@bjorkstromm bjorkstromm left a comment

Choose a reason for hiding this comment

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

LGTM!

Would it be possible (useful) to also add a test for the configuration builder?

@filipw
Copy link
Member Author

filipw commented Apr 26, 2021

Would it be possible (useful) to also add a test for the configuration builder?

I added some tests

@filipw filipw merged commit 6011913 into master Apr 26, 2021
@filipw filipw deleted the bugfix/config-fail branch April 26, 2021 14:54
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.

None yet

3 participants