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
Merged

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.

@filipw filipw force-pushed the bugfix/config-fail branch from 3034eec to fdd45ef Apr 24, 2021
Copy link
Member

@bjorkstromm bjorkstromm left a comment

LGTM!

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

@filipw
Copy link
Member Author

@filipw 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
16 checks passed
16 checks passed
@github-actions
Build (ubuntu-18.04)
Details
@github-actions
Test (ubuntu-18.04, OmniSharp.MSBuild.Tests,OmniSharp.Roslyn.CSharp.Tests,OmniSharp.Cake.Tests,Om... Test (ubuntu-18.04, OmniSharp.MSBuild.Tests,OmniSharp.Roslyn.CSharp.Tests,OmniSharp.Cake.Tests,Om...
Details
@github-actions
Build (windows-2019)
Details
@github-actions
Test (ubuntu-18.04, OmniSharp.DotNetTest.Tests) Test (ubuntu-18.04, OmniSharp.DotNetTest.Tests)
Details
@github-actions
Build (macos-10.15)
Details
@github-actions
Test (windows-2019, OmniSharp.MSBuild.Tests,OmniSharp.Roslyn.CSharp.Tests,OmniSharp.Cake.Tests,Om... Test (windows-2019, OmniSharp.MSBuild.Tests,OmniSharp.Roslyn.CSharp.Tests,OmniSharp.Cake.Tests,Om...
Details
@github-actions
Test (windows-2019, OmniSharp.DotNetTest.Tests) Test (windows-2019, OmniSharp.DotNetTest.Tests)
Details
@github-actions
Test (macos-10.15, OmniSharp.MSBuild.Tests,OmniSharp.Roslyn.CSharp.Tests,OmniSharp.Cake.Tests,Omn... Test (macos-10.15, OmniSharp.MSBuild.Tests,OmniSharp.Roslyn.CSharp.Tests,OmniSharp.Cake.Tests,Omn...
Details
@github-actions
Test (macos-10.15, OmniSharp.DotNetTest.Tests) Test (macos-10.15, OmniSharp.DotNetTest.Tests)
Details
@azure-pipelines
OmniSharp.omnisharp-roslyn Build #1.37.9-PullRequest2140.28 succeeded
Details
@azure-pipelines
OmniSharp.omnisharp-roslyn (GitVersion) GitVersion succeeded
Details
@azure-pipelines
OmniSharp.omnisharp-roslyn (Linux) Linux succeeded
Details
@azure-pipelines
OmniSharp.omnisharp-roslyn (Release) Release succeeded
Details
@azure-pipelines
OmniSharp.omnisharp-roslyn (Windows) Windows succeeded
Details
@azure-pipelines
OmniSharp.omnisharp-roslyn (macOS) macOS succeeded
Details
license/cla All CLA requirements met.
Details
@filipw filipw deleted the bugfix/config-fail branch Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants