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

Add error handling to discovery host #31

Merged
merged 2 commits into from
Oct 31, 2017

Conversation

vlow
Copy link
Contributor

@vlow vlow commented Oct 31, 2017

Setting the socket options in the DiscoveryHost class fails on some system configurations. The failure triggers a SocketException as described in issue #8. A first step to address this problem has been made by making the client discovery optional. Still, if you want to use the client discovery, you risk running into the above mentioned exception which will effectively take down the whole application.

This PR adds a little error handling which logs the failed address and the exception. If the error occurrs, the failed address is ignored and the initialization will resume with the next address. Logging more information about this problem is essential since it might give some insight in what actually causes the crash. That might give us enough information to eventually fix the filter and avoid the exception in the first place.

This PR also updates the NuGet binary to version 4.3.0 in order to support C# 6 language features.

vlow added 2 commits October 31, 2017 13:32
If the setup for a single socket fails, the whole initialization process is aborted. The exception is never handled and might therefore take down the whole application. This fix logs the exception for the single socket and continues with the next socket rather than aborting the initialization on failure.
Replaces the nuget binary with a newer version that supports C# 6 syntax.
@ayende ayende merged commit 27686da into ayende:master Oct 31, 2017
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.

2 participants